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.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to