* [PATCH V2] scsi_lib: removes ambiguous "Unhandled error code" messages. @ 2014-06-06 8:10 Maurizio Lombardi 2014-06-06 14:12 ` Christoph Hellwig ` (2 more replies) 0 siblings, 3 replies; 8+ messages in thread From: Maurizio Lombardi @ 2014-06-06 8:10 UTC (permalink / raw) To: James.Bottomley; +Cc: linux-scsi, hch During IO with fabric faults, one generally sees several "Unhandled error code" messages in the syslog as shown below: sd 4:0:6:2: [sdbw] Unhandled error code sd 4:0:6:2: [sdbw] Result: hostbyte=DID_NO_CONNECT driverbyte=DRIVER_OK sd 4:0:6:2: [sdbw] CDB: Read(10): 28 00 00 00 00 00 00 00 08 00 end_request: I/O error, dev sdbw, sector 0 This comes from scsi_io_completion (in scsi_lib.c) while handling error codes other than DID_RESET or not deferred sense keys i.e. this is actually handled by the SCSI mid layer. But what gets displayed here is "Unhandled error code" which is quite misleading as it indicates something that is not addressed by the mid layer. This patch removes "Unhandled error code" and replaces "Unhandled sense code" with "Failing command with sense code:". Signed-off-by: Maurizio Lombardi <mlombard@redhat.com> --- drivers/scsi/scsi_lib.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index 9db097a..b3c25cd 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -955,14 +955,12 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes) action = ACTION_FAIL; break; default: - description = "Unhandled sense code"; + description = "Failing command with sense code:"; action = ACTION_FAIL; break; } - } else { - description = "Unhandled error code"; + } else action = ACTION_FAIL; - } if (action != ACTION_FAIL && time_before(cmd->jiffies_at_alloc + wait_for, jiffies)) { -- Maurizio Lombardi ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH V2] scsi_lib: removes ambiguous "Unhandled error code" messages. 2014-06-06 8:10 [PATCH V2] scsi_lib: removes ambiguous "Unhandled error code" messages Maurizio Lombardi @ 2014-06-06 14:12 ` Christoph Hellwig 2014-06-25 11:06 ` Christoph Hellwig 2014-06-25 17:41 ` Mike Christie 2 siblings, 0 replies; 8+ messages in thread From: Christoph Hellwig @ 2014-06-06 14:12 UTC (permalink / raw) To: Maurizio Lombardi; +Cc: James.Bottomley, linux-scsi, hch On Fri, Jun 06, 2014 at 10:10:35AM +0200, Maurizio Lombardi wrote: > During IO with fabric faults, one generally sees several "Unhandled error > code" messages in the syslog as shown below: > > sd 4:0:6:2: [sdbw] Unhandled error code > sd 4:0:6:2: [sdbw] Result: hostbyte=DID_NO_CONNECT driverbyte=DRIVER_OK > sd 4:0:6:2: [sdbw] CDB: Read(10): 28 00 00 00 00 00 00 00 08 00 > end_request: I/O error, dev sdbw, sector 0 > > This comes from scsi_io_completion (in scsi_lib.c) while handling error > codes other than DID_RESET or not deferred sense keys i.e. this is > actually handled by the SCSI mid layer. But what gets displayed here is > "Unhandled error code" which is quite misleading as it indicates > something that is not addressed by the mid layer. > > This patch removes "Unhandled error code" and replaces "Unhandled sense code" > with "Failing command with sense code:". > > > Signed-off-by: Maurizio Lombardi <mlombard@redhat.com> Looks good, Reviewed-by: Christoph Hellwig <hch@lst.de> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V2] scsi_lib: removes ambiguous "Unhandled error code" messages. 2014-06-06 8:10 [PATCH V2] scsi_lib: removes ambiguous "Unhandled error code" messages Maurizio Lombardi 2014-06-06 14:12 ` Christoph Hellwig @ 2014-06-25 11:06 ` Christoph Hellwig 2014-06-25 20:38 ` Elliott, Robert (Server Storage) 2014-07-09 10:49 ` Hannes Reinecke 2014-06-25 17:41 ` Mike Christie 2 siblings, 2 replies; 8+ messages in thread From: Christoph Hellwig @ 2014-06-25 11:06 UTC (permalink / raw) To: Maurizio Lombardi; +Cc: James.Bottomley, linux-scsi, hch Can I get another review for this one? On Fri, Jun 06, 2014 at 10:10:35AM +0200, Maurizio Lombardi wrote: > During IO with fabric faults, one generally sees several "Unhandled error > code" messages in the syslog as shown below: > > sd 4:0:6:2: [sdbw] Unhandled error code > sd 4:0:6:2: [sdbw] Result: hostbyte=DID_NO_CONNECT driverbyte=DRIVER_OK > sd 4:0:6:2: [sdbw] CDB: Read(10): 28 00 00 00 00 00 00 00 08 00 > end_request: I/O error, dev sdbw, sector 0 > > This comes from scsi_io_completion (in scsi_lib.c) while handling error > codes other than DID_RESET or not deferred sense keys i.e. this is > actually handled by the SCSI mid layer. But what gets displayed here is > "Unhandled error code" which is quite misleading as it indicates > something that is not addressed by the mid layer. > > This patch removes "Unhandled error code" and replaces "Unhandled sense code" > with "Failing command with sense code:". > > > Signed-off-by: Maurizio Lombardi <mlombard@redhat.com> > --- > drivers/scsi/scsi_lib.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c > index 9db097a..b3c25cd 100644 > --- a/drivers/scsi/scsi_lib.c > +++ b/drivers/scsi/scsi_lib.c > @@ -955,14 +955,12 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes) > action = ACTION_FAIL; > break; > default: > - description = "Unhandled sense code"; > + description = "Failing command with sense code:"; > action = ACTION_FAIL; > break; > } > - } else { > - description = "Unhandled error code"; > + } else > action = ACTION_FAIL; > - } > > if (action != ACTION_FAIL && > time_before(cmd->jiffies_at_alloc + wait_for, jiffies)) { > -- > Maurizio Lombardi > > -- > To unsubscribe from this list: send the line "unsubscribe linux-scsi" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ---end quoted text--- ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH V2] scsi_lib: removes ambiguous "Unhandled error code" messages. 2014-06-25 11:06 ` Christoph Hellwig @ 2014-06-25 20:38 ` Elliott, Robert (Server Storage) 2014-07-03 8:41 ` Christoph Hellwig 2014-07-09 10:49 ` Hannes Reinecke 1 sibling, 1 reply; 8+ messages in thread From: Elliott, Robert (Server Storage) @ 2014-06-25 20:38 UTC (permalink / raw) To: Christoph Hellwig, Maurizio Lombardi; +Cc: James.Bottomley, linux-scsi > -----Original Message----- > From: linux-scsi-owner@vger.kernel.org [mailto:linux-scsi- > owner@vger.kernel.org] On Behalf Of Christoph Hellwig > Sent: Wednesday, 25 June, 2014 6:07 AM > To: Maurizio Lombardi > Cc: James.Bottomley@HansenPartnership.com; linux-scsi@vger.kernel.org; > hch@infradead.org > Subject: Re: [PATCH V2] scsi_lib: removes ambiguous "Unhandled error code" > messages. > > Can I get another review for this one? > > On Fri, Jun 06, 2014 at 10:10:35AM +0200, Maurizio Lombardi wrote: ... > > diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c ... > > @@ -955,14 +955,12 @@ void scsi_io_completion(struct scsi_cmnd *cmd, > unsigned int good_bytes) > > action = ACTION_FAIL; > > break; > > default: > > - description = "Unhandled sense code"; > > + description = "Failing command with sense code:"; The default case handles sense keys other than: #define NOT_READY 0x02 #define ILLEGAL_REQUEST 0x05 #define UNIT_ATTENTION 0x06 #define ABORTED_COMMAND 0x0b #define VOLUME_OVERFLOW 0x0d which means these #defines (and any other values) #define NO_SENSE 0x00 #define RECOVERED_ERROR 0x01 #define MEDIUM_ERROR 0x03 #define HARDWARE_ERROR 0x04 #define DATA_PROTECT 0x07 #define BLANK_CHECK 0x08 #define COPY_ABORTED 0x0a #define MISCOMPARE 0x0e The other description strings that result in ACTION_FAIL are based on the sense key and sometimes the additional sense code: description = "Media Changed"; description = "Host Data Integrity Failure"; description = "Discard failure"; description = "Write same failure"; description = "Invalid command failure"; description = "Target Data Integrity Failure"; description = "Device not ready"; Also, there is this one, which is not based on the sense key: description = "Command timed out"; Since the ACTION_FAIL case always prints the sense key and additional sense code: if (!(req->cmd_flags & REQ_QUIET)) { if (description) scmd_printk(KERN_INFO, cmd, " % \n", description); scsi_print_result(cmd); if (driver_byte(result) & DRIVER_SENSE) scsi_print_sense("", cmd); scsi_print_command(cmd); } perhaps the description string should be removed altogether? In this example: sd 1:0:0:0: [sdb] Unhandled sense code sd 1:0:0:0: [sdb] Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE sd 1:0:0:0: [sdb] Sense Key : Medium Error [current] sd 1:0:0:0: [sdb] Add. Sense: Unrecovered read error sd 1:0:0:0: [sdb] CDB: Read(10): 28 00 00 18 f8 a8 00 00 08 00 end_request: critical medium error, dev sdb, sector 1636520 "Sense Key : Medium Error" is much more informative than "Unhandled sense code" or "Failing command with sense code". All the other descriptions represent failing commands, so using different wording is a bit confusing. For the "Unhandled error code" (for which you are proposing removing the string) and the timeout case, the scsi_print_result call already prints hostbyte and driverbyte, which explain what happened in more detail: sd 1:0:0:0: [sda] Unhandled error code sd 1:0:0:0: [sda] Result: hostbyte=DID_TIME_OUT driverbyte=DRIVER_OK sd 1:0:0:0: [sda] CDB: Test Unit Ready: end_request: I/O error, dev sda, sector 481013632 --- Rob Elliott HP Server Storage ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V2] scsi_lib: removes ambiguous "Unhandled error code" messages. 2014-06-25 20:38 ` Elliott, Robert (Server Storage) @ 2014-07-03 8:41 ` Christoph Hellwig 2014-07-08 8:36 ` Maurizio Lombardi 0 siblings, 1 reply; 8+ messages in thread From: Christoph Hellwig @ 2014-07-03 8:41 UTC (permalink / raw) To: Elliott, Robert (Server Storage) Cc: Maurizio Lombardi, James.Bottomley, linux-scsi On Wed, Jun 25, 2014 at 08:38:47PM +0000, Elliott, Robert (Server Storage) wrote: > Since the ACTION_FAIL case always prints the sense key > and additional sense code: > perhaps the description string should be removed altogether? > For the "Unhandled error code" (for which you are proposing > removing the string) and the timeout case, the scsi_print_result > call already prints hostbyte and driverbyte, which explain what > happened in more detail: I'm tempted to agree and just remove the description. Do you want to send a patch for this? ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V2] scsi_lib: removes ambiguous "Unhandled error code" messages. 2014-07-03 8:41 ` Christoph Hellwig @ 2014-07-08 8:36 ` Maurizio Lombardi 0 siblings, 0 replies; 8+ messages in thread From: Maurizio Lombardi @ 2014-07-08 8:36 UTC (permalink / raw) To: Christoph Hellwig, Elliott, Robert (Server Storage) Cc: James.Bottomley, linux-scsi On 07/03/2014 10:41 AM, Christoph Hellwig wrote: > On Wed, Jun 25, 2014 at 08:38:47PM +0000, Elliott, Robert (Server Storage) wrote: >> Since the ACTION_FAIL case always prints the sense key >> and additional sense code: > >> perhaps the description string should be removed altogether? > > > I'm tempted to agree and just remove the description. Do you want to > send a patch for this? So I'll get rid of the "description" string completely... I'm going to send a new patch later today. Regards, Maurizio Lombardi ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V2] scsi_lib: removes ambiguous "Unhandled error code" messages. 2014-06-25 11:06 ` Christoph Hellwig 2014-06-25 20:38 ` Elliott, Robert (Server Storage) @ 2014-07-09 10:49 ` Hannes Reinecke 1 sibling, 0 replies; 8+ messages in thread From: Hannes Reinecke @ 2014-07-09 10:49 UTC (permalink / raw) To: Christoph Hellwig, Maurizio Lombardi; +Cc: James.Bottomley, linux-scsi On 06/25/2014 01:06 PM, Christoph Hellwig wrote: > Can I get another review for this one? > > On Fri, Jun 06, 2014 at 10:10:35AM +0200, Maurizio Lombardi wrote: >> During IO with fabric faults, one generally sees several "Unhandled error >> code" messages in the syslog as shown below: >> >> sd 4:0:6:2: [sdbw] Unhandled error code >> sd 4:0:6:2: [sdbw] Result: hostbyte=DID_NO_CONNECT driverbyte=DRIVER_OK >> sd 4:0:6:2: [sdbw] CDB: Read(10): 28 00 00 00 00 00 00 00 08 00 >> end_request: I/O error, dev sdbw, sector 0 >> >> This comes from scsi_io_completion (in scsi_lib.c) while handling error >> codes other than DID_RESET or not deferred sense keys i.e. this is >> actually handled by the SCSI mid layer. But what gets displayed here is >> "Unhandled error code" which is quite misleading as it indicates >> something that is not addressed by the mid layer. >> >> This patch removes "Unhandled error code" and replaces "Unhandled sense code" >> with "Failing command with sense code:". >> >> >> Signed-off-by: Maurizio Lombardi <mlombard@redhat.com> >> --- >> drivers/scsi/scsi_lib.c | 6 ++---- >> 1 file changed, 2 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c >> index 9db097a..b3c25cd 100644 >> --- a/drivers/scsi/scsi_lib.c >> +++ b/drivers/scsi/scsi_lib.c >> @@ -955,14 +955,12 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes) >> action = ACTION_FAIL; >> break; >> default: >> - description = "Unhandled sense code"; >> + description = "Failing command with sense code:"; >> action = ACTION_FAIL; >> break; >> } >> - } else { >> - description = "Unhandled error code"; >> + } else >> action = ACTION_FAIL; >> - } >> >> if (action != ACTION_FAIL && >> time_before(cmd->jiffies_at_alloc + wait_for, jiffies)) { >> -- >> Maurizio Lombardi >> Good to see it finally went in. We're carrying the same patch in our tree for ages as the original attempt to push it upstream failed. Reviewed-by: Hannes Reinecke <hare@suse.de> Cheers, Hannes -- Dr. Hannes Reinecke zSeries & Storage hare@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V2] scsi_lib: removes ambiguous "Unhandled error code" messages. 2014-06-06 8:10 [PATCH V2] scsi_lib: removes ambiguous "Unhandled error code" messages Maurizio Lombardi 2014-06-06 14:12 ` Christoph Hellwig 2014-06-25 11:06 ` Christoph Hellwig @ 2014-06-25 17:41 ` Mike Christie 2 siblings, 0 replies; 8+ messages in thread From: Mike Christie @ 2014-06-25 17:41 UTC (permalink / raw) To: Maurizio Lombardi; +Cc: James.Bottomley, linux-scsi, hch On 06/06/2014 03:10 AM, Maurizio Lombardi wrote: > During IO with fabric faults, one generally sees several "Unhandled error > code" messages in the syslog as shown below: > > sd 4:0:6:2: [sdbw] Unhandled error code > sd 4:0:6:2: [sdbw] Result: hostbyte=DID_NO_CONNECT driverbyte=DRIVER_OK > sd 4:0:6:2: [sdbw] CDB: Read(10): 28 00 00 00 00 00 00 00 08 00 > end_request: I/O error, dev sdbw, sector 0 > > This comes from scsi_io_completion (in scsi_lib.c) while handling error > codes other than DID_RESET or not deferred sense keys i.e. this is > actually handled by the SCSI mid layer. But what gets displayed here is > "Unhandled error code" which is quite misleading as it indicates > something that is not addressed by the mid layer. > > This patch removes "Unhandled error code" and replaces "Unhandled sense code" > with "Failing command with sense code:". > > > Signed-off-by: Maurizio Lombardi <mlombard@redhat.com> > --- > drivers/scsi/scsi_lib.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c > index 9db097a..b3c25cd 100644 > --- a/drivers/scsi/scsi_lib.c > +++ b/drivers/scsi/scsi_lib.c > @@ -955,14 +955,12 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes) > action = ACTION_FAIL; > break; > default: > - description = "Unhandled sense code"; > + description = "Failing command with sense code:"; > action = ACTION_FAIL; > break; > } > - } else { > - description = "Unhandled error code"; > + } else > action = ACTION_FAIL; > - } > > if (action != ACTION_FAIL && > time_before(cmd->jiffies_at_alloc + wait_for, jiffies)) { > Looks ok to me. Reviewed-by: Mike Christie <michaelc@cs.wisc.edu> ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2014-07-09 10:49 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-06-06 8:10 [PATCH V2] scsi_lib: removes ambiguous "Unhandled error code" messages Maurizio Lombardi 2014-06-06 14:12 ` Christoph Hellwig 2014-06-25 11:06 ` Christoph Hellwig 2014-06-25 20:38 ` Elliott, Robert (Server Storage) 2014-07-03 8:41 ` Christoph Hellwig 2014-07-08 8:36 ` Maurizio Lombardi 2014-07-09 10:49 ` Hannes Reinecke 2014-06-25 17:41 ` Mike Christie
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.