lkml.org 
[lkml]   [2011]   [Mar]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    Subject[16/87] block: fix mis-synchronisation in blkdev_issue_zeroout()
    2.6.37-stable review patch.  If anyone has any objections, please let us know.

    ------------------

    From: Lukas Czerner <lczerner@redhat.com>

    commit 0aeea18964173715a1037034ef6838198f319319 upstream.

    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.

    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>
    Reviewed-by: Jeff Moyer <jmoyer@redhat.com>
    CC: Dmitry Monakhov <dmonakhov@openvz.org>
    Signed-off-by: Jens Axboe <jaxboe@fusionio.com>
    Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

    ---
    block/blk-lib.c | 19 +++++++------------
    1 file changed, 7 insertions(+), 12 deletions(-)

    --- a/block/blk-lib.c
    +++ b/block/blk-lib.c
    @@ -109,7 +109,6 @@ 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 +121,9 @@ static void bio_batch_end_io(struct bio
    else
    clear_bit(BIO_UPTODATE, &bb->flags);
    }
    - if (bb) {
    - if (bb->end_io)
    - bb->end_io(bio, err);
    - atomic_inc(&bb->done);
    - complete(bb->wait);
    - }
    + if (bb)
    + if (atomic_dec_and_test(&bb->done))
    + complete(bb->wait);
    bio_put(bio);
    }

    @@ -150,13 +146,12 @@ int blkdev_issue_zeroout(struct block_de
    int ret;
    struct bio *bio;
    struct bio_batch bb;
    - unsigned int sz, issued = 0;
    + unsigned int sz;
    DECLARE_COMPLETION_ONSTACK(wait);

    - atomic_set(&bb.done, 0);
    + atomic_set(&bb.done, 1);
    bb.flags = 1 << BIO_UPTODATE;
    bb.wait = &wait;
    - bb.end_io = NULL;

    submit:
    ret = 0;
    @@ -185,12 +180,12 @@ submit:
    break;
    }
    ret = 0;
    - issued++;
    + atomic_inc(&bb.done);
    submit_bio(WRITE, bio);
    }

    /* Wait for bios in-flight */
    - while (issued != atomic_read(&bb.done))
    + if (!atomic_dec_and_test(&bb.done))
    wait_for_completion(&wait);

    if (!test_bit(BIO_UPTODATE, &bb.flags))



    \
     
     \ /
      Last update: 2011-03-22 00:45    [W:0.027 / U:0.396 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site