scsi_mid_low_api.txt recommend resid usage

PATCH In review Douglas Gilbert 2 participants 6 messages Dec 24, 2010 – Dec 30, 2010

Subsystems (2)

Documentation SCSI

lore

Share & tools

lore

Douglas Gilbert

author

2010-12-24 02:40 UTC #0 lore raw

PATCH 1/1 scsi_mid_low_api.txt recommend resid usage

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 <[email protected]>

Documentation/scsi/scsi_mid_low_api.txt +13 -1

index df322c1..7bcdea7 100644
@@ -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

Documentation/scsi/scsi_mid_low_api.txt +13 -1

index df322c1..7bcdea7 100644
@@ -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

Randy Dunlap

2010-12-24 17:46 UTC #1 lore raw

Quoted 25 lines · 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 <[email protected]>
Documentation/scsi/scsi_mid_low_api.txt +3 -0
index df322c1..7bcdea7 100644
@@ -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

Documentation/scsi/scsi_mid_low_api.txt Members of interest: · line 1366

Quoted 10 lines

Documentation/scsi/scsi_mid_low_api.txt +9 -0

+(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/

Douglas Gilbert

author

2010-12-24 20:17 UTC #2 lore raw

Quoted 41 lines · Randy Dunlap

Quoted 25 lines · 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<[email protected]>
Documentation/scsi/scsi_mid_low_api.txt +3 -0
index df322c1..7bcdea7 100644
@@ -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
Documentation/scsi/scsi_mid_low_api.txt Members of interest: · line 1366
Quoted 10 lines
Documentation/scsi/scsi_mid_low_api.txt +9 -0
+(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,

Quoted 2 lines

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

Documentation/scsi/scsi_mid_low_api.txt +15 -3

index df322c1..10d41ba 100644
@@ -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

Randy Dunlap

2010-12-24 20:36 UTC #3 lore raw

Quoted 8 lines · On Fri, 24 Dec 2010 15:17:40 -0500 Douglas Gilbert wrote:

Quoted 4 lines
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/

Douglas Gilbert

author

2010-12-30 16:08 UTC #4 lore raw

In reply to Randy Dunlap

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).

Documentation/scsi/scsi_mid_low_api.txt +19 -3

index df322c1..d998af0 100644
@@ -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

Quoted 50 lines · Randy Dunlap

Quoted 25 lines · 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<[email protected]>
Documentation/scsi/scsi_mid_low_api.txt +3 -0
index df322c1..7bcdea7 100644
@@ -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
Documentation/scsi/scsi_mid_low_api.txt Members of interest: · line 1366
Quoted 10 lines
Documentation/scsi/scsi_mid_low_api.txt +9 -0
+(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/

Randy Dunlap

Acked-by

2010-12-30 17:50 UTC #5 lore raw

Quoted 5 lines · 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:

Quoted 1 line

Quoted 1 line
Signed-off-by Douglas Gilbert<[email protected]>

for this patch?

Acked-by Randy Dunlap <[email protected]>

Thanks.

Quoted 58 lines

Documentation/scsi/scsi_mid_low_api.txt +19 -3

index df322c1..d998af0 100644
@@ -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

Quoted 44 lines

Quoted 43 lines · Randy Dunlap
Quoted 25 lines · 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<[email protected]>
Documentation/scsi/scsi_mid_low_api.txt +3 -0
index df322c1..7bcdea7 100644
@@ -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
Documentation/scsi/scsi_mid_low_api.txt Members of interest: · line 1366
Quoted 10 lines
Documentation/scsi/scsi_mid_low_api.txt +9 -0
+(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/