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