On 12/06/2016 11:03 PM, Wei Fang wrote:
On 2016/12/7 12:40, Bart Van Assche wrote:
I am aware that commit 5c10e63c943b caused the behavior change. But that
doesn't mean that a fix has to undo the changes introduced by that
commit. We do not only want to make sure that the SCSI core works as
intended but also that the SCSI core code is as easy to comprehend as
reasonably possible. Adding "&& sdev->sdev_state != SDEV_RUNNING" in
scsi_internal_device_unblock() would require a long comment to explain
why that code has been added. I think modifying scsi_sysfs_add_sdev()
such that it does not unblock devices will result in code that is easier
to understand.
Agree that we should make the code easier to comprehend if possible :)
If we modify scsi_sysfs_add_sdev() as below:
...
if (scsi_device_created(sdev))
error = scsi_device_set_state(sdev, SDEV_RUNNING);
if (error)
error = scsi_device_set_state(sdev, SDEV_BLOCK);
...
there's a chance that the state will be changed to SDEV_RUNNING.
That's not what I meant. What I meant is to test the sdev state in
scsi_sysfs_add_sdev() before scsi_device_set_state() is called.
Actually I don't know quite well about the synchronization of
scsi_device_set_state(). There are so many cases it can be called
simultaneously, will the state become a unpredictable value, or this
is tolerated?
It's a known bug. Some time ago I posted a patch that serializes all
scsi_device_set_state() calls but I have not yet found it in the list
archives. However, that patch has not yet been merged.
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
