19.05.2010 22:53, Christoph Hellwig wrote:
The difference between the start sectors of two requests can be larger than the size of the "int" type, which can lead to a not correctly sorted multiwrite array and thus spurious I/O errors and filesystem corruption due to incorrect request merges.So instead of doing the cute sector arithmetics trick spell out the exact comparisms. Spotted by Kevin Wolf based on a testcase from Michael Tokarev. Signed-off-by: Christoph Hellwig<h...@lst.de> Index: qemu/block.c =================================================================== --- qemu.orig/block.c 2010-05-19 17:08:24.970255636 +0200 +++ qemu/block.c 2010-05-19 17:17:34.227006021 +0200 @@ -1933,7 +1933,19 @@ static void multiwrite_cb(void *opaque, static int multiwrite_req_compare(const void *a, const void *b) { - return (((BlockRequest*) a)->sector - ((BlockRequest*) b)->sector); + const BlockRequest *req1 = a, *req2 = b; + + /* + * Note that we can't simply subtract req2->sector from req1->sector + * here as that could overflow the return value. + */ + if (req1->sector> req2->sector) { + return 1; + } else if (req1->sector< req2->sector) { + return -1; + } else { + return 0; + } }
Fantastic. I ran numerous mkfs'es of my 1.5Tb image, -- every single run before resulted in at least one "fun" comparison there. There is NO MORE OVERLAPPING REQUESTS. At least I can't trigger one. Fantastic! Thanks guys! Now the question is if the previous patch by Avi is actually worth to apply -- I mean this one: [Qemu-devel] [PATCH +stable] block: don't attempt to merge overlapping requests But it looks like all 3 should be applied: [Qemu-devel] [PATCH] block: fix sector comparism in multiwrite_req_compare [Qemu-devel] [PATCH +stable] block: don't attempt to merge overlapping requests [Qemu-devel] [PATCH] virtio-blk: fix barrier support ..to -stable. I wonder how many similar "funny" cases are still around... ;) /mjt
