| Message ID | 20230721160154.874010-2-bvanassche@acm.org |
|---|---|
| State | New |
| Headers | show |
| Series | Fix residual handling in two SCSI LLDs | expand |
Bart, > Because scsi_finish_command() subtracts the residual from the buffer > length, residual overflows must not be reported. Reflect this in the > SCSI documentation. See also commit 9237f04e12cc ("scsi: core: Fix > scsi_get/set_resid() interface") Applied to 6.6/scsi-staging, thanks!
On 8/16/23 02:58, Benjamin Block wrote: > On Fri, Jul 21, 2023 at 09:01:32AM -0700, Bart Van Assche wrote: >> Because scsi_finish_command() subtracts the residual from the buffer >> length, residual overflows must not be reported. Reflect this in the >> SCSI documentation. See also commit 9237f04e12cc ("scsi: core: Fix >> scsi_get/set_resid() interface") >> >> Cc: Damien Le Moal <dlemoal@kernel.org> >> Cc: Hannes Reinecke <hare@suse.de> >> Cc: Douglas Gilbert <dgilbert@interlog.com> >> Cc: stable@vger.kernel.org >> Signed-off-by: Bart Van Assche <bvanassche@acm.org> >> --- >> Documentation/scsi/scsi_mid_low_api.rst | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/Documentation/scsi/scsi_mid_low_api.rst b/Documentation/scsi/scsi_mid_low_api.rst >> index 6fa3a6279501..022198c51350 100644 >> --- a/Documentation/scsi/scsi_mid_low_api.rst >> +++ b/Documentation/scsi/scsi_mid_low_api.rst >> @@ -1190,11 +1190,11 @@ Members of interest: >> - pointer to scsi_device object that this command is >> associated with. >> resid >> - - an LLD should set this signed integer to the requested >> + - an LLD should set this unsigned integer to the requested >> transfer length (i.e. 'request_bufflen') less the number >> of bytes that are actually transferred. 'resid' is >> preset to 0 so an LLD can ignore it if it cannot detect >> - underruns (overruns should be rare). If possible an LLD >> + underruns (overruns should not be reported). An LLD > > I'm very late to party, sorry. But we have certainly seen at least one > overrun reported some years ago on a FC SAN. We've changed some handling > in zFCP due to that ( > a099b7b1fc1f ("scsi: zfcp: add handling for FCP_RESID_OVER to the fcp ingress path") > ). The theory back than was, that it was cause by either a faulty ISL in > the fabric, or some other "bit-errors" on the wire that caused some FC > frames being dropped during transmit. > > I added that we mark such commands with `DID_ERROR`, so they are > retried, if that is permissible. > >> should set 'resid' prior to invoking 'done'. The most >> interesting case is data transfers from a SCSI target >> device (e.g. READs) that underrun. > Thanks, that's interesting feedback. Feel free to submit a patch that refines the scsi_set_resid() documentation further. Bart.
diff --git a/Documentation/scsi/scsi_mid_low_api.rst b/Documentation/scsi/scsi_mid_low_api.rst index 6fa3a6279501..022198c51350 100644 --- a/Documentation/scsi/scsi_mid_low_api.rst +++ b/Documentation/scsi/scsi_mid_low_api.rst @@ -1190,11 +1190,11 @@ Members of interest: - pointer to scsi_device object that this command is associated with. resid - - an LLD should set this signed integer to the requested + - an LLD should set this unsigned integer to the requested transfer length (i.e. 'request_bufflen') less the number of bytes that are actually transferred. 'resid' is preset to 0 so an LLD can ignore it if it cannot detect - underruns (overruns should be rare). If possible an LLD + underruns (overruns should not be reported). An LLD should set 'resid' prior to invoking 'done'. The most interesting case is data transfers from a SCSI target device (e.g. READs) that underrun.
Because scsi_finish_command() subtracts the residual from the buffer length, residual overflows must not be reported. Reflect this in the SCSI documentation. See also commit 9237f04e12cc ("scsi: core: Fix scsi_get/set_resid() interface") Cc: Damien Le Moal <dlemoal@kernel.org> Cc: Hannes Reinecke <hare@suse.de> Cc: Douglas Gilbert <dgilbert@interlog.com> Cc: stable@vger.kernel.org Signed-off-by: Bart Van Assche <bvanassche@acm.org> --- Documentation/scsi/scsi_mid_low_api.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)