Re: [PATCH v12 6/6] Avoid re-enabling I/O after the transport became offline

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

On 07/01/13 10:27, Hannes Reinecke wrote:
> On 06/27/2013 04:57 PM, Bart Van Assche wrote:
>> diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
>> index dfbaa34..666b741 100644
>> --- a/drivers/scsi/scsi_sysfs.c
>> +++ b/drivers/scsi/scsi_sysfs.c
>> @@ -959,14 +959,16 @@ void __scsi_remove_device(struct scsi_device *sdev)
>>   {
>>   	struct Scsi_Host *shost = sdev->host;
>>   	struct device *dev = &sdev->sdev_gendev;
>> +	enum scsi_device_state sdev_state;
>>   	int res;
>>   
>>   	if (sdev->is_visible) {
>>   		spin_lock_irq(shost->host_lock);
>> +		sdev_state = sdev->sdev_state;
>>   		res = scsi_device_set_state(sdev, SDEV_CANCEL);
>>   		spin_unlock_irq(shost->host_lock);
>>   
>> -		if (res != 0)
>> +		if (res != 0 && sdev_state != SDEV_TRANSPORT_OFFLINE)
>>   			return;
>>   
>>   		bsg_unregister_queue(sdev->request_queue);
>>
> Hmm. This is really subtle. Do you mind adding inserting a comment
> here on why this is required?

How about inserting the following comment just above the last if-statement
in the code cited above ?

		/*
		 * The transition from SDEV_TRANSPORT_OFFLINE into SDEV_CANCEL
		 * is not allowed since this transition would re-enable I/O. If
		 * the device state was already SDEV_TRANSPORT_OFFLINE,
		 * proceed with device removal.
		 */

Bart.