On Fri, 16 Mar 2012, Oliver Neukum wrote:
> Hi Alan,
>
> looking at this code:
>
> 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;
>
> if (!ev) {
> /* for drivers still using the old ->media_changed method */
> if ((mask & DISK_EVENT_MEDIA_CHANGE) &&
> bdops->media_changed && bdops->media_changed(disk))
> return DISK_EVENT_MEDIA_CHANGE;
> return 0;
> }
>
> /* tell the workfn about the events being cleared */
> spin_lock_irq(&ev->lock);
> ev->clearing |= mask;
> spin_unlock_irq(&ev->lock);
>
> /* uncondtionally schedule event check and wait for it to finish */
> disk_block_events(disk);
> queue_delayed_work(system_nrt_freezable_wq, &ev->dwork, 0);
>
> <-- window for race with freezer
>
> flush_delayed_work(&ev->dwork);
> __disk_unblock_events(disk, 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);
>
> return pending;
> }
>
> What happens if the freezer starts during the window? It seems to me
> that the task flushing work would block and could not be frozen.
You're right. I didn't check for flush_delayed_work() calls when
writing the patch.
I guess it would be okay if we first do cancel_delayed_work_sync() and
there was some way to guarantee that ev->dwork would not be added to
the freezable workqueue until this routine was finished. Does
disk_block_events() do that for us already?
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
