Elias Oltmanns <eo@xxxxxxxxxxxxxx> wrote: > James Bottomley <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx> wrote: >> On Wed, 2008-07-02 at 20:45 +0200, Jens Axboe wrote: > >>> On Wed, Jul 02 2008, James Bottomley wrote: >> >>> > On Wed, 2008-07-02 at 13:50 +0200, Jens Axboe wrote: >>> > > Yep, blk_plug_device() needs to be called with the queue lock held. >>> > >>> > That's what the comment says ... but if you replaced the test_bit with >>> > an atomic operation then the rest of it does look to be in no need of >>> > serialisation ... unless there's something I missed? >>> >>> Indeed, but then you would have to use atomic bitops everywhere and that >>> is the bit we moved away from. >> >> Not necessarily ... only for QUEUE_FLAG_CLUSTER. That's really only in >> this one place and then the one in blk_remove_plug would have to become >> test_and_clear_bit. All the other places barring loop_unplug() are only >> tests (which don't affect the atomicity). >> >> It's just for SCSI the double spin lock followed by double spin unlock >> to get the locking right is kind of nasty ... I'm just wondering what >> the universe would look like if it were rendered unnecessary. > > We have to consider one more thing: Without the locking in > blk_plug_device(), the following sequence of events may occur: Actually, it's worse than that. Locking is required in order to make absolutely sure that the unplug_timer is active iff QUEUE_FLAG_PLUGGED is set. Admittedly, it seems *very* unlikely that blk_remove_plug() will complete before the call to mod_timer() in blk_plug_device() even though it has started only *after* a call to test_and_set_bit(). However, if such a thing would ever happen, it could have dire consequences. Regards, Elias -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
