[PATCH] scsi_mid_low_api.txt recommend resid usage

[PATCH] scsi_mid_low_api.txt recommend resid usage

[Douglas Gilbert]

[-- Attachment #1: Type: text/plain, Size: 2131 bytes --]

As discussed in a thread on this list titled:
    "RFC: short reads on block devices"
this patch adds recommendations for LLDs to set resid
when there might be uncertainty about how much data
has been returned by a device.

This patch inline and attached] is against scsi-misc-2.6.git


Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>


diff --git a/Documentation/scsi/scsi_mid_low_api.txt 
b/Documentation/scsi/scsi_mid_low_api.txt
index df322c1..7bcdea7 100644
--- a/Documentation/scsi/scsi_mid_low_api.txt
+++ b/Documentation/scsi/scsi_mid_low_api.txt
@@ -1343,7 +1343,7 @@ Members of interest:
                     underruns (overruns should be rare). If possible an LLD
                     should set 'resid' prior to invoking 'done'. The most
                     interesting case is data transfers from a SCSI target
-                   device device (i.e. READs) that underrun.
+                   device (e.g. READs) that underrun.
      underflow    - LLD should place (DID_ERROR << 16) in 'result' if
                     actual number of bytes transferred is less than this
                     figure. Not many LLDs implement this check and some that
@@ -1351,6 +1351,18 @@ Members of interest:
                     report a DID_ERROR. Better for an LLD to implement
                     'resid'.

+It is recommended that a LLD set 'resid' on data transfers from a SCSI
+target device (e.g. READs). It is especially important that 'resid' is set
+when such data transfers have sense keys of MEDIUM ERROR and HARDWARE ERROR
+(and possibly RECOVERED ERROR). In these cases if a LLD is in doubt how much
+data has been received then the safest approach is to indicate no bytes have
+been received. For example: to indicate that no valid data has been received
+a LLD might use these helpers:
+    scsi_set_resid(SCpnt, scsi_bufflen(SCpnt));
+where 'SCpnt' is a pointer to a scsi_cmnd object. To indicate only three 512
+bytes blocks has been received 'resid' could be set like this:
+    scsi_set_resid(SCpnt, scsi_bufflen(SCpnt) - (3 * 512));
+
  The scsi_cmnd structure is defined in include/scsi/scsi_cmnd.h



[-- Attachment #2: scsi_mid_low_api_resid.patch --]
[-- Type: text/x-patch, Size: 1782 bytes --]

diff --git a/Documentation/scsi/scsi_mid_low_api.txt b/Documentation/scsi/scsi_mid_low_api.txt
index df322c1..7bcdea7 100644
--- a/Documentation/scsi/scsi_mid_low_api.txt
+++ b/Documentation/scsi/scsi_mid_low_api.txt
@@ -1343,7 +1343,7 @@ Members of interest:
                    underruns (overruns should be rare). If possible an LLD
                    should set 'resid' prior to invoking 'done'. The most
                    interesting case is data transfers from a SCSI target
-                   device device (i.e. READs) that underrun. 
+                   device (e.g. READs) that underrun. 
     underflow    - LLD should place (DID_ERROR << 16) in 'result' if
                    actual number of bytes transferred is less than this
                    figure. Not many LLDs implement this check and some that
@@ -1351,6 +1351,18 @@ Members of interest:
                    report a DID_ERROR. Better for an LLD to implement
                    'resid'.
 
+It is recommended that a LLD set 'resid' on data transfers from a SCSI
+target device (e.g. READs). It is especially important that 'resid' is set
+when such data transfers have sense keys of MEDIUM ERROR and HARDWARE ERROR
+(and possibly RECOVERED ERROR). In these cases if a LLD is in doubt how much
+data has been received then the safest approach is to indicate no bytes have
+been received. For example: to indicate that no valid data has been received
+a LLD might use these helpers:
+    scsi_set_resid(SCpnt, scsi_bufflen(SCpnt));
+where 'SCpnt' is a pointer to a scsi_cmnd object. To indicate only three 512
+bytes blocks has been received 'resid' could be set like this:
+    scsi_set_resid(SCpnt, scsi_bufflen(SCpnt) - (3 * 512));
+
 The scsi_cmnd structure is defined in include/scsi/scsi_cmnd.h
 
 

Re: [PATCH] scsi_mid_low_api.txt recommend resid usage

[Randy Dunlap]

On Thu, 23 Dec 2010 21:40:37 -0500 Douglas Gilbert wrote:

> As discussed in a thread on this list titled:
>     "RFC: short reads on block devices"
> this patch adds recommendations for LLDs to set resid
> when there might be uncertainty about how much data
> has been returned by a device.
> 
> This patch inline and attached] is against scsi-misc-2.6.git
> 
> 
> Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>
> 
> 
> diff --git a/Documentation/scsi/scsi_mid_low_api.txt 
> b/Documentation/scsi/scsi_mid_low_api.txt
> index df322c1..7bcdea7 100644
> --- a/Documentation/scsi/scsi_mid_low_api.txt
> +++ b/Documentation/scsi/scsi_mid_low_api.txt

