lkml.org 
[lkml]   [2011]   [Mar]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Subject[PATCH] block: fix mis-synchronisation in blkdev_issue_zeroout()
Date
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@redhat.com>
Reported-by: Eric Whitney <eric.whitney@hp.com>
Tested-by: Eric Whitney <eric.whitney@hp.com>
CC: Jens Axboe <axboe@kernel.dk>
CC: Dmitry Monakhov <dmonakhov@openvz.org>
---
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


\
 
 \ /
  Last update: 2011-03-06 12:19    [W:0.078 / U:64.756 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site