Skip to content

Commit

Permalink
block: prevent race/cleanup
Browse files Browse the repository at this point in the history
Remove a race condition which causes a warning in disk_clear_events.  This
is a race between disk_clear_events() and disk_flush_events().
ev->clearing will be altered by disk_flush_events() even though we are
blocking event checking through disk_flush_events().  If this happens
after ev->clearing was cleared for disk_clear_events(), this can cause the
WARN_ON_ONCE() in that function to be triggered.

This change also has disk_clear_events() not go through a workqueue.
Since we have to wait for the work to complete, we should just call the
function directly.  Also, since this work cannot be put on a freezable
workqueue, it will have to contend with increased demand, so calling the
function directly avoids this.

[[email protected]: fix spello in comment]
Signed-off-by: Derek Basehore <[email protected]>
Cc: Mandeep Singh Baines <[email protected]>
Cc: Jens Axboe <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Jens Axboe <[email protected]>
  • Loading branch information
dbasehore authored and axboe committed Dec 19, 2012
1 parent aea24a8 commit 12c2bdb
Showing 1 changed file with 30 additions and 15 deletions.
45 changes: 30 additions & 15 deletions block/genhd.c
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ static DEFINE_IDR(ext_devt_idr);

static struct device_type disk_type;

static void disk_check_events(struct disk_events *ev,
unsigned int *clearing_ptr);
static void disk_alloc_events(struct gendisk *disk);
static void disk_add_events(struct gendisk *disk);
static void disk_del_events(struct gendisk *disk);
Expand Down Expand Up @@ -1549,6 +1551,7 @@ unsigned int disk_clear_events(struct gendisk *disk, unsigned int mask)
const struct block_device_operations *bdops = disk->fops;
struct disk_events *ev = disk->ev;
unsigned int pending;
unsigned int clearing = mask;

if (!ev) {
/* for drivers still using the old ->media_changed method */
Expand All @@ -1558,41 +1561,53 @@ unsigned int disk_clear_events(struct gendisk *disk, unsigned int mask)
return 0;
}

/* tell the workfn about the events being cleared */
disk_block_events(disk);

/*
* store the union of mask and ev->clearing on the stack so that the
* race with disk_flush_events does not cause ambiguity (ev->clearing
* can still be modified even if events are blocked).
*/
spin_lock_irq(&ev->lock);
ev->clearing |= mask;
clearing |= ev->clearing;
ev->clearing = 0;
spin_unlock_irq(&ev->lock);

/* uncondtionally schedule event check and wait for it to finish */
disk_block_events(disk);
disk_check_events(ev, &clearing);
/*
* We need to put the work on system_nrt_wq here since there is a
* deadlock that happens while probing a usb device while suspending. If
* we put work on a freezable workqueue here, a usb probe will wait here
* until the workqueue is unfrozen during suspend. Since suspend waits
* on all probes to complete, we have a deadlock
* if ev->clearing is not 0, the disk_flush_events got called in the
* middle of this function, so we want to run the workfn without delay.
*/
queue_delayed_work(system_nrt_wq, &ev->dwork, 0);
flush_delayed_work(&ev->dwork);
__disk_unblock_events(disk, false);
__disk_unblock_events(disk, ev->clearing ? true : false);

/* then, fetch and clear pending events */
spin_lock_irq(&ev->lock);
WARN_ON_ONCE(ev->clearing & mask); /* cleared by workfn */
pending = ev->pending & mask;
ev->pending &= ~mask;
spin_unlock_irq(&ev->lock);
WARN_ON_ONCE(clearing & mask);

return pending;
}

/*
* Separate this part out so that a different pointer for clearing_ptr can be
* passed in for disk_clear_events.
*/
static void disk_events_workfn(struct work_struct *work)
{
struct delayed_work *dwork = to_delayed_work(work);
struct disk_events *ev = container_of(dwork, struct disk_events, dwork);

disk_check_events(ev, &ev->clearing);
}

static void disk_check_events(struct disk_events *ev,
unsigned int *clearing_ptr)
{
struct gendisk *disk = ev->disk;
char *envp[ARRAY_SIZE(disk_uevents) + 1] = { };
unsigned int clearing = ev->clearing;
unsigned int clearing = *clearing_ptr;
unsigned int events;
unsigned long intv;
int nr_events = 0, i;
Expand All @@ -1605,7 +1620,7 @@ static void disk_events_workfn(struct work_struct *work)

events &= ~ev->pending;
ev->pending |= events;
ev->clearing &= ~clearing;
*clearing_ptr &= ~clearing;

intv = disk_events_poll_jiffies(disk);
if (!ev->block && intv)
Expand Down

0 comments on commit 12c2bdb

Please sign in to comment.