> @@ -1351,6 +1351,18 @@ Members of interest:
>                      report a DID_ERROR. Better for an LLD to implement
>                      'resid'.
> 
> +It is recommended that a LLD set 'resid' on data transfers from a SCSI
> +target device (e.g. READs). It is especially important that 'resid' is set
> +when such data transfers have sense keys of MEDIUM ERROR and HARDWARE ERROR
> +(and possibly RECOVERED ERROR). In these cases if a LLD is in doubt how much
> +data has been received then the safest approach is to indicate no bytes have
> +been received. For example: to indicate that no valid data has been received
> +a LLD might use these helpers:
> +    scsi_set_resid(SCpnt, scsi_bufflen(SCpnt));
> +where 'SCpnt' is a pointer to a scsi_cmnd object. To indicate only three 512
> +bytes blocks has been received 'resid' could be set like this:
> +    scsi_set_resid(SCpnt, scsi_bufflen(SCpnt) - (3 * 512));
> +
>   The scsi_cmnd structure is defined in include/scsi/scsi_cmnd.h


Hi Doug,

Using "an LLD" instead of "a LLD" would be more consistent with the rest
of this txt file (and read better to me).


---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
desserts:  http://www.xenotime.net/linux/recipes/

Re: [PATCH] scsi_mid_low_api.txt recommend resid usage

[Douglas Gilbert]

On 10-12-24 12:46 PM, Randy Dunlap wrote:
> On Thu, 23 Dec 2010 21:40:37 -0500 Douglas Gilbert wrote:
>
>> As discussed in a thread on this list titled:
>>      "RFC: short reads on block devices"
>> this patch adds recommendations for LLDs to set resid
>> when there might be uncertainty about how much data
>> has been returned by a device.
>>
>> This patch inline and attached] is against scsi-misc-2.6.git
>>
>>
>> Signed-off-by: Douglas Gilbert<dgilbert@interlog.com>
>>
>>
>> diff --git a/Documentation/scsi/scsi_mid_low_api.txt
>> b/Documentation/scsi/scsi_mid_low_api.txt
>> index df322c1..7bcdea7 100644
>> --- a/Documentation/scsi/scsi_mid_low_api.txt
>> +++ b/Documentation/scsi/scsi_mid_low_api.txt
>
>> @@ -1351,6 +1351,18 @@ Members of interest:
>>                       report a DID_ERROR. Better for an LLD to implement
>>                       'resid'.
>>
>> +It is recommended that a LLD set 'resid' on data transfers from a SCSI
>> +target device (e.g. READs). It is especially important that 'resid' is set
>> +when such data transfers have sense keys of MEDIUM ERROR and HARDWARE ERROR
>> +(and possibly RECOVERED ERROR). In these cases if a LLD is in doubt how much
>> +data has been received then the safest approach is to indicate no bytes have
>> +been received. For example: to indicate that no valid data has been received
>> +a LLD might use these helpers:
>> +    scsi_set_resid(SCpnt, scsi_bufflen(SCpnt));
>> +where 'SCpnt' is a pointer to a scsi_cmnd object. To indicate only three 512
>> +bytes blocks has been received 'resid' could be set like this:
>> +    scsi_set_resid(SCpnt, scsi_bufflen(SCpnt) - (3 * 512));
>> +
>>    The scsi_cmnd structure is defined in include/scsi/scsi_cmnd.h
>
>
> Hi Doug,
>
> Using "an LLD" instead of "a LLD" would be more consistent with the rest
> of this txt file (and read better to me).

