BZ29402 https://bugzilla.kernel.org/show_bug.cgi?id=29402 We can hit serious mis-synchronization in bio completion path of blkdev_issue_zeroout() leading to a panic. The problem is that when we are going to wait_for_completion() in blkdev_issue_zeroout() we check if the bb.done equals issued (number of submitted bios). If it does, we can skip the wait_for_completition() and just out of the function since there is nothing to wait for. However, there is a ordering problem because bio_batch_end_io() is calling atomic_inc(&bb->done) before complete(), hence it might seem to blkdev_issue_zeroout() that all bios has been completed and exit. At this point when bio_batch_end_io() is going to call complete(bb->wait), bb and wait does not longer exist since it was allocated on stack in blkdev_issue_zeroout() ==> panic! (thread 1) (thread 2) bio_batch_end_io() blkdev_issue_zeroout() if(bb) { ... if (bb->end_io) ... bb->end_io(bio, err); ... atomic_inc(&bb->done); ... ... while (issued != atomic_read(&bb.done)) ... (let issued == bb.done) ... (do the rest of the function) ... return ret; complete(bb->wait); ^^^^^^^^ panic We can fix this easily by simplifying bio_batch and completion counting. We can count completion locally in blkdev_issue_zeroout() without need of locking or atomic operation because we are the only one handling issued variable holding the number of submitted bios. So remove atomic_t done from struct bio_batch. Also remove bio_end_io_t *end_io since it is not used. Signed-off-by: Lukas Czerner <lczerner@xxxxxxxxxx> Reported-by: Eric Whitney <eric.whitney@xxxxxx> Tested-by: Eric Whitney <eric.whitney@xxxxxx> CC: Jens Axboe <axboe@xxxxxxxxx> CC: Dmitry Monakhov <dmonakhov@xxxxxxxxxx> --- block/blk-lib.c | 12 ++---------- 1 files changed, 2 insertions(+), 10 deletions(-) diff --git a/block/blk-lib.c b/block/blk-lib.c index 1a320d2..dcee6e2 100644 --- a/block/blk-lib.c +++ b/block/blk-lib.c @@ -106,10 +106,8 @@ EXPORT_SYMBOL(blkdev_issue_discard); struct bio_batch { - atomic_t done; unsigned long flags; struct completion *wait; - bio_end_io_t *end_io; }; static void bio_batch_end_io(struct bio *bio, int err) @@ -122,12 +120,8 @@ static void bio_batch_end_io(struct bio *bio, int err) else clear_bit(BIO_UPTODATE, &bb->flags); } - if (bb) { - if (bb->end_io) - bb->end_io(bio, err); - atomic_inc(&bb->done); + if (bb) complete(bb->wait); - } bio_put(bio); } @@ -153,10 +147,8 @@ int blkdev_issue_zeroout(struct block_device *bdev, sector_t sector, unsigned int sz, issued = 0; DECLARE_COMPLETION_ONSTACK(wait); - atomic_set(&bb.done, 0); bb.flags = 1 << BIO_UPTODATE; bb.wait = &wait; - bb.end_io = NULL; submit: ret = 0; @@ -190,7 +182,7 @@ submit: } /* Wait for bios in-flight */ - while (issued != atomic_read(&bb.done)) + while (issued--) wait_for_completion(&wait); if (!test_bit(BIO_UPTODATE, &bb.flags)) -- 1.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
