SCSI: Make error printing more user friendly
PATCH 1/1 SCSI: Make error printing more user friendly
This patch makes SCSI error printing more user friendly by:
1. Replacing the (currently unused) functions scsi_print_hostbyte() and scsi_print_driverbyte() with scsi_print_result()
2. Replacing the dreaded "SCSI error: return code" in scsi_io_completion with a call to scsi_print_result()
3. Making scmd_printk() output the disk name if available
The net result is that:
sd 0:0:0:0: SCSI error: return code = 0x08000002
sda: Current: sense key: Aborted Command
Additional sense: Logical block reference tag check failed
becomes:
sda: Error: hostbyte=DID_OK driverbyte=DRIVER_SENSE,SUGGEST_OK
sda: Current: sense key: Aborted Command
Additional sense: Logical block reference tag check failed
---
drivers/scsi/constants.c | 48 +++++++++++++++++++-------------------------- drivers/scsi/scsi_lib.c | 4 --- include/scsi/scsi_dbg.h | 3 -- include/scsi/scsi_device.h | 3 ++ 4 files changed, 26 insertions(+), 32 deletions(-)
drivers/scsi/constants.c +21 -27
@@ -1327,25 +1327,6 @@ static const char * const hostbyte_table[]={ "DID_PASSTHROUGH", "DID_SOFT_ERROR", "DID_IMM_RETRY"};#define NUM_HOSTBYTE_STRS ARRAY_SIZE(hostbyte_table) -void scsi_print_hostbyte (int scsiresult) -{ -int hb =host_byte (scsiresult); - -printk ("Hostbyte=0x%02x" , hb); -if (hb <NUM_HOSTBYTE_STRS ) -printk ("(%s) " , hostbyte_table[hb]); -else -printk ("is invalid " ); -} -#else -void scsi_print_hostbyte (int scsiresult) -{ -printk ("Hostbyte=0x%02x " ,host_byte (scsiresult)); -} -#endif - -#ifdef CONFIG_SCSI_CONSTANTS -static const char *const driverbyte_table[]={"DRIVER_OK" ,"DRIVER_BUSY" ,"DRIVER_SOFT" ,"DRIVER_MEDIA" ,"DRIVER_ERROR" ,"DRIVER_INVALID" ,"DRIVER_TIMEOUT" ,"DRIVER_HARD" ,"DRIVER_SENSE" };
@@ -1356,19 +1337,32 @@ static const char * const driversuggest_table[]={"SUGGEST_OK", "SUGGEST_5", "SUGGEST_6", "SUGGEST_7", "SUGGEST_SENSE"};#define NUM_SUGGEST_STRS ARRAY_SIZE(driversuggest_table) -void scsi_print_driverbyte (int scsiresult) +void scsi_print_result (struct scsi_cmnd *cmd) { -int dr = (driver_byte (scsiresult) &DRIVER_MASK ); -int su = ((driver_byte (scsiresult) &SUGGEST_MASK ) >>4 ); +int hb =host_byte (cmd->result); +int db = (driver_byte (cmd->result) &DRIVER_MASK ); +int su = ((driver_byte (cmd->result) &SUGGEST_MASK ) >>4 ); + +scmd_printk (KERN_INFO , cmd,"Error: " ); -printk ("Driverbyte=0x%02x " ,driver_byte (scsiresult)); -printk ("(%s,%s) " , - (dr <NUM_DRIVERBYTE_STRS ? driverbyte_table[dr] :"invalid" ), +printk ("hostbyte=%s driverbyte=%s,%s\n" , + (hb <NUM_HOSTBYTE_STRS ? hostbyte_table[hb] :"invalid" ), + (db <NUM_DRIVERBYTE_STRS ? driverbyte_table[db] :"invalid" ), (su <NUM_SUGGEST_STRS ? driversuggest_table[su] :"invalid" )); } +#else -void scsi_print_driverbyte (int scsiresult) + +void scsi_print_result (struct scsi_cmnd *cmd) { -printk ("Driverbyte=0x%02x " ,driver_byte (scsiresult)); +int result = cmd->result; + +scmd_printk (KERN_INFO , cmd, +"Error: hostbyte=0x%02x driverbyte=0x%02x\n" , +host_byte (result),driver_byte (result)); } +#endif + +EXPORT_SYMBOL (scsi_print_result); +
drivers/scsi/scsi_lib.c +1 -3
@@ -969,9 +969,7 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes) }if (result) {if (!(req->cmd_flags &REQ_QUIET )) { -scmd_printk (KERN_INFO , cmd, -"SCSI error: return code = 0x%08x\n" , - result); +scsi_print_result (cmd);if (driver_byte (result) &DRIVER_SENSE )scsi_print_sense ("" , cmd); }
include/scsi/scsi_dbg.h +1 -2
@@ -11,8 +11,7 @@ extern void scsi_print_sense(const char *, struct scsi_cmnd *);extern void __scsi_print_sense (const char *name,const unsigned char *sense_buffer,int sense_len); -extern void scsi_print_driverbyte (int ); -extern void scsi_print_hostbyte (int ); +extern void scsi_print_result (struct scsi_cmnd *);extern void scsi_print_status (unsigned char );extern const char *scsi_sense_key_string (unsigned char );extern const char *scsi_extd_sense_format (unsigned char ,unsigned char );
include/scsi/scsi_device.h +3 -0
@@ -5,6 +5,7 @@#include <linux/list.h> #include <linux/spinlock.h> #include <linux/workqueue.h> +#include <linux/blkdev.h> #include <asm/atomic.h> struct request_queue ;
@@ -154,6 +155,8 @@ struct scsi_device { dev_printk(prefix, &(sdev)->sdev_gendev, fmt, ##a) #define scmd_printk(prefix, scmd, fmt, a...) \ + (&(scmd)->request->rq_disk) ? \ + printk(prefix "%s: " fmt, (char *) &(scmd)->request->rq_disk->disk_name, ##a) : \ dev_printk(prefix, &(scmd)->device->sdev_gendev, fmt, ##a)enum scsi_target_state {
I think all of this is OK except this piece
include/scsi/scsi_device.h struct scsi_device { · line 160
Quoted 4 lines · Martin K. Petersen
#define
scmd_printk(prefix, scmd, fmt, a...)\include/scsi/scsi_device.h +2
-0+ (&(scmd)->request->rq_disk) ? \ + printk(prefix "%s: " fmt, (char *) &(scmd)->request->rq_disk->disk_name, ##a) : \ dev_printk(prefix, &(scmd)->device->sdev_gendev, fmt, ##a)
I think we should be going through dev_printk as the primary, particularly as doing this would make the output of sdev_printk different from scmd_printk. How about
#define scmd_printk(prefix, scmd, fmt, a...) \ (scmd)->request->rq_disk ? \ sdev_printk(prefix, (scmd)->device, "(%s) " fmt, \ (scmd)->request->rq_disk->disk_name, ##a) : \ sdev_printk(prefix, (scmd)->device, fmt, ##a)
?
The other nasty is that we can't actually deref rq_disk unless <linux/blkdev.h> is included. You'll find megaraid_sas.c fails to compile becuase of this. It can probably be fixed by including blkdev.h from scsi_device.h
James
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"James" == James Bottomley <[email protected]> writes:
James> I think we should be going through dev_printk as the primary, James> particularly as doing this would make the output of sdev_printk James> different from scmd_printk. How about
Fine with me except we're now back to having inconsistent prefixes for the result printing and the matching sense data.
I'll try to sanitize the sense printing too and post a new patch.
James> The other nasty is that we can't actually deref rq_disk unless James> <linux/blkdev.h> is included. You'll find megaraid_sas.c fails James> to compile becuase of this. It can probably be fixed by James> including blkdev.h from scsi_device.h
You must have had a reject on scsi_device.h. My patch did include <linux/blkdev.h> to fix that very issue.
Signature · 1 line
Martin K. Petersen Oracle Linux Engineering