Randy,
There were other examples of "a LLD" which I
cleaned up as well.

Doug Gilbert


diff --git a/Documentation/scsi/scsi_mid_low_api.txt 
b/Documentation/scsi/scsi_mid_low_api.txt
index df322c1..10d41ba 100644
--- a/Documentation/scsi/scsi_mid_low_api.txt
+++ b/Documentation/scsi/scsi_mid_low_api.txt
@@ -185,7 +185,7 @@ scsi_remove_host() ---------+
  scsi_host_put()
  ------------------------------------------------------------

-It may be useful for a LLD to keep track of struct Scsi_Host instances
+It may be useful for an LLD to keep track of struct Scsi_Host instances
  (a pointer is returned by scsi_host_alloc()). Such instances are "owned"
  by the mid-level.  struct Scsi_Host instances are freed from
  scsi_host_put() when the reference count hits zero.
@@ -749,7 +749,7 @@ void scsi_unblock_requests(struct Scsi_Host * shost)
   *
   *      Notes: Should not be invoked if the "hotplug initialization
   *      model" is being used. Called internally by exit_this_scsi_driver()
- *      in the "passive initialization model". Hence a LLD has no need to
+ *      in the "passive initialization model". Hence an LLD has no need to
   *      call this function directly.
   *
   *      Defined in: drivers/scsi/hosts.c .
@@ -1343,7 +1343,7 @@ Members of interest:
                     underruns (overruns should be rare). If possible an LLD
                     should set 'resid' prior to invoking 'done'. The most
                     interesting case is data transfers from a SCSI target
-                   device device (i.e. READs) that underrun.
+                   device (e.g. READs) that underrun.
      underflow    - LLD should place (DID_ERROR << 16) in 'result' if
                     actual number of bytes transferred is less than this
                     figure. Not many LLDs implement this check and some that
@@ -1351,6 +1351,18 @@ Members of interest:
                     report a DID_ERROR. Better for an LLD to implement
                     'resid'.

+It is recommended that an LLD set 'resid' on data transfers from a SCSI
+target device (e.g. READs). It is especially important that 'resid' is set
+when such data transfers have sense keys of MEDIUM ERROR and HARDWARE ERROR
+(and possibly RECOVERED ERROR). In these cases if an LLD is in doubt how much
+data has been received then the safest approach is to indicate no bytes have
+been received. For example: to indicate that no valid data has been received
+an LLD might use these helpers:
+    scsi_set_resid(SCpnt, scsi_bufflen(SCpnt));
+where 'SCpnt' is a pointer to a scsi_cmnd object. To indicate only three 512
+bytes blocks has been received 'resid' could be set like this:
+    scsi_set_resid(SCpnt, scsi_bufflen(SCpnt) - (3 * 512));
+
  The scsi_cmnd structure is defined in include/scsi/scsi_cmnd.h

Re: [PATCH] scsi_mid_low_api.txt recommend resid usage

[Randy Dunlap]

On Fri, 24 Dec 2010 15:17:40 -0500 Douglas Gilbert wrote:

