Re: [PATCH 03/11] block: add rq->resid_len

[Boaz Harrosh]

On 05/11/2009 05:59 PM, FUJITA Tomonori wrote:
> On Mon, 11 May 2009 14:31:41 +0300
> Boaz Harrosh <bharrosh@xxxxxxxxxxx> wrote:
> 
>>>>> diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
>>>>> index 3da02e4..6605ec9 100644
>>>>> --- a/drivers/scsi/libsas/sas_expander.c
>>>>> +++ b/drivers/scsi/libsas/sas_expander.c
>>>>> @@ -1936,12 +1936,8 @@ int sas_smp_handler(struct Scsi_Host *shost, struct sas_rphy *rphy,
>>>>>  			       bio_data(rsp->bio), rsp->data_len);
>>>>>  	if (ret > 0) {
>>>>>  		/* positive number is the untransferred residual */
>>>>> -		rsp->data_len = ret;
>>>>> -		req->data_len = 0;
>>>>> +		rsp->resid_len = ret;
>>>>>  		ret = 0;
>>>>> -	} else if (ret == 0) {
>>>>> -		rsp->data_len = 0;
>>>>> -		req->data_len = 0;
>>>>>  	}
>>>>>  
>>>>>  	return ret;
>>>> This is actually a bug fix, as well as a strait conversion
>>> Can you elaborate a bit about the bug fix part?
>>>
>> Nothing big really, just that before (according to the comment), the theoretical
>> negative case would be full-residual. and now it is zero (untouched).
>>
>> I know that in iscsi a negative residual is possible which means over-flow. That is:
>> the target had more data to give then the buffer had space for. (which is not an error at all)
> 
> Hmm, iSCSI? This code is for SAS management Protocol.
> 

I gave that as an example of what the scsi standard says about negative
residual count return from the target. If SAS as sepecific and different
meaning to negative residual, it should be noted and handled.

> smp_execute_task returns a negative value for some errors (ENOMEM, the
> hardware doesn't respond, etc). It's not related with 'transferred
> buffer length' at all.

This is a serious problem, and a violation of the block/scsi stacks. If so, then
in that case error/status should be set properly. And residual cleared.

Failing to return an error might theoretically be catastrophic. If what
you say is certain then previous behaviour is better then after this patch.

I'm not at all familiar with this code. Could you send a patch to
fix these things?

Thanks
Boaz
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/