lkml.org 
[lkml]   [2011]   [May]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    Subject[PATCH UPDATED 2/3 v2.6.39-rc7] block: make disk_block_events() properly wait for work cancellation
    disk_block_events() should guarantee that the event work is not in
    flight on return and once blocked it shouldn't issue further
    cancellations.

    Because there was no synchronization between the first blocker doing
    cancel_delayed_work_sync() and the following blockers, the following
    blockers could finish before cancellation was complete, which broke
    both guarantees - event work could be in flight and cancellation could
    happen after return.

    This bug triggered WARN_ON_ONCE() in disk_clear_events() reported in
    bug#34662.

    https://bugzilla.kernel.org/show_bug.cgi?id=34662

    Fix it by adding an outer mutex which protects both block count
    manipulation and work cancellation.

    -v2: Use outer mutex instead of bit waitqueue per Linus.

    Signed-off-by: Tejun Heo <tj@kernel.org>
    Tested-by: Sitsofe Wheeler <sitsofe@yahoo.com>
    Reported-by: Sitsofe Wheeler <sitsofe@yahoo.com>
    Reported-by: Borislav Petkov <bp@alien8.de>
    Reported-by: Meelis Roos <mroos@linux.ee>
    Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
    Cc: Andrew Morton <akpm@linux-foundation.org>
    Cc: Jens Axboe <axboe@kernel.dk>
    Cc: Kay Sievers <kay.sievers@vrfy.org>
    ---
    Yeap, it's so much simpler this way. Thanks.

    block/genhd.c | 10 ++++++++++
    1 file changed, 10 insertions(+)

    Index: work/block/genhd.c
    ===================================================================
    --- work.orig/block/genhd.c
    +++ work/block/genhd.c
    @@ -1371,6 +1371,7 @@ struct disk_events {
    struct gendisk *disk; /* the associated disk */
    spinlock_t lock;

    + struct mutex block_mutex; /* protects blocking */
    int block; /* event blocking depth */
    unsigned int pending; /* events already sent out */
    unsigned int clearing; /* events being cleared */
    @@ -1438,12 +1439,20 @@ void disk_block_events(struct gendisk *d
    if (!ev)
    return;

    + /*
    + * Outer mutex ensures that the first blocker completes canceling
    + * the event work before further blockers are allowed to finish.
    + */
    + mutex_lock(&ev->block_mutex);
    +
    spin_lock_irqsave(&ev->lock, flags);
    cancel = !ev->block++;
    spin_unlock_irqrestore(&ev->lock, flags);

    if (cancel)
    cancel_delayed_work_sync(&disk->ev->dwork);
    +
    + mutex_unlock(&ev->block_mutex);
    }

    static void __disk_unblock_events(struct gendisk *disk, bool check_now)
    @@ -1751,6 +1760,7 @@ static void disk_add_events(struct gendi
    INIT_LIST_HEAD(&ev->node);
    ev->disk = disk;
    spin_lock_init(&ev->lock);
    + mutex_init(&ev->block_mutex);
    ev->block = 1;
    ev->poll_msecs = -1;
    INIT_DELAYED_WORK(&ev->dwork, disk_events_workfn);

    \
     
     \ /
      Last update: 2011-05-17 17:51    [W:0.023 / U:30.496 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site