> > Hi Doug,
> >
> > Using "an LLD" instead of "a LLD" would be more consistent with the rest
> > of this txt file (and read better to me).
> 
> Randy,
> There were other examples of "a LLD" which I
> cleaned up as well.

Ack.  Thanks.

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
desserts:  http://www.xenotime.net/linux/recipes/

Re: [PATCH] scsi_mid_low_api.txt recommend resid usage

[Douglas Gilbert]

Randy,
Add some extra words to define what is expected if
a transport can do out-of-order data delivery
(e.g. FCP_RESID, theoretically, is not precisely
the resid we want).

diff --git a/Documentation/scsi/scsi_mid_low_api.txt 
b/Documentation/scsi/scsi_mid_low_api.txt
index df322c1..d998af0 100644
--- a/Documentation/scsi/scsi_mid_low_api.txt
+++ b/Documentation/scsi/scsi_mid_low_api.txt
@@ -185,7 +185,7 @@ scsi_remove_host() ---------+
  scsi_host_put()
  ------------------------------------------------------------

-It may be useful for a LLD to keep track of struct Scsi_Host instances
+It may be useful for an LLD to keep track of struct Scsi_Host instances
  (a pointer is returned by scsi_host_alloc()). Such instances are "owned"
  by the mid-level.  struct Scsi_Host instances are freed from
  scsi_host_put() when the reference count hits zero.
@@ -749,7 +749,7 @@ void scsi_unblock_requests(struct Scsi_Host * shost)
   *
   *      Notes: Should not be invoked if the "hotplug initialization
   *      model" is being used. Called internally by exit_this_scsi_driver()
- *      in the "passive initialization model". Hence a LLD has no need to
+ *      in the "passive initialization model". Hence an LLD has no need to
   *      call this function directly.
   *
   *      Defined in: drivers/scsi/hosts.c .
@@ -1343,7 +1343,7 @@ Members of interest:
                     underruns (overruns should be rare). If possible an LLD
                     should set 'resid' prior to invoking 'done'. The most
                     interesting case is data transfers from a SCSI target
-                   device device (i.e. READs) that underrun.
+                   device (e.g. READs) that underrun.
      underflow    - LLD should place (DID_ERROR << 16) in 'result' if
                     actual number of bytes transferred is less than this
                     figure. Not many LLDs implement this check and some that
@@ -1351,6 +1351,22 @@ Members of interest:
                     report a DID_ERROR. Better for an LLD to implement
                     'resid'.

+It is recommended that an LLD set 'resid' on data transfers from a SCSI
+target device (e.g. READs). It is especially important that 'resid' is set
+when such data transfers have sense keys of MEDIUM ERROR and HARDWARE ERROR
+(and possibly RECOVERED ERROR). In these cases if an LLD is in doubt how much
+data has been received then the safest approach is to indicate no bytes have
+been received. For example: to indicate that no valid data has been received
+an LLD might use these helpers:
+    scsi_set_resid(SCpnt, scsi_bufflen(SCpnt));
+where 'SCpnt' is a pointer to a scsi_cmnd object. To indicate only three 512
+bytes blocks has been received 'resid' could be set like this:
+    scsi_set_resid(SCpnt, scsi_bufflen(SCpnt) - (3 * 512));
+
+For transports that can do "out of order" delivery of data from a SCSI target
+device, the 'resid' should reflect the number of valid, consecutive bytes
+from the start of the buffer.
+
  The scsi_cmnd structure is defined in include/scsi/scsi_cmnd.h


Doug Gilbert


