Re: [PATCH] SCSI: Fix some locking issues

[James Bottomley <James.Bottomley@HansenPartnership.com>]

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:
> > > On Wed, Jul 02 2008, Elias Oltmanns wrote:
> > > > > The blk_plug_queue change looks reasonable ... however, blk_plug_queue
> > > > > itself looks like it might not entirely need the queue lock ... I need
> > > > > to investigate more closely.
> > > > 
> > > > Well, I rather think it does. We have to serialise access to the
> > > > unplug_timer and there is a call to __set_bit() which, as I understand,
> > > > requires the calling function to ensure atomicity.
> > > 
> > > 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.

James