From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bart Van Assche Subject: Re: [PATCH 08/25] scsi: fix fast-fail for non-passthrough requests Date: Thu, 13 Apr 2017 20:41:07 +0000 Message-ID: <1492116064.24345.12.camel@sandisk.com> References: <20170406153944.10058-1-hch@lst.de> <20170406153944.10058-9-hch@lst.de> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: <20170406153944.10058-9-hch@lst.de> Content-Language: en-US Content-ID: <9F9BCC3B00A9BD40BBAFED87C6DCCC21@namprd04.prod.outlook.com> Sender: linux-block-owner@vger.kernel.org To: "hch@lst.de" , "axboe@kernel.dk" Cc: "linux-block@vger.kernel.org" , "konrad.wilk@oracle.com" , "roger.pau@citrix.com" , "linux-scsi@vger.kernel.org" , "linux-nvme@lists.infradead.org" , "jbacik@fb.com" , "james.smart@broadcom.com" , "dm-devel@redhat.com" List-Id: linux-scsi@vger.kernel.org On Thu, 2017-04-06 at 17:39 +0200, Christoph Hellwig wrote: > Currently error is always 0 for non-passthrough requests when reaching th= e > scsi_noretry_cmd check in scsi_io_completion, which effectively disables > all fastfail logic. Fix this by having a single call to > __scsi_error_from_host_byte at the beginning of the function and always > having a valid error value. >=20 > Signed-off-by: Christoph Hellwig > --- > drivers/scsi/scsi_lib.c | 28 +++++++--------------------- > 1 file changed, 7 insertions(+), 21 deletions(-) >=20 > diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c > index 11972d1075f1..89b4d9e69866 100644 > --- a/drivers/scsi/scsi_lib.c > +++ b/drivers/scsi/scsi_lib.c > @@ -779,21 +779,17 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsi= gned int good_bytes) > sense_valid =3D scsi_command_normalize_sense(cmd, &sshdr); > if (sense_valid) > sense_deferred =3D scsi_sense_is_deferred(&sshdr); > + > + if (!sense_deferred) > + error =3D __scsi_error_from_host_byte(cmd, result); > } Hello Christoph, Sorry but this doesn't look correct to me. Further down a "error =3D __scsi_error_from_host_byte(cmd, result)" statement is removed that does no= t depend on "if (!sense_deferred)" so I think that assignment should happen independent of the value of "sense_deferred". Additionally, how can it make sense to call __scsi_error_from_host_byte() only if sense_deferred =3D=3D f= alse? As you know the SCSI command result is generated by the LLD so I don't thin= k that it depends on whether or not the sense data has been deferred. Bart.=