On 10-12-24 12:46 PM, Randy Dunlap wrote:
> On Thu, 23 Dec 2010 21:40:37 -0500 Douglas Gilbert wrote:
>
>> As discussed in a thread on this list titled:
>>      "RFC: short reads on block devices"
>> this patch adds recommendations for LLDs to set resid
>> when there might be uncertainty about how much data
>> has been returned by a device.
>>
>> This patch inline and attached] is against scsi-misc-2.6.git
>>
>>
>> Signed-off-by: Douglas Gilbert<dgilbert@interlog.com>
>>
>>
>> diff --git a/Documentation/scsi/scsi_mid_low_api.txt
>> b/Documentation/scsi/scsi_mid_low_api.txt
>> index df322c1..7bcdea7 100644
>> --- a/Documentation/scsi/scsi_mid_low_api.txt
>> +++ b/Documentation/scsi/scsi_mid_low_api.txt
>
>> @@ -1351,6 +1351,18 @@ Members of interest:
>>                       report a DID_ERROR. Better for an LLD to implement
>>                       'resid'.
>>
>> +It is recommended that a LLD set 'resid' on data transfers from a SCSI
>> +target device (e.g. READs). It is especially important that 'resid' is set
>> +when such data transfers have sense keys of MEDIUM ERROR and HARDWARE ERROR
>> +(and possibly RECOVERED ERROR). In these cases if a LLD is in doubt how much
>> +data has been received then the safest approach is to indicate no bytes have
>> +been received. For example: to indicate that no valid data has been received
>> +a LLD might use these helpers:
>> +    scsi_set_resid(SCpnt, scsi_bufflen(SCpnt));
>> +where 'SCpnt' is a pointer to a scsi_cmnd object. To indicate only three 512
>> +bytes blocks has been received 'resid' could be set like this:
>> +    scsi_set_resid(SCpnt, scsi_bufflen(SCpnt) - (3 * 512));
>> +
>>    The scsi_cmnd structure is defined in include/scsi/scsi_cmnd.h
>
>
> Hi Doug,
>
> Using "an LLD" instead of "a LLD" would be more consistent with the rest
> of this txt file (and read better to me).
>
>
> ---
> ~Randy
> *** Remember to use Documentation/SubmitChecklist when testing your code ***
> desserts:  http://www.xenotime.net/linux/recipes/
>

Re: [PATCH] scsi_mid_low_api.txt recommend resid usage

[Randy Dunlap]

On Thu, 30 Dec 2010 11:08:20 -0500 Douglas Gilbert wrote:

> Randy,
> Add some extra words to define what is expected if
> a transport can do out-of-order data delivery
> (e.g. FCP_RESID, theoretically, is not precisely
> the resid we want).

Can James use your previous:
>> Signed-off-by: Douglas Gilbert<dgilbert@interlog.com>

for this patch?

Acked-by: Randy Dunlap <rdunlap@xenotime.net>

Thanks.


