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

[Bart Van Assche <bvanassche@acm.org>]

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.

At the time I wrote this patch I think there was a caller that invoked 
scsi_device_set_state() with the host lock held. Hence my choice for the 
host lock. However, I can't find that caller anymore. So the suggestion 
to use the device lock instead makes sense to me. I'll double check 
whether there are no callers of that function that already hold the 
device lock.

Bart.