Re: [PATCH v12 5/6] Avoid that scsi_device_set_state() triggers a race

[James Bottomley <jbottomley@parallels.com>]

On Mon, 2013-07-01 at 17:17 +0200, Bart Van Assche wrote:
> On 07/01/13 16:49, James Bottomley wrote:
> > On Thu, 2013-06-27 at 16:56 +0200, Bart Van Assche wrote:
> >> Make concurrent invocations of scsi_device_set_state() safe.
> >
> > Firstly, I don't understand from this where you think the races are.
> > Secondly, shouldn't this be the device lock? and thirdly, if we accept
> > that locking is required, encapsulate it in the function: Having the
> > callers manage locking is asking for trouble.  The latter may require a
> > new lock for the state to avoid entanglement.
> 
> Today there is no guarantee that scsi_device_set_state() calls are 
> serialized, so two scsi_device_set_state() invocations may be in 
> progress concurrently. It is e.g. possible that both calls report 
> "device state has been changed successfully" to their callers although 
> only one of these two state changes will be effective due to the race.

We could say the above about a significant fraction of the functions in
the kernel; it's not a reason to add fine grained locking to them all.

I want to know what the actual races you're trying to fix are; what
causes them and, in particular, is adding yet another fine grained lock
going to mitigate them effectively or should they be mediated in a
different way.

James