> diff --git a/Documentation/scsi/scsi_mid_low_api.txt 
> b/Documentation/scsi/scsi_mid_low_api.txt
> index df322c1..d998af0 100644
> --- a/Documentation/scsi/scsi_mid_low_api.txt
> +++ b/Documentation/scsi/scsi_mid_low_api.txt
> @@ -185,7 +185,7 @@ scsi_remove_host() ---------+
>   scsi_host_put()
>   ------------------------------------------------------------
> 
> -It may be useful for a LLD to keep track of struct Scsi_Host instances
> +It may be useful for an LLD to keep track of struct Scsi_Host instances
>   (a pointer is returned by scsi_host_alloc()). Such instances are "owned"
>   by the mid-level.  struct Scsi_Host instances are freed from
>   scsi_host_put() when the reference count hits zero.
> @@ -749,7 +749,7 @@ void scsi_unblock_requests(struct Scsi_Host * shost)
>    *
>    *      Notes: Should not be invoked if the "hotplug initialization
>    *      model" is being used. Called internally by exit_this_scsi_driver()
> - *      in the "passive initialization model". Hence a LLD has no need to
> + *      in the "passive initialization model". Hence an LLD has no need to
>    *      call this function directly.
>    *
>    *      Defined in: drivers/scsi/hosts.c .
> @@ -1343,7 +1343,7 @@ Members of interest:
>                      underruns (overruns should be rare). If possible an LLD
>                      should set 'resid' prior to invoking 'done'. The most
>                      interesting case is data transfers from a SCSI target
> -                   device device (i.e. READs) that underrun.
> +                   device (e.g. READs) that underrun.
>       underflow    - LLD should place (DID_ERROR << 16) in 'result' if
>                      actual number of bytes transferred is less than this
>                      figure. Not many LLDs implement this check and some that
> @@ -1351,6 +1351,22 @@ Members of interest:
>                      report a DID_ERROR. Better for an LLD to implement
>                      'resid'.
> 
> +It is recommended that an LLD set 'resid' on data transfers from a SCSI
> +target device (e.g. READs). It is especially important that 'resid' is set
> +when such data transfers have sense keys of MEDIUM ERROR and HARDWARE ERROR
> +(and possibly RECOVERED ERROR). In these cases if an LLD is in doubt how much
> +data has been received then the safest approach is to indicate no bytes have
> +been received. For example: to indicate that no valid data has been received
> +an LLD might use these helpers:
> +    scsi_set_resid(SCpnt, scsi_bufflen(SCpnt));
> +where 'SCpnt' is a pointer to a scsi_cmnd object. To indicate only three 512
> +bytes blocks has been received 'resid' could be set like this:
> +    scsi_set_resid(SCpnt, scsi_bufflen(SCpnt) - (3 * 512));
> +
> +For transports that can do "out of order" delivery of data from a SCSI target
> +device, the 'resid' should reflect the number of valid, consecutive bytes
> +from the start of the buffer.
> +
>   The scsi_cmnd structure is defined in include/scsi/scsi_cmnd.h
> 
> 
> Doug Gilbert
> 
> 
> On 10-12-24 12:46 PM, Randy Dunlap wrote:
> > On Thu, 23 Dec 2010 21:40:37 -0500 Douglas Gilbert wrote:
> >
> >> As discussed in a thread on this list titled:
> >>      "RFC: short reads on block devices"
> >> this patch adds recommendations for LLDs to set resid
> >> when there might be uncertainty about how much data
> >> has been returned by a device.
> >>
> >> This patch inline and attached] is against scsi-misc-2.6.git
> >>
> >>
> >> Signed-off-by: Douglas Gilbert<dgilbert@interlog.com>
> >>
> >>
> >> diff --git a/Documentation/scsi/scsi_mid_low_api.txt
> >> b/Documentation/scsi/scsi_mid_low_api.txt
> >> index df322c1..7bcdea7 100644
> >> --- a/Documentation/scsi/scsi_mid_low_api.txt
> >> +++ b/Documentation/scsi/scsi_mid_low_api.txt
> >
> >> @@ -1351,6 +1351,18 @@ Members of interest:
> >>                       report a DID_ERROR. Better for an LLD to implement
> >>                       'resid'.
> >>
> >> +It is recommended that a LLD set 'resid' on data transfers from a SCSI
> >> +target device (e.g. READs). It is especially important that 'resid' is set
> >> +when such data transfers have sense keys of MEDIUM ERROR and HARDWARE ERROR
> >> +(and possibly RECOVERED ERROR). In these cases if a LLD is in doubt how much
> >> +data has been received then the safest approach is to indicate no bytes have
> >> +been received. For example: to indicate that no valid data has been received
> >> +a LLD might use these helpers:
> >> +    scsi_set_resid(SCpnt, scsi_bufflen(SCpnt));
> >> +where 'SCpnt' is a pointer to a scsi_cmnd object. To indicate only three 512
> >> +bytes blocks has been received 'resid' could be set like this:
> >> +    scsi_set_resid(SCpnt, scsi_bufflen(SCpnt) - (3 * 512));
> >> +
> >>    The scsi_cmnd structure is defined in include/scsi/scsi_cmnd.h
> >
> >
> > Hi Doug,
> >
> > Using "an LLD" instead of "a LLD" would be more consistent with the rest
> > of this txt file (and read better to me).


---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
desserts:  http://www.xenotime.net/linux/recipes/