Hi, Bart,

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.

If a SCSI device is blocked after the check of the device's creating
and before being changed to SDEV_RUNNING state, the state will still
become SDEV_RUNNING. If we fix this problem in this way, we need
introduce a way to synchronize those code.

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?

Thanks,
Wei

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to