Re: [LSF/MM TOPIC] SCSI referrals support

[Hannes Reinecke <hare@suse.de>]

On 01/23/2011 12:25 PM, Boaz Harrosh wrote:
> On 01/20/2011 06:36 PM, James Bottomley wrote:
>> On Thu, 2011-01-20 at 17:14 +0100, Douglas Gilbert wrote:
>>> On 11-01-20 05:00 PM, Hannes Reinecke wrote:
>>>> Agenda topic proposal:
>>>>
>>>> SCSI referrals support has already been discussed at last year's LSF
>>>> conference. However, the solution proposed there would not support
>>>> failover and would require quite a lot of changes to multipathing.
>>>>
>>>> To enable failover it might be an idea to handle the LUN directly in
>>>> multipathing. This would require eg:
>>>> - request splitting
>>>> - I/O alignment handling
>>>> - SCSI unit attention handling
>>>>
>>>> I would be giving a short overview/presentation of the current
>>>> state of the art, the shortcomings on the original proposal,
>>>> and would like to invite a discussion on how to best support
>>>> SCSI referrals.
>>>
>>> IMO a worrying aspect of the changes associated with SCSI
>>> referrals is that sense data can now be returned with any
>>> SCSI status (i.e. not just CHECK CONDITION). How well would
>>> the SCSI subsystem cope with that? I know that the sg driver
>>> (and probably bsg) would need changes, as would my libsgutils
>>> library used by sg3_utils.
>>
>> Well, not necessarily.  It scuppers plans to try to use the sense buffer
>> more efficiently, but right at the moment, we can receive sense for any
>> command.  We only actually look at it on a check condition return, but
>> that's easily updated.
>>
>> As I read the standard, referrals sense data on successfully completed
>> commands is only really relevant to mp implementations, so it looks like
>> the mid-layer should ignore it anyway (as it would now) but provide a
>> hook for the device handlers to see it if they wanted.
>>
>> James
>>
> Something like:
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index eafeeda..454e562 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -722,20 +722,23 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)
>  			sense_deferred = scsi_sense_is_deferred(&sshdr);
>  	}
>  
> +	if (req->sense) {
> +		/*
> +		 * SG_IO wants current and deferred errors
> +		 */
> +		int len = 8 + cmd->sense_buffer[7];
> +
> +		if (unlikely(len)) {
> +			if (len > SCSI_SENSE_BUFFERSIZE)
> +				len = SCSI_SENSE_BUFFERSIZE;
> +			memcpy(req->sense, cmd->sense_buffer,  len);
> +			req->sense_len = len;
> +		}
> +	}
> +
>  	if (req->cmd_type == REQ_TYPE_BLOCK_PC) { /* SG_IO ioctl from block level */
>  		req->errors = result;
>  		if (result) {
> -			if (sense_valid && req->sense) {
> -				/*
> -				 * SG_IO wants current and deferred errors
> -				 */
> -				int len = 8 + cmd->sense_buffer[7];
> -
> -				if (len > SCSI_SENSE_BUFFERSIZE)
> -					len = SCSI_SENSE_BUFFERSIZE;
> -				memcpy(req->sense, cmd->sense_buffer,  len);
> -				req->sense_len = len;
> -			}
>  			if (!sense_deferred)
>  				error = -EIO;
>  		}
> 
> Then an interested user can just look at req->sense_len. The reset of the stack will
> ignore all that because it looks for status/error-code.
> 
Ah, if it were so easy. Currently sense codes have two problems:

- They are limited to 96 bytes. Anything larger than that will just
  be discarded (or crash with your patch above :-)
- They inherit the same lifetime than the scsi command. But for any
  decent handling you really need to push them into some
  asynchronous context as you might well be within an interrupt
  handler here.

I'm currently working on a handling framework using relayfs
(basically blktrace for SCSI Unit Attention); I can be doing a short
presentation at LSF if requested.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html