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

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

On 07/01/13 18:52, James Bottomley wrote:
> 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.

Since this patch is something I came up with as the result of source 
reading maybe I should defer this patch to a later time such that it 
doesn't slow down acceptance of this patch series.

Bart.