scsi logging update (the boring part)
Subsystems (11)
v5 cover scsi logging update (the boring part)
This is the next iteration of my scsi logging patchset. The patchset just contains some logging updates, code reshuffling and sanity fixes. Nothing major.
The second part of this patchset (the printk bit) has been vetoed for the moment, so I'll have to work on that. But in any case this patchset is pretty much independent.
Difference to v4:
- Add 'Reviewed-by' tags from hch where applicable
- Split off the fas216 patch into two different patches
- Added a patch to document scsi_try_to_abort_cmd
Hannes Reinecke (24):
Remove scsi_cmd_print_sense_hdr()
sd: Remove scsi_print_sense() in sd_done()
aha152x: Debug output update and whitespace cleanup
scsi: introduce sdev_prefix_printk()
scsi: Use sdev as argument for sense code printing
acornscsi: use scsi_print_command()
fas216: Return DID_ERROR for incomplete data transfer
fas216: Update logging messages
53c700: remove scsi_print_sense() usage
scsi: stop decoding if scsi_normalize_sense() fails
scsi: do not decode sense extras
scsi: use 'bool' as return value for scsi_normalize_sense()
scsi: remove scsi_print_status()
Implement scsi_opcode_sa_name
scsi: merge print_opcode_name()
scsi: consolidate opcode lookup in scsi_opcode_sa_name()
scsi: remove last argument from print_opcode_name()
scsi: Remove scsi_print_command when calling abort
scsi: separate out scsi_(host|driver)byte_string()
sd: Cleanup logging
scsi: simplify scsi_log_(send|completion)
scsi: fixup logging messages in scsi_error.c
scsi: use shost argument in scsi_eh_prt_fail_stats
scsi_error: document scsi_try_to_abort_cmd
drivers/scsi/53c700.c | 13 +- drivers/scsi/NCR5380.c | 5 +- drivers/scsi/aha152x.c | 994 ++++++++++--------------------------------- drivers/scsi/arm/acornscsi.c | 12 +- drivers/scsi/arm/fas216.c | 41 +- drivers/scsi/atari_NCR5380.c | 3 +- drivers/scsi/ch.c | 5 +- drivers/scsi/constants.c | 572 ++++++++++--------------- drivers/scsi/hosts.c | 4 +- drivers/scsi/osst.c | 8 +- drivers/scsi/ps3rom.c | 4 - drivers/scsi/scsi.c | 45 +- drivers/scsi/scsi_error.c | 120 +++--- drivers/scsi/scsi_ioctl.c | 2 +- drivers/scsi/scsi_lib.c | 19 +- drivers/scsi/sd.c | 55 +-- drivers/scsi/sd.h | 7 +- drivers/scsi/sg.c | 6 +- drivers/scsi/sr.h | 3 +- drivers/scsi/sr_ioctl.c | 6 +- drivers/scsi/st.c | 9 +- drivers/scsi/stex.c | 9 +- drivers/scsi/storvsc_drv.c | 3 +- drivers/scsi/sun3_NCR5380.c | 3 +- include/scsi/scsi_dbg.h | 26 +- include/scsi/scsi_device.h | 9 + include/scsi/scsi_eh.h | 14 +- 27 files changed, 662 insertions(+), 1335 deletions(-)
Signature · 1 line
1.8.5.2
PATCH v5 1/24 Remove scsi_cmd_print_sense_hdr()
Unused.
Reviewed-by Christoph Hellwig <[email protected]>
Signed-off-by Hannes Reinecke <[email protected]>
--- drivers/scsi/constants.c | 14 -------------- include/scsi/scsi_dbg.h | 2 -- 2 files changed, 16 deletions(-)
drivers/scsi/constants.c +0 -14
@@ -1428,20 +1428,6 @@ scsi_print_sense_hdr(const char *name, struct scsi_sense_hdr *sshdr) }EXPORT_SYMBOL (scsi_print_sense_hdr); -/* -* Print normalized SCSI sense header with device information and a prefix. -*/ -void -scsi_cmd_print_sense_hdr (struct scsi_cmnd *scmd,const char *desc, -struct scsi_sense_hdr *sshdr) -{ -scmd_printk (KERN_INFO , scmd,"%s: " , desc); -scsi_show_sense_hdr (sshdr); -scmd_printk (KERN_INFO , scmd,"%s: " , desc); -scsi_show_extd_sense (sshdr->asc, sshdr->ascq); -} -EXPORT_SYMBOL (scsi_cmd_print_sense_hdr); -static void scsi_decode_sense_buffer (const unsigned char *sense_buffer,int sense_len,struct scsi_sense_hdr *sshdr)
include/scsi/scsi_dbg.h +0 -2
@@ -9,8 +9,6 @@ extern void __scsi_print_command(unsigned char *);extern void scsi_show_extd_sense (unsigned char ,unsigned char );extern void scsi_show_sense_hdr (struct scsi_sense_hdr *);extern void scsi_print_sense_hdr (const char *,struct scsi_sense_hdr *); -extern void scsi_cmd_print_sense_hdr (struct scsi_cmnd *,const char *, -struct scsi_sense_hdr *);extern void scsi_print_sense (char *,struct scsi_cmnd *); extern void __scsi_print_sense(const char *name, const unsigned char *sense_buffer,
Signature · 1 line
1.8.5.2
PATCH v5 2/24 sd: Remove scsi_print_sense() in sd_done()
In reply to Hannes Reineckesd_done() was calling scsi_print_sense() for a sense code of 'NO_SENSE'.
Reviewed-by Christoph Hellwig <[email protected]>
Signed-off-by Hannes Reinecke <[email protected]>
--- drivers/scsi/sd.c | 1 - 1 file changed, 1 deletion(-)
drivers/scsi/sd.c +0 -1
@@ -1730,7 +1730,6 @@ static int sd_done(struct scsi_cmnd *SCpnt)* unknown amount of data was transferred so treat it as an * error. */ -scsi_print_sense ("sd" , SCpnt); SCpnt->result =0 ;memset (SCpnt->sense_buffer,0 ,SCSI_SENSE_BUFFERSIZE );break ;
Signature · 1 line
1.8.5.2
PATCH v5 3/24 aha152x: Debug output update and whitespace cleanup
In reply to Hannes ReineckeRemove all uncommented debugging code and move all printk() statements over to dev_printk().
And while we're at it we should be doing a whitespace cleanup, too.
Reviewed-by Christoph Hellwig <[email protected]>
Signed-off-by Hannes Reinecke <[email protected]>
--- drivers/scsi/aha152x.c | 994 +++++++++++-------------------------------------- 1 file changed, 224 insertions(+), 770 deletions(-)
drivers/scsi/aha152x.c +224 -770
@@ -230,7 +230,7 @@* * ************************************************************************** -+ see Documentation/scsi/aha152x.txt for configuration details **************************************************************************/
@@ -279,45 +279,11 @@ static LIST_HEAD(aha152x_host_list);#error define AUTOCONF or SETUP0#endif -#if defined(AHA152X_DEBUG ) -#define DEBUG_DEFAULT debug_eh - -#define DPRINTK(when,msgs...) \ - do { if(HOSTDATA(shpnt)->debug & (when)) printk(msgs); } while(0) - -#define DO_LOCK (flags) \ - do { \ - if(spin_is_locked(&QLOCK)) { \ - DPRINTK(debug_intr, DEBUG_LEAD "(%s:%d) already locked at %s:%d\n", CMDINFO(CURRENT_SC), __func__, __LINE__, QLOCKER, QLOCKERL); \ - } \ - DPRINTK(debug_locking, DEBUG_LEAD "(%s:%d) locking\n", CMDINFO(CURRENT_SC), __func__, __LINE__); \ - spin_lock_irqsave(&QLOCK,flags); \ - DPRINTK(debug_locking, DEBUG_LEAD "(%s:%d) locked\n", CMDINFO(CURRENT_SC), __func__, __LINE__); \ - QLOCKER=__func__; \ - QLOCKERL=__LINE__; \ - } while(0) - -#define DO_UNLOCK (flags) \ - do { \ - DPRINTK(debug_locking, DEBUG_LEAD "(%s:%d) unlocking (locked at %s:%d)\n", CMDINFO(CURRENT_SC), __func__, __LINE__, QLOCKER, QLOCKERL); \ - spin_unlock_irqrestore(&QLOCK,flags); \ - DPRINTK(debug_locking, DEBUG_LEAD "(%s:%d) unlocked\n", CMDINFO(CURRENT_SC), __func__, __LINE__); \ - QLOCKER="(not locked)"; \ - QLOCKERL=0; \ - } while(0) - -#else -#define DPRINTK(when,msgs...)#define DO_LOCK (flags) spin_lock_irqsave(&QLOCK,flags)#define DO_UNLOCK (flags) spin_unlock_irqrestore(&QLOCK,flags) -#endif #define LEAD "(scsi%d:%d:%d) " -#define WARN_LEAD KERN_WARNING LEAD#define INFO_LEAD KERN_INFO LEAD -#define NOTE_LEAD KERN_NOTICE LEAD -#define ERR_LEAD KERN_ERR LEAD -#define DEBUG_LEAD KERN_DEBUG LEAD#define CMDINFO (cmd) \ (cmd) ? ((cmd)->device->host->host_no) : -1, \ (cmd) ? ((cmd)->device->id & 0x0f) : -1, \
@@ -345,10 +311,10 @@ CMD_INC_RESID(struct scsi_cmnd *cmd, int inc)enum { not_issued =0x0001 ,/* command not yet issued */ - selecting =0x0002 ,/* target is beeing selected */ + selecting =0x0002 ,/* target is beeing selected */ identified =0x0004 ,/* IDENTIFY was sent */ disconnected =0x0008 ,/* target disconnected */ - completed =0x0010 ,/* target sent COMMAND COMPLETE */ + completed =0x0010 ,/* target sent COMMAND COMPLETE */ aborted =0x0020 ,/* ABORT was sent */ resetted =0x0040 ,/* BUS DEVICE RESET was sent */ spiordy =0x0080 ,/* waiting for SPIORDY to raise */
@@ -396,7 +362,6 @@ static int exttrans[] = {0, 0};module_param_array (exttrans, int,NULL ,0 );MODULE_PARM_DESC (exttrans,"use extended translation" ); -#if !defined(AHA152X_DEBUG )static int aha152x[] = {0 ,11 ,7 ,1 ,1 ,0 ,DELAY_DEFAULT ,0 };module_param_array (aha152x, int,NULL ,0 );MODULE_PARM_DESC (aha152x,"parameters for first controller" );
@@ -404,19 +369,6 @@ MODULE_PARM_DESC(aha152x, "parameters for first controller");static int aha152x1[] = {0 ,11 ,7 ,1 ,1 ,0 ,DELAY_DEFAULT ,0 };module_param_array (aha152x1, int,NULL ,0 );MODULE_PARM_DESC (aha152x1,"parameters for second controller" ); -#else -static int debug[] = {DEBUG_DEFAULT ,DEBUG_DEFAULT }; -module_param_array (debug, int,NULL ,0 ); -MODULE_PARM_DESC (debug,"flags for driver debugging" ); - -static int aha152x[] = {0 ,11 ,7 ,1 ,1 ,1 ,DELAY_DEFAULT ,0 ,DEBUG_DEFAULT }; -module_param_array (aha152x, int,NULL ,0 ); -MODULE_PARM_DESC (aha152x,"parameters for first controller" ); - -static int aha152x1[] = {0 ,11 ,7 ,1 ,1 ,1 ,DELAY_DEFAULT ,0 ,DEBUG_DEFAULT }; -module_param_array (aha152x1, int,NULL ,0 ); -MODULE_PARM_DESC (aha152x1,"parameters for second controller" ); -#endif /* !defined(AHA152X_DEBUG) */ #endif /* MODULE */ #ifdef __ISAPNP__
@@ -446,7 +398,7 @@ static struct scsi_host_template aha152x_driver_template;/* * internal states of the host * -*/ +*/ enum aha152x_state { idle=0 , unknown,
@@ -485,24 +437,16 @@ struct aha152x_hostdata {spinlock_t lock;/* host lock */ -#if defined(AHA152X_DEBUG ) -const char *locker; -/* which function has the lock */ -int lockerl;/* where did it get it */ - -int debug;/* current debugging setting */ -#endif -#if defined(AHA152X_STAT ) -int total_commands; +int total_commands;int disconnections;int busfree_without_any_action;int busfree_without_old_command;int busfree_without_new_command;int busfree_without_done_command;int busfree_with_check_condition; -int count[maxstate]; -int count_trans[maxstate]; +int count[maxstate]; +int count_trans[maxstate];unsigned long time[maxstate];#endif
@@ -514,7 +458,7 @@ struct aha152x_hostdata {int delay;/* reset out delay */ int ext_trans;/* extended translation enabled */ -int swint;/* software-interrupt was fired during detect() */ +int swint;/* software-interrupt was fired during detect() */ int service;/* bh needs to be run */ int in_intr;/* bh is running */
@@ -543,7 +487,7 @@ struct aha152x_hostdata {unsigned char msgi[256 ];/* received message bytes */ -int msgo_i, msgo_len; +int msgo_i, msgo_len;/* number of sent bytes and length of current messages */ unsigned char msgo[256 ];/* pending messages */
@@ -689,7 +633,6 @@ static void aha152x_error(struct Scsi_Host *shpnt, char *msg);static void done (struct Scsi_Host *shpnt,int error);/* diagnostics */ -static void disp_ports (struct Scsi_Host *shpnt);static void show_command (Scsi_Cmnd * ptr);static void show_queues (struct Scsi_Host *shpnt);static void disp_enintr (struct Scsi_Host *shpnt);
@@ -812,10 +755,6 @@ struct Scsi_Host *aha152x_probe_one(struct aha152x_setup *setup)DELAY = setup->delay;EXT_TRANS = setup->ext_trans; -#if defined(AHA152X_DEBUG ) -HOSTDATA (shpnt)->debug = setup->debug; -#endif -SETPORT (SCSIID , setup->scsiid <<4 ); shpnt->this_id = setup->scsiid;
@@ -941,31 +880,24 @@ void aha152x_release(struct Scsi_Host *shpnt)* setup controller to generate interrupts depending * on current state (lock has to be acquired) * -*/ +*/ static int setup_expected_interrupts (struct Scsi_Host *shpnt) {if (CURRENT_SC ) {CURRENT_SC ->SCp.phase |=1 <<16 ; - +if (CURRENT_SC ->SCp.phase & selecting) { -DPRINTK (debug_intr,DEBUG_LEAD "expecting: (seldo) (seltimo) (seldi)\n" ,CMDINFO (CURRENT_SC ));SETPORT (SSTAT1 ,SELTO );SETPORT (SIMODE0 ,ENSELDO | (DISCONNECTED_SC ?ENSELDI :0 ));SETPORT (SIMODE1 ,ENSELTIMO ); }else { -DPRINTK (debug_intr,DEBUG_LEAD "expecting: (phase change) (busfree) %s\n" ,CMDINFO (CURRENT_SC ),CURRENT_SC ->SCp.phase & spiordy ?"(spiordy)" :"" );SETPORT (SIMODE0 , (CURRENT_SC ->SCp.phase & spiordy) ?ENSPIORDY :0 ); -SETPORT (SIMODE1 ,ENPHASEMIS |ENSCSIRST |ENSCSIPERR |ENBUSFREE ); +SETPORT (SIMODE1 ,ENPHASEMIS |ENSCSIRST |ENSCSIPERR |ENBUSFREE ); } }else if (STATE ==seldi) { -DPRINTK (debug_intr,DEBUG_LEAD "expecting: (phase change) (identify)\n" ,CMDINFO (CURRENT_SC ));SETPORT (SIMODE0 ,0 ); -SETPORT (SIMODE1 ,ENPHASEMIS |ENSCSIRST |ENSCSIPERR |ENBUSFREE ); +SETPORT (SIMODE1 ,ENPHASEMIS |ENSCSIRST |ENSCSIPERR |ENBUSFREE ); }else { -DPRINTK (debug_intr,DEBUG_LEAD "expecting: %s %s\n" , -CMDINFO (CURRENT_SC ), -DISCONNECTED_SC ?"(reselection)" :"" , -ISSUE_SC ?"(busfree)" :"" );SETPORT (SIMODE0 ,DISCONNECTED_SC ?ENSELDI :0 );SETPORT (SIMODE1 ,ENSCSIRST | ( (ISSUE_SC ||DONE_SC ) ?ENBUSFREE :0 )); }
@@ -977,7 +909,7 @@ static int setup_expected_interrupts(struct Scsi_Host *shpnt) } -/* +/* * Queue a command and setup interrupts for a free bus. */ static int aha152x_internal_queue(Scsi_Cmnd *SCpnt, struct completion *complete,
@@ -986,15 +918,6 @@ static int aha152x_internal_queue(Scsi_Cmnd *SCpnt, struct completion *complete,struct Scsi_Host *shpnt = SCpnt->device->host;unsigned long flags; -#if defined(AHA152X_DEBUG ) -if (HOSTDATA (shpnt)->debug & debug_queue) { -printk (INFO_LEAD "queue: %p; cmd_len=%d pieces=%d size=%u cmnd=" , -CMDINFO (SCpnt), SCpnt, SCpnt->cmd_len, -scsi_sg_count (SCpnt),scsi_bufflen (SCpnt)); -__scsi_print_command (SCpnt->cmnd); - } -#endif - SCpnt->scsi_done = done; SCpnt->SCp.phase = not_issued | phase; SCpnt->SCp.Status =0x1 ;/* Ilegal status by SCSI standard */
@@ -1004,13 +927,13 @@ static int aha152x_internal_queue(Scsi_Cmnd *SCpnt, struct completion *complete,if (SCpnt->SCp.phase & (resetting|check_condition)) {if (!SCpnt->host_scribble ||SCSEM (SCpnt) ||SCNEXT (SCpnt)) { -printk (ERR_LEAD "cannot reuse command\n" ,CMDINFO (SCpnt)); +scmd_printk (KERN_ERR , SCpnt,"cannot reuse command\n" );return FAILED ; } }else { SCpnt->host_scribble =kmalloc (sizeof (struct aha152x_scdata ),GFP_ATOMIC );if (!SCpnt->host_scribble) { -printk (ERR_LEAD "allocation failed\n" ,CMDINFO (SCpnt)); +scmd_printk (KERN_ERR , SCpnt,"allocation failed\n" );return FAILED ; } }
@@ -1066,15 +989,6 @@ static int aha152x_internal_queue(Scsi_Cmnd *SCpnt, struct completion *complete,*/ static int aha152x_queue_lck (Scsi_Cmnd *SCpnt,void (*done)(Scsi_Cmnd *)) { -#if 0 -if (*SCpnt->cmnd ==REQUEST_SENSE ) { - SCpnt->result =0 ; -done (SCpnt); - -return 0 ; - } -#endif -return aha152x_internal_queue (SCpnt,NULL ,0 , done); }
@@ -1082,15 +996,10 @@ static DEF_SCSI_QCMD(aha152x_queue)/* -* * */ static void reset_done (Scsi_Cmnd *SCpnt) { -#if 0 -struct Scsi_Host *shpnt = SCpnt->host; -DPRINTK (debug_eh,INFO_LEAD "reset_done called\n" ,CMDINFO (SCpnt)); -#endif if (SCSEM (SCpnt)) {complete (SCSEM (SCpnt)); }else {
@@ -1108,20 +1017,11 @@ static int aha152x_abort(Scsi_Cmnd *SCpnt)Scsi_Cmnd *ptr;unsigned long flags; -#if defined(AHA152X_DEBUG ) -if (HOSTDATA (shpnt)->debug & debug_eh) { -printk (DEBUG_LEAD "abort(%p)" ,CMDINFO (SCpnt), SCpnt); -show_queues (shpnt); - } -#endif -DO_LOCK (flags); ptr=remove_SC (&ISSUE_SC , SCpnt);if (ptr) { -DPRINTK (debug_eh,DEBUG_LEAD "not yet issued - SUCCESS\n" ,CMDINFO (SCpnt)); -HOSTDATA (shpnt)->commands--;if (!HOSTDATA (shpnt)->commands)SETPORT (PORTA ,0 );
@@ -1131,7 +1031,7 @@ static int aha152x_abort(Scsi_Cmnd *SCpnt) SCpnt->host_scribble=NULL ;return SUCCESS ; - } + }DO_UNLOCK (flags);
@@ -1142,7 +1042,8 @@ static int aha152x_abort(Scsi_Cmnd *SCpnt)* */ -printk (ERR_LEAD "cannot abort running or disconnected command\n" ,CMDINFO (SCpnt)); +scmd_printk (KERN_ERR , SCpnt, +"cannot abort running or disconnected command\n" );return FAILED ; }
@@ -1160,15 +1061,8 @@ static int aha152x_device_reset(Scsi_Cmnd * SCpnt)unsigned long flags;unsigned long timeleft; -#if defined(AHA152X_DEBUG ) -if (HOSTDATA (shpnt)->debug & debug_eh) { -printk (INFO_LEAD "aha152x_device_reset(%p)" ,CMDINFO (SCpnt), SCpnt); -show_queues (shpnt); - } -#endif -if (CURRENT_SC ==SCpnt) { -printk (ERR_LEAD "cannot reset current device\n" ,CMDINFO (SCpnt)); +scmd_printk (KERN_ERR , SCpnt,"cannot reset current device\n" );return FAILED ; }
@@ -1208,7 +1102,7 @@ static int aha152x_device_reset(Scsi_Cmnd * SCpnt) }else if (disconnected) {append_SC (&DISCONNECTED_SC , SCpnt); } - + ret =FAILED ; }
@@ -1227,12 +1121,12 @@ static void free_hard_reset_SCs(struct Scsi_Host *shpnt, Scsi_Cmnd **SCs)if (SCDATA (ptr)) { next =SCNEXT (ptr); }else { -printk (DEBUG_LEAD "queue corrupted at %p\n" ,CMDINFO (ptr), ptr); +scmd_printk (KERN_DEBUG , ptr, +"queue corrupted at %p\n" , ptr); next =NULL ; }if (!ptr->device->soft_reset) { -DPRINTK (debug_eh,DEBUG_LEAD "disconnected command %p removed\n" ,CMDINFO (ptr), ptr);remove_SC (SCs, ptr);HOSTDATA (shpnt)->commands--;kfree (ptr->host_scribble);
@@ -1253,25 +1147,14 @@ static int aha152x_bus_reset_host(struct Scsi_Host *shpnt)DO_LOCK (flags); -#if defined(AHA152X_DEBUG ) -if (HOSTDATA (shpnt)->debug & debug_eh) { -printk (KERN_DEBUG "scsi%d: bus reset" , shpnt->host_no); -show_queues (shpnt); - } -#endif -free_hard_reset_SCs (shpnt, &ISSUE_SC );free_hard_reset_SCs (shpnt, &DISCONNECTED_SC ); -DPRINTK (debug_eh,KERN_DEBUG "scsi%d: resetting bus\n" , shpnt->host_no); -SETPORT (SCSISEQ ,SCSIRSTO );mdelay (256 );SETPORT (SCSISEQ ,0 );mdelay (DELAY ); -DPRINTK (debug_eh,KERN_DEBUG "scsi%d: bus resetted\n" , shpnt->host_no); -setup_expected_interrupts (shpnt);if (HOSTDATA (shpnt)->commands==0 )SETPORT (PORTA ,0 );
@@ -1333,11 +1216,7 @@ static void reset_ports(struct Scsi_Host *shpnt)*/ int aha152x_host_reset_host (struct Scsi_Host *shpnt) { -DPRINTK (debug_eh,KERN_DEBUG "scsi%d: host reset\n" , shpnt->host_no); -aha152x_bus_reset_host (shpnt); - -DPRINTK (debug_eh,KERN_DEBUG "scsi%d: resetting ports\n" , shpnt->host_no);reset_ports (shpnt);return SUCCESS ;
@@ -1345,7 +1224,7 @@ int aha152x_host_reset_host(struct Scsi_Host *shpnt)/* * Reset the host (bus and controller) -* +* */ static int aha152x_host_reset (Scsi_Cmnd *SCpnt) {
@@ -1411,7 +1290,9 @@ static void done(struct Scsi_Host *shpnt, int error) {if (CURRENT_SC ) {if (DONE_SC ) -printk (ERR_LEAD "there's already a completed command %p - will cause abort\n" ,CMDINFO (CURRENT_SC ),DONE_SC ); +scmd_printk (KERN_ERR ,CURRENT_SC , +"there's already a completed command %p " +"- will cause abort\n" ,DONE_SC );DONE_SC =CURRENT_SC ;CURRENT_SC =NULL ;
@@ -1466,7 +1347,7 @@ static irqreturn_t intr(int irqno, void *dev_id)return IRQ_NONE ;if (TESTLO (DMASTAT ,INTSTAT ) ) -return IRQ_NONE ; +return IRQ_NONE ;/* no more interrupts from the controller, while we're busy. INTEN is restored by the BH handler */
@@ -1501,7 +1382,7 @@ static void busfree_run(struct Scsi_Host *shpnt)SETPORT (SXFRCTL0 ,CH1 );SETPORT (SSTAT1 ,CLRBUSFREE ); - +if (CURRENT_SC ) {#if defined(AHA152X_STAT ) action++;
@@ -1513,19 +1394,13 @@ static void busfree_run(struct Scsi_Host *shpnt)done (shpnt, (CURRENT_SC ->SCp.Status &0xff ) | ((CURRENT_SC ->SCp.Message &0xff ) <<8 ) | (DID_OK <<16 )); }else if (CURRENT_SC ->SCp.phase & aborted) { -DPRINTK (debug_eh,DEBUG_LEAD "ABORT sent\n" ,CMDINFO (CURRENT_SC ));done (shpnt, (CURRENT_SC ->SCp.Status &0xff ) | ((CURRENT_SC ->SCp.Message &0xff ) <<8 ) | (DID_ABORT <<16 )); }else if (CURRENT_SC ->SCp.phase & resetted) { -DPRINTK (debug_eh,DEBUG_LEAD "BUS DEVICE RESET sent\n" ,CMDINFO (CURRENT_SC ));done (shpnt, (CURRENT_SC ->SCp.Status &0xff ) | ((CURRENT_SC ->SCp.Message &0xff ) <<8 ) | (DID_RESET <<16 )); }else if (CURRENT_SC ->SCp.phase & disconnected) {/* target sent DISCONNECT */ -DPRINTK (debug_selection,DEBUG_LEAD "target disconnected at %d/%d\n" , -CMDINFO (CURRENT_SC ), -scsi_get_resid (CURRENT_SC ), -scsi_bufflen (CURRENT_SC ));#if defined(AHA152X_STAT )HOSTDATA (shpnt)->disconnections++;#endif
@@ -1553,13 +1428,6 @@ static void busfree_run(struct Scsi_Host *shpnt)struct scsi_cmnd *cmd =HOSTDATA (shpnt)->done_SC;struct aha152x_scdata *sc =SCDATA (cmd); -#if 0 -if (HOSTDATA (shpnt)->debug & debug_eh) { -printk (ERR_LEAD "received sense: " ,CMDINFO (DONE_SC )); -scsi_print_sense ("bh" ,DONE_SC ); - } -#endif -scsi_eh_restore_cmnd (cmd, &sc->ses); cmd->SCp.Status =SAM_STAT_CHECK_CONDITION ;
@@ -1571,17 +1439,11 @@ static void busfree_run(struct Scsi_Host *shpnt)#if defined(AHA152X_STAT )HOSTDATA (shpnt)->busfree_with_check_condition++;#endif -#if 0 -DPRINTK (debug_eh,ERR_LEAD "CHECK CONDITION found\n" ,CMDINFO (DONE_SC )); -#endif if (!(DONE_SC ->SCp.phase & not_issued)) {struct aha152x_scdata *sc;Scsi_Cmnd *ptr =DONE_SC ;DONE_SC =NULL ; -#if 0 -DPRINTK (debug_eh,ERR_LEAD "requesting sense\n" ,CMDINFO (ptr)); -#endif sc =SCDATA (ptr);/* It was allocated in aha152x_internal_queue? */
@@ -1591,19 +1453,10 @@ static void busfree_run(struct Scsi_Host *shpnt)DO_UNLOCK (flags);aha152x_internal_queue (ptr,NULL , check_condition, ptr->scsi_done);DO_LOCK (flags); -#if 0 - } else { -DPRINTK (debug_eh,ERR_LEAD "command not issued - CHECK CONDITION ignored\n" ,CMDINFO (DONE_SC )); -#endif } }if (DONE_SC &&DONE_SC ->scsi_done) { -#if defined(AHA152X_DEBUG ) -int hostno=DONE_SC ->device->host->host_no; -int id=DONE_SC ->device->id &0xf ; -int lun=((u8 )DONE_SC ->device->lun) &0x7 ; -#endif Scsi_Cmnd *ptr =DONE_SC ;DONE_SC =NULL ;
@@ -1618,9 +1471,7 @@ static void busfree_run(struct Scsi_Host *shpnt) }DO_UNLOCK (flags); -DPRINTK (debug_done,DEBUG_LEAD "calling scsi_done(%p)\n" , hostno, id, lun, ptr); - ptr->scsi_done (ptr); -DPRINTK (debug_done,DEBUG_LEAD "scsi_done(%p) returned\n" , hostno, id, lun, ptr); + ptr->scsi_done (ptr);DO_LOCK (flags); }
@@ -1640,9 +1491,7 @@ static void busfree_run(struct Scsi_Host *shpnt)#if defined(AHA152X_STAT ) action++;#endif -CURRENT_SC ->SCp.phase |= selecting; - -DPRINTK (debug_selection,DEBUG_LEAD "selecting target\n" ,CMDINFO (CURRENT_SC )); +CURRENT_SC ->SCp.phase |= selecting;/* clear selection timeout */ SETPORT (SSTAT1 ,SELTO );
@@ -1674,18 +1523,19 @@ static void seldo_run(struct Scsi_Host *shpnt)SETPORT (SSTAT1 ,CLRBUSFREE );SETPORT (SSTAT1 ,CLRPHASECHG ); -CURRENT_SC ->SCp.phase &= ~(selecting|not_issued); +CURRENT_SC ->SCp.phase &= ~(selecting|not_issued);SETPORT (SCSISEQ ,0 );if (TESTLO (SSTAT0 ,SELDO )) { -printk (ERR_LEAD "aha152x: passing bus free condition\n" ,CMDINFO (CURRENT_SC )); +scmd_printk (KERN_ERR ,CURRENT_SC , +"aha152x: passing bus free condition\n" );done (shpnt,DID_NO_CONNECT <<16 );return ; }SETPORT (SSTAT0 ,CLRSELDO ); - +ADDMSGO (IDENTIFY (RECONNECT ,CURRENT_SC ->device->lun));if (CURRENT_SC ->SCp.phase & aborting) {
@@ -1693,7 +1543,7 @@ static void seldo_run(struct Scsi_Host *shpnt) }else if (CURRENT_SC ->SCp.phase & resetting) {ADDMSGO (BUS_DEVICE_RESET ); }else if (SYNCNEG ==0 &&SYNCHRONOUS ) { -CURRENT_SC ->SCp.phase |= syncneg; +CURRENT_SC ->SCp.phase |= syncneg;MSGOLEN +=spi_populate_sync_msg (&MSGO (MSGOLEN ),50 ,8 );SYNCNEG =1 ;/* negotiation in progress */ }
@@ -1708,29 +1558,21 @@ static void seldo_run(struct Scsi_Host *shpnt)*/ static void selto_run (struct Scsi_Host *shpnt) { -SETPORT (SCSISEQ ,0 ); +SETPORT (SCSISEQ ,0 );SETPORT (SSTAT1 ,CLRSELTIMO ); -DPRINTK (debug_selection,DEBUG_LEAD "selection timeout\n" ,CMDINFO (CURRENT_SC )); - -if (!CURRENT_SC ) { -DPRINTK (debug_selection,DEBUG_LEAD "!CURRENT_SC\n" ,CMDINFO (CURRENT_SC )); +if (!CURRENT_SC )return ; - } -CURRENT_SC ->SCp.phase &= ~selecting; +CURRENT_SC ->SCp.phase &= ~selecting; -if (CURRENT_SC ->SCp.phase & aborted) { -DPRINTK (debug_selection,DEBUG_LEAD "aborted\n" ,CMDINFO (CURRENT_SC )); +if (CURRENT_SC ->SCp.phase & aborted)done (shpnt,DID_ABORT <<16 ); - }else if (TESTLO (SSTAT0 ,SELINGO )) { -DPRINTK (debug_selection,DEBUG_LEAD "arbitration not won\n" ,CMDINFO (CURRENT_SC )); +else if (TESTLO (SSTAT0 ,SELINGO ))done (shpnt,DID_BUS_BUSY <<16 ); - }else { +else /* ARBITRATION won, but SELECTION failed */ -DPRINTK (debug_selection,DEBUG_LEAD "selection failed\n" ,CMDINFO (CURRENT_SC ));done (shpnt,DID_NO_CONNECT <<16 ); - } }/*
@@ -1753,9 +1595,8 @@ static void seldi_run(struct Scsi_Host *shpnt)if (CURRENT_SC ) {if (!(CURRENT_SC ->SCp.phase & not_issued)) -printk (ERR_LEAD "command should not have been issued yet\n" ,CMDINFO (CURRENT_SC )); - -DPRINTK (debug_selection,ERR_LEAD "command requeued - reselection\n" ,CMDINFO (CURRENT_SC )); +scmd_printk (KERN_ERR ,CURRENT_SC , +"command should not have been issued yet\n" );DO_LOCK (flags);append_SC (&ISSUE_SC ,CURRENT_SC );
@@ -1764,17 +1605,16 @@ static void seldi_run(struct Scsi_Host *shpnt)CURRENT_SC =NULL ; } -if (!DISCONNECTED_SC ) { -DPRINTK (debug_selection,DEBUG_LEAD "unexpected SELDI " ,CMDINFO (CURRENT_SC )); +if (!DISCONNECTED_SC )return ; - }RECONN_TARGET =-1 ; selid =GETPORT (SELID ) & ~(1 << shpnt->this_id);if (selid==0 ) { -printk ("aha152x%d: target id unknown (%02x)\n" ,HOSTNO , selid); +shost_printk (KERN_INFO , shpnt, +"target id unknown (%02x)\n" , selid);return ; }
@@ -1782,8 +1622,8 @@ static void seldi_run(struct Scsi_Host *shpnt) ;if (selid & ~(1 << target)) { -printk ("aha152x%d: multiple targets reconnected (%02x)\n" , -HOSTNO , selid); +shost_printk (KERN_INFO , shpnt, +"multiple targets reconnected (%02x)\n" , selid); }
@@ -1793,7 +1633,6 @@ static void seldi_run(struct Scsi_Host *shpnt)SETRATE (HOSTDATA (shpnt)->syncrate[target]);RECONN_TARGET =target; -DPRINTK (debug_selection,DEBUG_LEAD "target %d reselected (%02x).\n" ,CMDINFO (CURRENT_SC ), target, selid); }/*
@@ -1817,31 +1656,24 @@ static void msgi_run(struct Scsi_Host *shpnt)if (sstat1 & (PHASECHG |PHASEMIS |BUSFREE ) || !(sstat1 &REQINIT ))return ; -if (TESTLO (SSTAT0 ,SPIORDY )) { -DPRINTK (debug_msgi,DEBUG_LEAD "!SPIORDY\n" ,CMDINFO (CURRENT_SC )); +if (TESTLO (SSTAT0 ,SPIORDY ))return ; - }ADDMSGI (GETPORT (SCSIDAT )); -#if defined(AHA152X_DEBUG ) -if (HOSTDATA (shpnt)->debug & debug_msgi) { -printk (INFO_LEAD "inbound message %02x " ,CMDINFO (CURRENT_SC ),MSGI (0 )); -spi_print_msg (&MSGI (0 )); -printk ("\n" ); - } -#endif -if (!CURRENT_SC ) {if (LASTSTATE !=seldi) { -printk (KERN_ERR "aha152x%d: message in w/o current command not after reselection\n" ,HOSTNO ); +shost_printk (KERN_ERR , shpnt, +"message in w/o current command" +" not after reselection\n" ); }/* -* Handle reselection -*/ +* Handle reselection +*/ if (!(MSGI (0 ) &IDENTIFY_BASE )) { -printk (KERN_ERR "aha152x%d: target didn't identify after reselection\n" ,HOSTNO ); +shost_printk (KERN_ERR , shpnt, +"target didn't identify after reselection\n" );continue ; }
@@ -1849,12 +1681,13 @@ static void msgi_run(struct Scsi_Host *shpnt)if (!CURRENT_SC ) {show_queues (shpnt); -printk (KERN_ERR "aha152x%d: no disconnected command for target %d/%d\n" ,HOSTNO ,RECONN_TARGET ,MSGI (0 ) &0x3f ); +shost_printk (KERN_ERR , shpnt, +"no disconnected command" +" for target %d/%d\n" , +RECONN_TARGET ,MSGI (0 ) &0x3f );continue ; } -DPRINTK (debug_msgi,DEBUG_LEAD "target reconnected\n" ,CMDINFO (CURRENT_SC )); -CURRENT_SC ->SCp.Message =MSGI (0 );CURRENT_SC ->SCp.phase &= ~disconnected;
@@ -1862,31 +1695,32 @@ static void msgi_run(struct Scsi_Host *shpnt)/* next message if any */ continue ; - } + }CURRENT_SC ->SCp.Message =MSGI (0 );switch (MSGI (0 )) {case DISCONNECT :if (!RECONNECT ) -printk (WARN_LEAD "target was not allowed to disconnect\n" ,CMDINFO (CURRENT_SC )); +scmd_printk (KERN_WARN ,CURRENT_SC , +"target was not allowed to disconnect\n" );CURRENT_SC ->SCp.phase |= disconnected;break ;case COMMAND_COMPLETE : -if (CURRENT_SC ->SCp.phase & completed) -DPRINTK (debug_msgi,DEBUG_LEAD "again COMMAND COMPLETE\n" ,CMDINFO (CURRENT_SC )); -CURRENT_SC ->SCp.phase |= completed;break ;case MESSAGE_REJECT :if (SYNCNEG ==1 ) { -printk (INFO_LEAD "Synchronous Data Transfer Request was rejected\n" ,CMDINFO (CURRENT_SC )); +scmd_printk (KERN_INFO ,CURRENT_SC , +"Synchronous Data Transfer Request" +" was rejected\n" );SYNCNEG =2 ;/* negotiation completed */ }else -printk (INFO_LEAD "inbound message (MESSAGE REJECT)\n" ,CMDINFO (CURRENT_SC )); +scmd_printk (KERN_INFO ,CURRENT_SC , +"inbound message (MESSAGE REJECT)\n" );break ;case SAVE_POINTERS :
@@ -1907,7 +1741,8 @@ static void msgi_run(struct Scsi_Host *shpnt)long ticks;if (MSGI (1 ) !=3 ) { -printk (ERR_LEAD "SDTR message length!=3\n" ,CMDINFO (CURRENT_SC )); +scmd_printk (KERN_ERR ,CURRENT_SC , +"SDTR message length!=3\n" );break ; }
@@ -1924,10 +1759,12 @@ static void msgi_run(struct Scsi_Host *shpnt)/* negotiation in progress */ if (ticks >9 ||MSGI (4 ) <1 ||MSGI (4 ) >8 ) {ADDMSGO (MESSAGE_REJECT ); -printk (INFO_LEAD "received Synchronous Data Transfer Request invalid - rejected\n" ,CMDINFO (CURRENT_SC )); +scmd_printk (KERN_INFO , +CURRENT_SC , +"received Synchronous Data Transfer Request invalid - rejected\n" );break ; } - +SYNCRATE |= ((ticks -2 ) <<4 ) +MSGI (4 ); }else if (ticks <=9 &&MSGI (4 ) >=1 ) {ADDMSGO (EXTENDED_MESSAGE );
@@ -1947,11 +1784,14 @@ static void msgi_run(struct Scsi_Host *shpnt)SYNCRATE |= ((ticks -2 ) <<4 ) +MSGI (4 ); }else {/* requested SDTR is too slow, do it asynchronously */ -printk (INFO_LEAD "Synchronous Data Transfer Request too slow - Rejecting\n" ,CMDINFO (CURRENT_SC )); +scmd_printk (KERN_INFO , +CURRENT_SC , +"Synchronous Data Transfer Request too slow - Rejecting\n" );ADDMSGO (MESSAGE_REJECT ); } -SYNCNEG =2 ;/* negotiation completed */ +/* negotiation completed */ +SYNCNEG =2 ;SETRATE (SYNCRATE ); }break ;
@@ -1985,12 +1825,12 @@ static void msgi_run(struct Scsi_Host *shpnt)static void msgi_end (struct Scsi_Host *shpnt) {if (MSGILEN >0 ) -printk (WARN_LEAD "target left before message completed (%d)\n" ,CMDINFO (CURRENT_SC ),MSGILEN ); +scmd_printk (KERN_WARN ,CURRENT_SC , +"target left before message completed (%d)\n" , +MSGILEN ); -if (MSGOLEN >0 && !(GETPORT (SSTAT1 ) &BUSFREE )) { -DPRINTK (debug_msgi,DEBUG_LEAD "msgo pending\n" ,CMDINFO (CURRENT_SC )); +if (MSGOLEN >0 && !(GETPORT (SSTAT1 ) &BUSFREE ))SETPORT (SCSISIG ,P_MSGI |SIG_ATNO ); - } }/*
@@ -2003,21 +1843,12 @@ static void msgo_init(struct Scsi_Host *shpnt)if ((CURRENT_SC ->SCp.phase & syncneg) &&SYNCNEG ==2 &&SYNCRATE ==0 ) {ADDMSGO (IDENTIFY (RECONNECT ,CURRENT_SC ->device->lun)); }else { -printk (INFO_LEAD "unexpected MESSAGE OUT phase; rejecting\n" ,CMDINFO (CURRENT_SC )); +scmd_printk (KERN_INFO ,CURRENT_SC , +"unexpected MESSAGE OUT phase; rejecting\n" );ADDMSGO (MESSAGE_REJECT ); } } -#if defined(AHA152X_DEBUG ) -if (HOSTDATA (shpnt)->debug & debug_msgo) { -int i; - -printk (DEBUG_LEAD "messages( " ,CMDINFO (CURRENT_SC )); -for (i=0 ; i<MSGOLEN ; i+=spi_print_msg (&MSGO (i)),printk (" " )) - ; -printk (")\n" ); - } -#endif }/*
@@ -2026,16 +1857,9 @@ static void msgo_init(struct Scsi_Host *shpnt)*/ static void msgo_run (struct Scsi_Host *shpnt) { -if (MSGO_I ==MSGOLEN ) -DPRINTK (debug_msgo,DEBUG_LEAD "messages all sent (%d/%d)\n" ,CMDINFO (CURRENT_SC ),MSGO_I ,MSGOLEN ); -while (MSGO_I <MSGOLEN ) { -DPRINTK (debug_msgo,DEBUG_LEAD "message byte %02x (%d/%d)\n" ,CMDINFO (CURRENT_SC ),MSGO (MSGO_I ),MSGO_I ,MSGOLEN ); - -if (TESTLO (SSTAT0 ,SPIORDY )) { -DPRINTK (debug_msgo,DEBUG_LEAD "!SPIORDY\n" ,CMDINFO (CURRENT_SC )); +if (TESTLO (SSTAT0 ,SPIORDY ))return ; - }if (MSGO_I ==MSGOLEN -1 ) {/* Leave MESSAGE OUT after transfer */
@@ -2059,36 +1883,33 @@ static void msgo_run(struct Scsi_Host *shpnt)static void msgo_end (struct Scsi_Host *shpnt) {if (MSGO_I <MSGOLEN ) { -printk (ERR_LEAD "message sent incompletely (%d/%d)\n" ,CMDINFO (CURRENT_SC ),MSGO_I ,MSGOLEN ); +scmd_printk (KERN_ERR ,CURRENT_SC , +"message sent incompletely (%d/%d)\n" , +MSGO_I ,MSGOLEN );if (SYNCNEG ==1 ) { -printk (INFO_LEAD "Synchronous Data Transfer Request was rejected\n" ,CMDINFO (CURRENT_SC )); +scmd_printk (KERN_INFO ,CURRENT_SC , +"Synchronous Data Transfer Request was rejected\n" );SYNCNEG =2 ; } } - +MSGO_I =0 ;MSGOLEN =0 ; } -/* +/* * command phase * */ static void cmd_init (struct Scsi_Host *shpnt) {if (CURRENT_SC ->SCp.sent_command) { -printk (ERR_LEAD "command already sent\n" ,CMDINFO (CURRENT_SC )); +scmd_printk (KERN_ERR ,CURRENT_SC , +"command already sent\n" );done (shpnt,DID_ERROR <<16 );return ; } -#if defined(AHA152X_DEBUG ) -if (HOSTDATA (shpnt)->debug & debug_cmd) { -printk (DEBUG_LEAD "cmd_init: " ,CMDINFO (CURRENT_SC )); -__scsi_print_command (CURRENT_SC ->cmnd); - } -#endif -CMD_I =0 ; }
@@ -2098,18 +1919,9 @@ static void cmd_init(struct Scsi_Host *shpnt)*/ static void cmd_run (struct Scsi_Host *shpnt) { -if (CMD_I ==CURRENT_SC ->cmd_len) { -DPRINTK (debug_cmd,DEBUG_LEAD "command already completely sent (%d/%d)" ,CMDINFO (CURRENT_SC ),CMD_I ,CURRENT_SC ->cmd_len); -disp_ports (shpnt); - } -while (CMD_I <CURRENT_SC ->cmd_len) { -DPRINTK (debug_cmd,DEBUG_LEAD "command byte %02x (%d/%d)\n" ,CMDINFO (CURRENT_SC ),CURRENT_SC ->cmnd[CMD_I ],CMD_I ,CURRENT_SC ->cmd_len); - -if (TESTLO (SSTAT0 ,SPIORDY )) { -DPRINTK (debug_cmd,DEBUG_LEAD "!SPIORDY\n" ,CMDINFO (CURRENT_SC )); +if (TESTLO (SSTAT0 ,SPIORDY ))return ; - }SETPORT (SCSIDAT ,CURRENT_SC ->cmnd[CMD_I ++]); }
@@ -2118,7 +1930,9 @@ static void cmd_run(struct Scsi_Host *shpnt)static void cmd_end (struct Scsi_Host *shpnt) {if (CMD_I <CURRENT_SC ->cmd_len) -printk (ERR_LEAD "command sent incompletely (%d/%d)\n" ,CMDINFO (CURRENT_SC ),CMD_I ,CURRENT_SC ->cmd_len); +scmd_printk (KERN_ERR ,CURRENT_SC , +"command sent incompletely (%d/%d)\n" , +CMD_I ,CURRENT_SC ->cmd_len);else CURRENT_SC ->SCp.sent_command++; }
@@ -2129,20 +1943,11 @@ static void cmd_end(struct Scsi_Host *shpnt)*/ static void status_run (struct Scsi_Host *shpnt) { -if (TESTLO (SSTAT0 ,SPIORDY )) { -DPRINTK (debug_status,DEBUG_LEAD "!SPIORDY\n" ,CMDINFO (CURRENT_SC )); +if (TESTLO (SSTAT0 ,SPIORDY ))return ; - }CURRENT_SC ->SCp.Status =GETPORT (SCSIDAT ); -#if defined(AHA152X_DEBUG ) -if (HOSTDATA (shpnt)->debug & debug_status) { -printk (DEBUG_LEAD "inbound status %02x " ,CMDINFO (CURRENT_SC ),CURRENT_SC ->SCp.Status); -scsi_print_status (CURRENT_SC ->SCp.Status); -printk ("\n" ); - } -#endif }/*
@@ -2161,10 +1966,6 @@ static void datai_init(struct Scsi_Host *shpnt)SETPORT (SIMODE1 ,ENSCSIPERR |ENSCSIRST |ENPHASEMIS |ENBUSFREE );DATA_LEN =0 ; -DPRINTK (debug_datai, -DEBUG_LEAD "datai_init: request_bufflen=%d resid=%d\n" , -CMDINFO (CURRENT_SC ),scsi_bufflen (CURRENT_SC ), -scsi_get_resid (CURRENT_SC )); }static void datai_run (struct Scsi_Host *shpnt)
@@ -2186,8 +1987,7 @@ static void datai_run(struct Scsi_Host *shpnt)barrier ();if (TESTLO (DMASTAT ,DFIFOFULL |INTSTAT )) { -printk (ERR_LEAD "datai timeout" ,CMDINFO (CURRENT_SC )); -disp_ports (shpnt); +scmd_printk (KERN_ERR ,CURRENT_SC ,"datai timeout\n" );break ; }
@@ -2199,8 +1999,8 @@ static void datai_run(struct Scsi_Host *shpnt)barrier ();if (TESTLO (SSTAT2 ,SEMPTY )) { -printk (ERR_LEAD "datai sempty timeout" ,CMDINFO (CURRENT_SC )); -disp_ports (shpnt); +scmd_printk (KERN_ERR ,CURRENT_SC , +"datai sempty timeout" );break ; }
@@ -2209,48 +2009,49 @@ static void datai_run(struct Scsi_Host *shpnt)if (CURRENT_SC ->SCp.this_residual>0 ) {while (fifodata>0 &&CURRENT_SC ->SCp.this_residual>0 ) { - data_count = fifodata>CURRENT_SC ->SCp.this_residual ? + data_count = fifodata >CURRENT_SC ->SCp.this_residual ?CURRENT_SC ->SCp.this_residual : fifodata; fifodata -= data_count; -if (data_count &1 ) { -DPRINTK (debug_datai,DEBUG_LEAD "8bit\n" ,CMDINFO (CURRENT_SC )); -SETPORT (DMACNTRL0 ,ENDMA |_8BIT); - *CURRENT_SC ->SCp.ptr++ =GETPORT (DATAPORT ); -CURRENT_SC ->SCp.this_residual--; -DATA_LEN ++; -SETPORT (DMACNTRL0 ,ENDMA ); - } - -if (data_count >1 ) { -DPRINTK (debug_datai,DEBUG_LEAD "16bit(%d)\n" ,CMDINFO (CURRENT_SC ), data_count); - data_count >>=1 ; -insw (DATAPORT ,CURRENT_SC ->SCp.ptr, data_count); -CURRENT_SC ->SCp.ptr +=2 * data_count; -CURRENT_SC ->SCp.this_residual -=2 * data_count; -DATA_LEN +=2 * data_count; - } - -if (CURRENT_SC ->SCp.this_residual==0 &&CURRENT_SC ->SCp.buffers_residual>0 ) { -/* advance to next buffer */ -CURRENT_SC ->SCp.buffers_residual--; -CURRENT_SC ->SCp.buffer++; -CURRENT_SC ->SCp.ptr =SG_ADDRESS (CURRENT_SC ->SCp.buffer); -CURRENT_SC ->SCp.this_residual =CURRENT_SC ->SCp.buffer->length; - } - } - }else if (fifodata>0 ) { -printk (ERR_LEAD "no buffers left for %d(%d) bytes (data overrun!?)\n" ,CMDINFO (CURRENT_SC ), fifodata,GETPORT (FIFOSTAT )); -SETPORT (DMACNTRL0 ,ENDMA |_8BIT); +if (data_count &1 ) { +SETPORT (DMACNTRL0 ,ENDMA |_8BIT); + *CURRENT_SC ->SCp.ptr++ =GETPORT (DATAPORT ); +CURRENT_SC ->SCp.this_residual--; +DATA_LEN ++; +SETPORT (DMACNTRL0 ,ENDMA ); + } + +if (data_count >1 ) { + data_count >>=1 ; +insw (DATAPORT ,CURRENT_SC ->SCp.ptr, data_count); +CURRENT_SC ->SCp.ptr +=2 * data_count; +CURRENT_SC ->SCp.this_residual -=2 * data_count; +DATA_LEN +=2 * data_count; + } + +if (CURRENT_SC ->SCp.this_residual ==0 && +CURRENT_SC ->SCp.buffers_residual >0 ) { +/* advance to next buffer */ +CURRENT_SC ->SCp.buffers_residual--; +CURRENT_SC ->SCp.buffer++; +CURRENT_SC ->SCp.ptr =SG_ADDRESS (CURRENT_SC ->SCp.buffer); +CURRENT_SC ->SCp.this_residual =CURRENT_SC ->SCp.buffer->length; + } + } + }else if (fifodata >0 ) { +scmd_printk (KERN_ERR ,CURRENT_SC , +"no buffers left for %d(%d) bytes" +" (data overrun!?)\n" , + fifodata,GETPORT (FIFOSTAT )); +SETPORT (DMACNTRL0 ,ENDMA |_8BIT);while (fifodata>0 ) {int data; data=GETPORT (DATAPORT ); -DPRINTK (debug_datai,DEBUG_LEAD "data=%02x\n" ,CMDINFO (CURRENT_SC ), data); fifodata--;DATA_LEN ++; } -SETPORT (DMACNTRL0 ,ENDMA |_8BIT); +SETPORT (DMACNTRL0 ,ENDMA |_8BIT); } }
@@ -2258,19 +2059,20 @@ static void datai_run(struct Scsi_Host *shpnt) TESTLO(DMASTAT, DFIFOEMP) || TESTLO(SSTAT2, SEMPTY) || GETPORT(FIFOSTAT)>0) { -/* +/* * something went wrong, if there's something left in the fifos * or the phase didn't change */ -printk (ERR_LEAD "fifos should be empty and phase should have changed\n" ,CMDINFO (CURRENT_SC )); -disp_ports (shpnt); +scmd_printk (KERN_ERR ,CURRENT_SC , +"fifos should be empty and phase should have changed\n" ); }if (DATA_LEN !=GETSTCNT ()) { -printk (ERR_LEAD -"manual transfer count differs from automatic (count=%d;stcnt=%d;diff=%d;fifostat=%d)" , -CMDINFO (CURRENT_SC ),DATA_LEN ,GETSTCNT (),GETSTCNT ()-DATA_LEN ,GETPORT (FIFOSTAT )); -disp_ports (shpnt); +scmd_printk (KERN_ERR ,CURRENT_SC , +"manual transfer count differs from automatic " +"(count=%d;stcnt=%d;diff=%d;fifostat=%d)" , +DATA_LEN ,GETSTCNT (),GETSTCNT ()-DATA_LEN , +GETPORT (FIFOSTAT ));mdelay (10000 ); } }
@@ -2279,11 +2081,6 @@ static void datai_end(struct Scsi_Host *shpnt) {CMD_INC_RESID (CURRENT_SC , -GETSTCNT ()); -DPRINTK (debug_datai, -DEBUG_LEAD "datai_end: request_bufflen=%d resid=%d stcnt=%d\n" , -CMDINFO (CURRENT_SC ),scsi_bufflen (CURRENT_SC ), -scsi_get_resid (CURRENT_SC ),GETSTCNT ()); -SETPORT (SXFRCTL0 ,CH1 |CLRSTCNT );SETPORT (DMACNTRL0 ,0 ); }
@@ -2304,11 +2101,6 @@ static void datao_init(struct Scsi_Host *shpnt)SETPORT (SIMODE1 ,ENSCSIPERR |ENSCSIRST |ENPHASEMIS |ENBUSFREE );DATA_LEN =scsi_get_resid (CURRENT_SC ); - -DPRINTK (debug_datao, -DEBUG_LEAD "datao_init: request_bufflen=%d; resid=%d\n" , -CMDINFO (CURRENT_SC ),scsi_bufflen (CURRENT_SC ), -scsi_get_resid (CURRENT_SC )); }static void datao_run (struct Scsi_Host *shpnt)
@@ -2323,8 +2115,9 @@ static void datao_run(struct Scsi_Host *shpnt) data_count=CURRENT_SC ->SCp.this_residual;if (TESTLO (DMASTAT ,DFIFOEMP )) { -printk (ERR_LEAD "datao fifo not empty (%d)" ,CMDINFO (CURRENT_SC ),GETPORT (FIFOSTAT )); -disp_ports (shpnt); +scmd_printk (KERN_ERR ,CURRENT_SC , +"datao fifo not empty (%d)" , +GETPORT (FIFOSTAT ));break ; }
@@ -2342,7 +2135,7 @@ static void datao_run(struct Scsi_Host *shpnt)CURRENT_SC ->SCp.ptr +=2 * data_count;CURRENT_SC ->SCp.this_residual -=2 * data_count;CMD_INC_RESID (CURRENT_SC ,-2 * data_count); - } + }if (CURRENT_SC ->SCp.this_residual==0 &&CURRENT_SC ->SCp.buffers_residual>0 ) {/* advance to next buffer */
@@ -2357,8 +2150,7 @@ static void datao_run(struct Scsi_Host *shpnt)barrier ();if (TESTLO (DMASTAT ,DFIFOEMP |INTSTAT )) { -printk (ERR_LEAD "dataout timeout" ,CMDINFO (CURRENT_SC )); -disp_ports (shpnt); +scmd_printk (KERN_ERR ,CURRENT_SC ,"dataout timeout\n" );break ; } }
@@ -2368,35 +2160,23 @@ static void datao_end(struct Scsi_Host *shpnt) {if (TESTLO (DMASTAT ,DFIFOEMP )) {int data_count = (DATA_LEN -scsi_get_resid (CURRENT_SC )) - -GETSTCNT (); - -DPRINTK (debug_datao,DEBUG_LEAD "datao: %d bytes to resend (%d written, %d transferred)\n" , -CMDINFO (CURRENT_SC ), - data_count, -DATA_LEN -scsi_get_resid (CURRENT_SC ), -GETSTCNT ()); +GETSTCNT ();CMD_INC_RESID (CURRENT_SC , data_count); data_count -=CURRENT_SC ->SCp.ptr - -SG_ADDRESS (CURRENT_SC ->SCp.buffer); +SG_ADDRESS (CURRENT_SC ->SCp.buffer);while (data_count>0 ) {CURRENT_SC ->SCp.buffer--;CURRENT_SC ->SCp.buffers_residual++; data_count -=CURRENT_SC ->SCp.buffer->length; }CURRENT_SC ->SCp.ptr =SG_ADDRESS (CURRENT_SC ->SCp.buffer) - - data_count; + data_count;CURRENT_SC ->SCp.this_residual =CURRENT_SC ->SCp.buffer->length + - data_count; + data_count; } -DPRINTK (debug_datao,DEBUG_LEAD "datao_end: request_bufflen=%d; resid=%d; stcnt=%d\n" , -CMDINFO (CURRENT_SC ), -scsi_bufflen (CURRENT_SC ), -scsi_get_resid (CURRENT_SC ), -GETSTCNT ()); -SETPORT (SXFRCTL0 ,CH1 |CLRCH1 |CLRSTCNT );SETPORT (SXFRCTL0 ,CH1 );
@@ -2420,7 +2200,7 @@ static int update_state(struct Scsi_Host *shpnt)STATE =rsti;SETPORT (SCSISEQ ,0 );SETPORT (SSTAT1 ,SCSIRSTI ); - }else if (stat0 &SELDI &&PREVSTATE ==busfree) { + }else if (stat0 &SELDI &&PREVSTATE == busfree) {STATE =seldi; }else if (stat0 &SELDO &&CURRENT_SC && (CURRENT_SC ->SCp.phase & selecting)) {STATE =seldo;
@@ -2445,8 +2225,7 @@ static int update_state(struct Scsi_Host *shpnt) }if ((stat0 &SELDI ) &&STATE !=seldi && !dataphase) { -printk (INFO_LEAD "reselection missed?" ,CMDINFO (CURRENT_SC )); -disp_ports (shpnt); +scmd_printk (KERN_INFO ,CURRENT_SC ,"reselection missed?" ); }if (STATE !=PREVSTATE ) {
@@ -2464,7 +2243,7 @@ static int update_state(struct Scsi_Host *shpnt)*/ static void parerr_run (struct Scsi_Host *shpnt) { -printk (ERR_LEAD "parity error\n" ,CMDINFO (CURRENT_SC )); +scmd_printk (KERN_ERR ,CURRENT_SC ,"parity error\n" );done (shpnt,DID_PARITY <<16 ); }
@@ -2476,8 +2255,8 @@ static void rsti_run(struct Scsi_Host *shpnt) {Scsi_Cmnd *ptr; -printk (KERN_NOTICE "aha152x%d: scsi reset in\n" ,HOSTNO ); - +shost_printk (KERN_NOTICE , shpnt,"scsi reset in\n" ); + ptr=DISCONNECTED_SC ;while (ptr) {Scsi_Cmnd *next =SCNEXT (ptr);
@@ -2539,8 +2318,6 @@ static void is_complete(struct Scsi_Host *shpnt) dataphase=update_state (shpnt); -DPRINTK (debug_phases,LEAD "start %s %s(%s)\n" ,CMDINFO (CURRENT_SC ), states[STATE ].name, states[PREVSTATE ].name, states[LASTSTATE ].name); -/* * end previous state *
@@ -2567,9 +2344,9 @@ static void is_complete(struct Scsi_Host *shpnt)if (dataphase) {SETPORT (SSTAT0 ,REQINIT );SETPORT (SCSISIG ,GETPORT (SCSISIG ) &P_MASK ); -SETPORT (SSTAT1 ,PHASECHG ); +SETPORT (SSTAT1 ,PHASECHG ); } - +/* * enable SPIO mode if previous didn't use it * and this one does
@@ -2581,14 +2358,14 @@ static void is_complete(struct Scsi_Host *shpnt)if (CURRENT_SC )CURRENT_SC ->SCp.phase |= spiordy; } - +/* * initialize for new state * */ if (PREVSTATE !=STATE && states[STATE ].init) states[STATE ].init (shpnt); - +/* * handle current state *
@@ -2596,8 +2373,9 @@ static void is_complete(struct Scsi_Host *shpnt)if (states[STATE ].run) states[STATE ].run (shpnt);else -printk (ERR_LEAD "unexpected state (%x)\n" ,CMDINFO (CURRENT_SC ),STATE ); - +scmd_printk (KERN_ERR ,CURRENT_SC , +"unexpected state (%x)\n" ,STATE ); +/* * setup controller to interrupt on * the next expected condition and
@@ -2613,7 +2391,6 @@ static void is_complete(struct Scsi_Host *shpnt)HOSTDATA (shpnt)->time[STATE ] += jiffies-start;#endif -DPRINTK (debug_phases,LEAD "end %s %s(%s)\n" ,CMDINFO (CURRENT_SC ), states[STATE ].name, states[PREVSTATE ].name, states[LASTSTATE ].name); }while (pending);/*
@@ -2626,289 +2403,42 @@ static void is_complete(struct Scsi_Host *shpnt) } -/* +/* * Dump the current driver status and panic */ static void aha152x_error (struct Scsi_Host *shpnt,char *msg) { -printk (KERN_EMERG "\naha152x%d: %s\n" ,HOSTNO , msg); +shost_printk (KERN_EMERG , shpnt,"%s\n" , msg);show_queues (shpnt);panic ("aha152x panic\n" ); }/* -* Display registers of AIC-6260 -*/ -static void disp_ports (struct Scsi_Host *shpnt) -{ -#if defined(AHA152X_DEBUG ) -int s; - -printk ("\n%s: %s(%s) " , -CURRENT_SC ?"busy" :"waiting" , - states[STATE ].name, - states[PREVSTATE ].name); - - s =GETPORT (SCSISEQ ); -printk ("SCSISEQ( " ); -if (s &TEMODEO ) -printk ("TARGET MODE " ); -if (s &ENSELO ) -printk ("SELO " ); -if (s &ENSELI ) -printk ("SELI " ); -if (s &ENRESELI ) -printk ("RESELI " ); -if (s &ENAUTOATNO ) -printk ("AUTOATNO " ); -if (s &ENAUTOATNI ) -printk ("AUTOATNI " ); -if (s &ENAUTOATNP ) -printk ("AUTOATNP " ); -if (s &SCSIRSTO ) -printk ("SCSIRSTO " ); -printk (");" ); - -printk (" SCSISIG(" ); - s =GETPORT (SCSISIG ); -switch (s &P_MASK ) { -case P_DATAO : -printk ("DATA OUT" ); -break ; -case P_DATAI : -printk ("DATA IN" ); -break ; -case P_CMD : -printk ("COMMAND" ); -break ; -case P_STATUS : -printk ("STATUS" ); -break ; -case P_MSGO : -printk ("MESSAGE OUT" ); -break ; -case P_MSGI : -printk ("MESSAGE IN" ); -break ; -default : -printk ("*invalid*" ); -break ; - } - -printk ("); " ); - -printk ("INTSTAT (%s); " ,TESTHI (DMASTAT ,INTSTAT ) ?"hi" :"lo" ); - -printk ("SSTAT( " ); - s =GETPORT (SSTAT0 ); -if (s &TARGET ) -printk ("TARGET " ); -if (s &SELDO ) -printk ("SELDO " ); -if (s &SELDI ) -printk ("SELDI " ); -if (s &SELINGO ) -printk ("SELINGO " ); -if (s &SWRAP ) -printk ("SWRAP " ); -if (s &SDONE ) -printk ("SDONE " ); -if (s &SPIORDY ) -printk ("SPIORDY " ); -if (s &DMADONE ) -printk ("DMADONE " ); - - s =GETPORT (SSTAT1 ); -if (s &SELTO ) -printk ("SELTO " ); -if (s &ATNTARG ) -printk ("ATNTARG " ); -if (s &SCSIRSTI ) -printk ("SCSIRSTI " ); -if (s &PHASEMIS ) -printk ("PHASEMIS " ); -if (s &BUSFREE ) -printk ("BUSFREE " ); -if (s &SCSIPERR ) -printk ("SCSIPERR " ); -if (s &PHASECHG ) -printk ("PHASECHG " ); -if (s &REQINIT ) -printk ("REQINIT " ); -printk ("); " ); - - -printk ("SSTAT( " ); - - s =GETPORT (SSTAT0 ) &GETPORT (SIMODE0 ); - -if (s &TARGET ) -printk ("TARGET " ); -if (s &SELDO ) -printk ("SELDO " ); -if (s &SELDI ) -printk ("SELDI " ); -if (s &SELINGO ) -printk ("SELINGO " ); -if (s &SWRAP ) -printk ("SWRAP " ); -if (s &SDONE ) -printk ("SDONE " ); -if (s &SPIORDY ) -printk ("SPIORDY " ); -if (s &DMADONE ) -printk ("DMADONE " ); - - s =GETPORT (SSTAT1 ) &GETPORT (SIMODE1 ); - -if (s &SELTO ) -printk ("SELTO " ); -if (s &ATNTARG ) -printk ("ATNTARG " ); -if (s &SCSIRSTI ) -printk ("SCSIRSTI " ); -if (s &PHASEMIS ) -printk ("PHASEMIS " ); -if (s &BUSFREE ) -printk ("BUSFREE " ); -if (s &SCSIPERR ) -printk ("SCSIPERR " ); -if (s &PHASECHG ) -printk ("PHASECHG " ); -if (s &REQINIT ) -printk ("REQINIT " ); -printk ("); " ); - -printk ("SXFRCTL0( " ); - - s =GETPORT (SXFRCTL0 ); -if (s &SCSIEN ) -printk ("SCSIEN " ); -if (s &DMAEN ) -printk ("DMAEN " ); -if (s &CH1 ) -printk ("CH1 " ); -if (s &CLRSTCNT ) -printk ("CLRSTCNT " ); -if (s &SPIOEN ) -printk ("SPIOEN " ); -if (s &CLRCH1 ) -printk ("CLRCH1 " ); -printk ("); " ); - -printk ("SIGNAL( " ); - - s =GETPORT (SCSISIG ); -if (s &SIG_ATNI ) -printk ("ATNI " ); -if (s &SIG_SELI ) -printk ("SELI " ); -if (s &SIG_BSYI ) -printk ("BSYI " ); -if (s &SIG_REQI ) -printk ("REQI " ); -if (s &SIG_ACKI ) -printk ("ACKI " ); -printk ("); " ); - -printk ("SELID (%02x), " ,GETPORT (SELID )); - -printk ("STCNT (%d), " ,GETSTCNT ()); - -printk ("SSTAT2( " ); - - s =GETPORT (SSTAT2 ); -if (s &SOFFSET ) -printk ("SOFFSET " ); -if (s &SEMPTY ) -printk ("SEMPTY " ); -if (s &SFULL ) -printk ("SFULL " ); -printk ("); SFCNT (%d); " , s & (SFULL |SFCNT )); - - s =GETPORT (SSTAT3 ); -printk ("SCSICNT (%d), OFFCNT(%d), " , (s &0xf0 ) >>4 , s &0x0f ); - -printk ("SSTAT4( " ); - s =GETPORT (SSTAT4 ); -if (s &SYNCERR ) -printk ("SYNCERR " ); -if (s &FWERR ) -printk ("FWERR " ); -if (s &FRERR ) -printk ("FRERR " ); -printk ("); " ); - -printk ("DMACNTRL0( " ); - s =GETPORT (DMACNTRL0 ); -printk ("%s " , s & _8BIT ?"8BIT" :"16BIT" ); -printk ("%s " , s &DMA ?"DMA" :"PIO" ); -printk ("%s " , s &WRITE_READ ?"WRITE" :"READ" ); -if (s &ENDMA ) -printk ("ENDMA " ); -if (s &INTEN ) -printk ("INTEN " ); -if (s &RSTFIFO ) -printk ("RSTFIFO " ); -if (s &SWINT ) -printk ("SWINT " ); -printk ("); " ); - -printk ("DMASTAT( " ); - s =GETPORT (DMASTAT ); -if (s &ATDONE ) -printk ("ATDONE " ); -if (s &WORDRDY ) -printk ("WORDRDY " ); -if (s &DFIFOFULL ) -printk ("DFIFOFULL " ); -if (s &DFIFOEMP ) -printk ("DFIFOEMP " ); -printk (")\n" ); -#endif -} - -/* * display enabled interrupts */ static void disp_enintr (struct Scsi_Host *shpnt) { -int s; - -printk (KERN_DEBUG "enabled interrupts ( " ); - - s =GETPORT (SIMODE0 ); -if (s &ENSELDO ) -printk ("ENSELDO " ); -if (s &ENSELDI ) -printk ("ENSELDI " ); -if (s &ENSELINGO ) -printk ("ENSELINGO " ); -if (s &ENSWRAP ) -printk ("ENSWRAP " ); -if (s &ENSDONE ) -printk ("ENSDONE " ); -if (s &ENSPIORDY ) -printk ("ENSPIORDY " ); -if (s &ENDMADONE ) -printk ("ENDMADONE " ); - - s =GETPORT (SIMODE1 ); -if (s &ENSELTIMO ) -printk ("ENSELTIMO " ); -if (s &ENATNTARG ) -printk ("ENATNTARG " ); -if (s &ENPHASEMIS ) -printk ("ENPHASEMIS " ); -if (s &ENBUSFREE ) -printk ("ENBUSFREE " ); -if (s &ENSCSIPERR ) -printk ("ENSCSIPERR " ); -if (s &ENPHASECHG ) -printk ("ENPHASECHG " ); -if (s &ENREQINIT ) -printk ("ENREQINIT " ); -printk (")\n" ); +int s0, s1; + + s0 =GETPORT (SIMODE0 ); + s1 =GETPORT (SIMODE1 ); + +shost_printk (KERN_DEBUG , shpnt, +"enabled interrupts (%s%s%s%s%s%s%s%s%s%s%s%s%s%s)\n" , + (s0 &ENSELDO ) ?"ENSELDO " :"" , + (s0 &ENSELDI ) ?"ENSELDI " :"" , + (s0 &ENSELINGO ) ?"ENSELINGO " :"" , + (s0 &ENSWRAP ) ?"ENSWRAP " :"" , + (s0 &ENSDONE ) ?"ENSDONE " :"" , + (s0 &ENSPIORDY ) ?"ENSPIORDY " :"" , + (s0 &ENDMADONE ) ?"ENDMADONE " :"" , + (s1 &ENSELTIMO ) ?"ENSELTIMO " :"" , + (s1 &ENATNTARG ) ?"ENATNTARG " :"" , + (s1 &ENPHASEMIS ) ?"ENPHASEMIS " :"" , + (s1 &ENBUSFREE ) ?"ENBUSFREE " :"" , + (s1 &ENSCSIPERR ) ?"ENSCSIPERR " :"" , + (s1 &ENPHASECHG ) ?"ENPHASECHG " :"" , + (s1 &ENREQINIT ) ?"ENREQINIT " :"" ); }/*
@@ -2916,36 +2446,21 @@ static void disp_enintr(struct Scsi_Host *shpnt)*/ static void show_command (Scsi_Cmnd *ptr) { -scmd_printk (KERN_DEBUG , ptr,"%p: cmnd=(" , ptr); - -__scsi_print_command (ptr->cmnd); - -printk (KERN_DEBUG "); request_bufflen=%d; resid=%d; phase |" , -scsi_bufflen (ptr),scsi_get_resid (ptr)); - -if (ptr->SCp.phase & not_issued) -printk ("not issued|" ); -if (ptr->SCp.phase & selecting) -printk ("selecting|" ); -if (ptr->SCp.phase & identified) -printk ("identified|" ); -if (ptr->SCp.phase & disconnected) -printk ("disconnected|" ); -if (ptr->SCp.phase & completed) -printk ("completed|" ); -if (ptr->SCp.phase & spiordy) -printk ("spiordy|" ); -if (ptr->SCp.phase & syncneg) -printk ("syncneg|" ); -if (ptr->SCp.phase & aborted) -printk ("aborted|" ); -if (ptr->SCp.phase & resetted) -printk ("resetted|" ); -if (SCDATA (ptr) ) { -printk ("; next=0x%p\n" ,SCNEXT (ptr)); - }else { -printk ("; next=(host scribble NULL)\n" ); - } +scsi_print_comment (ptr); +scmd_printk (KERN_DEBUG , ptr, +"request_bufflen=%d; resid=%d; " +"phase |%s%s%s%s%s%s%s%s%s%s; next=0x%p" , +scsi_bufflen (ptr),scsi_get_resid (ptr), + (ptr->SCp.phase & not_issued) ?"not issued|" :"" , + (ptr->SCp.phase & selecting) ?"selecting|" :"" , + (ptr->SCp.phase & identified) ?"identified|" :"" , + (ptr->SCp.phase & disconnected) ?"disconnected|" :"" , + (ptr->SCp.phase & completed) ?"completed|" :"" , + (ptr->SCp.phase & spiordy) ?"spiordy|" :"" , + (ptr->SCp.phase & syncneg) ?"syncneg|" :"" , + (ptr->SCp.phase & aborted) ?"aborted|" :"" , + (ptr->SCp.phase & resetted) ?"resetted|" :"" , + (SCDATA (ptr)) ?SCNEXT (ptr) :NULL ); }/*
@@ -2972,7 +2487,6 @@ static void show_queues(struct Scsi_Host *shpnt)for (ptr =DISCONNECTED_SC ; ptr; ptr =SCDATA (ptr) ?SCNEXT (ptr) :NULL )show_command (ptr); -disp_ports (shpnt);disp_enintr (shpnt); }
@@ -3276,15 +2790,6 @@ static int aha152x_set_info(struct Scsi_Host *shpnt, char *buffer, int length)if (!shpnt || !buffer || length<8 ||strncmp ("aha152x " , buffer,8 )!=0 )return -EINVAL ; -#if defined(AHA152X_DEBUG ) -if (length>14 &&strncmp ("debug " , buffer+8 ,6 )==0 ) { -int debug =HOSTDATA (shpnt)->debug; - -HOSTDATA (shpnt)->debug =simple_strtoul (buffer+14 ,NULL ,0 ); - -printk (KERN_INFO "aha152x%d: debugging options set to 0x%04x (were 0x%04x)\n" ,HOSTNO ,HOSTDATA (shpnt)->debug, debug); - } else -#endif #if defined(AHA152X_STAT )if (length>13 &&strncmp ("reset" , buffer+8 ,5 )==0 ) {int i;
@@ -3302,7 +2807,7 @@ static int aha152x_set_info(struct Scsi_Host *shpnt, char *buffer, int length) HOSTDATA(shpnt)->time[i]=0; } -printk (KERN_INFO "aha152x%d: stats reseted.\n" ,HOSTNO ); +shost_printk (KERN_INFO , shpnt,"aha152x: stats reset.\n" ); } else#endif
@@ -3343,29 +2848,6 @@ static int aha152x_show_info(struct seq_file *m, struct Scsi_Host *shpnt) (((HOSTDATA (shpnt)->syncrate[i] &0x70 ) >>4 ) +2 ) *50 , HOSTDATA(shpnt)->syncrate[i] & 0x0f); } -#if defined(AHA152X_DEBUG ) -#define PDEBUG (flags,txt) \ - if(HOSTDATA(shpnt)->debug & flags) SPRINTF("(%s) ", txt); - -SPRINTF ("enabled debugging options: " ); - -PDEBUG (debug_procinfo,"procinfo" ); -PDEBUG (debug_queue,"queue" ); -PDEBUG (debug_intr,"interrupt" ); -PDEBUG (debug_selection,"selection" ); -PDEBUG (debug_msgo,"message out" ); -PDEBUG (debug_msgi,"message in" ); -PDEBUG (debug_status,"status" ); -PDEBUG (debug_cmd,"command" ); -PDEBUG (debug_datai,"data in" ); -PDEBUG (debug_datao,"data out" ); -PDEBUG (debug_eh,"eh" ); -PDEBUG (debug_locking,"locks" ); -PDEBUG (debug_phases,"phases" ); - -SPRINTF ("\n" ); -#endif -SPRINTF ("\nqueue status:\n" );DO_LOCK (flags);if (ISSUE_SC ) {
@@ -3393,8 +2875,8 @@ static int aha152x_show_info(struct seq_file *m, struct Scsi_Host *shpnt)#if defined(AHA152X_STAT )SPRINTF ("statistics:\n" -"total commands: %d\n" -"disconnections: %d\n" +"total commands: %d\n" +"disconnections: %d\n" "busfree with check condition: %d\n" "busfree without old command: %d\n" "busfree without new command: %d\n"
@@ -3413,7 +2895,7 @@ static int aha152x_show_info(struct seq_file *m, struct Scsi_Host *shpnt) HOSTDATA(shpnt)->busfree_without_any_action);for (i=0 ; i<maxstate; i++) {SPRINTF ("%-10s %-12d %-12d %-12ld\n" , - states[i].name, + states[i].name,HOSTDATA (shpnt)->count_trans[i],HOSTDATA (shpnt)->count[i],HOSTDATA (shpnt)->time[i]);
@@ -3671,25 +3153,19 @@ static int __init aha152x_init(void) setup[setup_count].synchronous = aha152x[5 ]; setup[setup_count].delay = aha152x[6 ]; setup[setup_count].ext_trans = aha152x[7 ]; -#if defined(AHA152X_DEBUG ) - setup[setup_count].debug = aha152x[8 ]; -#endif - }else if (io[0 ]!=0 || irq[0 ]!=0 ) { + }else if (io[0 ] !=0 || irq[0 ] !=0 ) {if (io[0 ]!=0 ) setup[setup_count].io_port = io[0 ];if (irq[0 ]!=0 ) setup[setup_count].irq = irq[0 ]; - setup[setup_count].scsiid = scsiid[0 ]; - setup[setup_count].reconnect = reconnect[0 ]; - setup[setup_count].parity = parity[0 ]; - setup[setup_count].synchronous = sync[0 ]; - setup[setup_count].delay = delay[0 ]; - setup[setup_count].ext_trans = exttrans[0 ]; -#if defined(AHA152X_DEBUG ) - setup[setup_count].debug = debug[0 ]; -#endif + setup[setup_count].scsiid = scsiid[0 ]; + setup[setup_count].reconnect = reconnect[0 ]; + setup[setup_count].parity = parity[0 ]; + setup[setup_count].synchronous = sync[0 ]; + setup[setup_count].delay = delay[0 ]; + setup[setup_count].ext_trans = exttrans[0 ]; } -if (checksetup (&setup[setup_count])) +if (checksetup (&setup[setup_count])) setup_count++;else printk(KERN_ERR "aha152x: invalid module params io=0x%x, irq=%d,scsiid=%d,reconnect=%d,parity=%d,sync=%d,delay=%d,exttrans=%d\n",
@@ -3714,22 +3190,16 @@ static int __init aha152x_init(void) setup[setup_count].synchronous = aha152x1[5 ]; setup[setup_count].delay = aha152x1[6 ]; setup[setup_count].ext_trans = aha152x1[7 ]; -#if defined(AHA152X_DEBUG ) - setup[setup_count].debug = aha152x1[8 ]; -#endif - }else if (io[1 ]!=0 || irq[1 ]!=0 ) { + }else if (io[1 ] !=0 || irq[1 ] !=0 ) {if (io[1 ]!=0 ) setup[setup_count].io_port = io[1 ];if (irq[1 ]!=0 ) setup[setup_count].irq = irq[1 ]; - setup[setup_count].scsiid = scsiid[1 ]; - setup[setup_count].reconnect = reconnect[1 ]; - setup[setup_count].parity = parity[1 ]; - setup[setup_count].synchronous = sync[1 ]; - setup[setup_count].delay = delay[1 ]; - setup[setup_count].ext_trans = exttrans[1 ]; -#if defined(AHA152X_DEBUG ) - setup[setup_count].debug = debug[1 ]; -#endif + setup[setup_count].scsiid = scsiid[1 ]; + setup[setup_count].reconnect = reconnect[1 ]; + setup[setup_count].parity = parity[1 ]; + setup[setup_count].synchronous = sync[1 ]; + setup[setup_count].delay = delay[1 ]; + setup[setup_count].ext_trans = exttrans[1 ]; }if (checksetup (&setup[setup_count])) setup_count++;
@@ -3776,9 +3246,6 @@ static int __init aha152x_init(void) setup[setup_count].synchronous =1 ; setup[setup_count].delay =DELAY_DEFAULT ; setup[setup_count].ext_trans =0 ; -#if defined(AHA152X_DEBUG ) - setup[setup_count].debug =DEBUG_DEFAULT ; -#endif #if defined(__ISAPNP__) pnpdev[setup_count] = dev;#endif
@@ -3847,9 +3314,6 @@ static int __init aha152x_init(void) setup[setup_count].synchronous = conf.cf_syncneg; setup[setup_count].delay =DELAY_DEFAULT ; setup[setup_count].ext_trans =0 ; -#if defined(AHA152X_DEBUG ) - setup[setup_count].debug =DEBUG_DEFAULT ; -#endif setup_count++; }
@@ -3903,11 +3367,8 @@ module_exit(aha152x_exit);#if !defined(MODULE )static int __init aha152x_setup (char *str) { -#if defined(AHA152X_DEBUG ) -int ints[11 ]; -#else int ints[10 ]; -#endif +get_options (str,ARRAY_SIZE (ints), ints);if (setup_count>=ARRAY_SIZE (setup)) {
@@ -3924,16 +3385,9 @@ static int __init aha152x_setup(char *str) setup[setup_count].synchronous = ints[0 ] >=6 ? ints[6 ] :1 ; setup[setup_count].delay = ints[0 ] >=7 ? ints[7 ] :DELAY_DEFAULT ; setup[setup_count].ext_trans = ints[0 ] >=8 ? ints[8 ] :0 ; -#if defined(AHA152X_DEBUG ) - setup[setup_count].debug = ints[0 ] >=9 ? ints[9 ] :DEBUG_DEFAULT ; -if (ints[0 ] >9 ) { -printk (KERN_NOTICE "aha152x: usage: aha152x=<IOBASE>[,<IRQ>[,<SCSI ID>" -"[,<RECONNECT>[,<PARITY>[,<SYNCHRONOUS>[,<DELAY>[,<EXT_TRANS>[,<DEBUG>]]]]]]]]\n" ); -#else if (ints[0 ] >8 ) {/*}*/ printk (KERN_NOTICE "aha152x: usage: aha152x=<IOBASE>[,<IRQ>[,<SCSI ID>" "[,<RECONNECT>[,<PARITY>[,<SYNCHRONOUS>[,<DELAY>[,<EXT_TRANS>]]]]]]]\n" ); -#endif }else { setup_count++;return 0 ;
Signature · 1 line
1.8.5.2
RE: [PATCH 03/24] aha152x: Debug output update and whitespace cleanup
Quoted 2 lines
-----Original Message-----
From: Hannes Reinecke [mailto:[email protected]]
...
Quoted 4 lines
drivers/scsi/aha152x.c +0
-0
...
drivers/scsi/aha152x.c CMD_INC_RESID(struct scsi_cmnd *cmd, int inc) · line 314
Quoted 6 lines
drivers/scsi/aha152x.c +1
-1@@ -345,10 +311,10 @@ CMD_INC_RESID(struct scsi_cmnd *cmd, int inc)enum { not_issued =0x0001 ,/* command not yet issued */ - selecting =0x0002 ,/* target is beeing selected */ + selecting =0x0002 ,/* target is beeing selected */
You might want to fix the spelling of "being"
while modifying that line.
PATCH v5 4/24 scsi: introduce sdev_prefix_printk()
In reply to Hannes ReineckeLike scmd_printk(), but the device name is passed in as a string. Can be used by eg ULDs which do not have access to the scsi_cmnd structure.
Reviewed-by Christoph Hellwig <[email protected]>
Signed-off-by Hannes Reinecke <[email protected]>
--- drivers/scsi/ch.c | 3 +-- drivers/scsi/sd.h | 7 ++++--- drivers/scsi/sg.c | 4 ++-- drivers/scsi/sr.h | 3 +-- drivers/scsi/st.c | 3 +-- include/scsi/scsi_device.h | 9 +++++++++ 6 files changed, 18 insertions(+), 11 deletions(-)
drivers/scsi/ch.c +1 -2
@@ -85,8 +85,7 @@ static const char * vendor_labels[CH_TYPES-4] = {// module_param_string_array(vendor_labels, NULL, 0444); #define ch_printk(prefix, ch, fmt, a...) \ - sdev_printk(prefix, (ch)->device, "[%s] " fmt, \ - (ch)->name, ##a) + sdev_prefix_printk(prefix, (ch)->device, (ch)->name, fmt, ##a) #define DPRINTK(fmt, arg...) \ do { \
drivers/scsi/sd.h +4 -3
@@ -103,9 +103,10 @@ static inline struct scsi_disk *scsi_disk(struct gendisk *disk) #define sd_printk(prefix, sdsk, fmt, a...) \ (sdsk)->disk ? \ - sdev_printk(prefix, (sdsk)->device, "[%s] " fmt, \ - (sdsk)->disk->disk_name, ##a) : \ - sdev_printk(prefix, (sdsk)->device, fmt, ##a) + sdev_prefix_printk(prefix, (sdsk)->device, \ + (sdsk)->disk->disk_name, fmt, ##a) : \ + sdev_prefix_printk(prefix, (sdsk)->device, \ + NULL, fmt, ##a) #define sd_first_printk(prefix, sdsk, fmt, a...) \ do { \
drivers/scsi/sg.c +2 -2
@@ -219,8 +219,8 @@ static void sg_device_destroy(struct kref *kref);#define SZ_SG_REQ_INFO sizeof(sg_req_info_t) #define sg_printk(prefix, sdp, fmt, a...) \ - sdev_printk(prefix, (sdp)->device, "[%s] " fmt, \ - (sdp)->disk->disk_name, ##a) + sdev_prefix_printk(prefix, (sdp)->device, \ + (sdp)->disk->disk_name, fmt, ##a)static int sg_allow_access (struct file *filp,unsigned char *cmd) {
drivers/scsi/sr.h +1 -2
@@ -57,8 +57,7 @@ typedef struct scsi_cd { } Scsi_CD; #define sr_printk(prefix, cd, fmt, a...) \ - sdev_printk(prefix, (cd)->device, "[%s] " fmt, \ - (cd)->cdi.name, ##a) + sdev_prefix_printk(prefix, (cd)->device, (cd)->cdi.name, fmt, ##a)int sr_do_ioctl (Scsi_CD *,struct packet_command *);
drivers/scsi/st.c +1 -2
@@ -306,8 +306,7 @@ static inline char *tape_name(struct scsi_tape *tape) } #define st_printk(prefix, t, fmt, a...) \ - sdev_printk(prefix, (t)->device, "%s: " fmt, \ - tape_name(t), ##a) + sdev_prefix_printk(prefix, (t)->device, tape_name(t), fmt, ##a)#ifdef DEBUG #define DEBC_printk(t, fmt, a...) \ if (debugging) { st_printk(ST_DEB_MSG, t, fmt, ##a ); }
include/scsi/scsi_device.h +9 -0
@@ -244,6 +244,15 @@ struct scsi_dh_data { #define sdev_dbg(sdev, fmt, a...) \ dev_dbg(&(sdev)->sdev_gendev, fmt, ##a) +/* +* like scmd_printk, but the device name is passed in +* as a string pointer +*/ +#define sdev_prefix_printk(l, sdev, p, fmt, a...) \ + (p) ? \ + sdev_printk(l, sdev, "[%s] " fmt, p, ##a) : \ + sdev_printk(l, sdev, fmt, ##a) + #define scmd_printk(prefix, scmd, fmt, a...) \ (scmd)->request->rq_disk ? \ sdev_printk(prefix, (scmd)->device, "[%s] " fmt, \
Signature · 1 line
1.8.5.2
RE: [PATCH 04/24] scsi: introduce sdev_prefix_printk()
Quoted 2 lines
-----Original Message-----
From: Hannes Reinecke [mailto:[email protected]]
...
Quoted 19 lines
drivers/scsi/sd.h +4
-3@@ -103,9 +103,10 @@ static inline struct scsi_disk *scsi_disk(struct gendisk *disk) #define sd_printk(prefix, sdsk, fmt, a...) \ (sdsk)->disk ? \ - sdev_printk(prefix, (sdsk)->device, "[%s] " fmt, \ - (sdsk)->disk->disk_name, ##a) : \ - sdev_printk(prefix, (sdsk)->device, fmt, ##a) + sdev_prefix_printk(prefix, (sdsk)->device, \ + (sdsk)->disk->disk_name, fmt, ##a) : \ + sdev_prefix_printk(prefix, (sdsk)->device, \ + NULL, fmt, ##a) #define sd_first_printk(prefix, sdsk, fmt, a...) \do { \
...
Quoted 21 lines
include/scsi/scsi_device.h +9
-1@@ -244,6 +244,15 @@ struct scsi_dh_data { #define sdev_dbg(sdev, fmt, a...) \ dev_dbg(&(sdev)->sdev_gendev, fmt, ##a) +/* +* like scmd_printk, but the device name is passed in +* as a string pointer +*/ +#define sdev_prefix_printk(l, sdev, p, fmt, a...) \ + (p) ? \ + sdev_printk(l, sdev, "[%s] " fmt, p, ##a) : \ + sdev_printk(l, sdev, fmt, ##a) + #define scmd_printk(prefix, scmd, fmt, a...) \ (scmd)->request->rq_disk ? \ sdev_printk(prefix, (scmd)->device, "[%s] " fmt, \ --
Quoted 1 line
1.8.5.2
This triggers lots of compiler warnings with gcc 4.4.7 like:
drivers/scsi/sd.c: In function 'sd_open':drivers/scsi/sd.c:1179: warning: reading through null pointer (argument 4) drivers/scsi/sd.c:1179: warning: format '%s' expects type 'char *', but argument 4 has type 'void *'
That is from:
SCSI_LOG_HLQUEUE(3, sd_printk(KERN_INFO, sdkp, "sd_open\n"));
Since:
#define NULL ((void *)0)
gcc probably doesn't realize the (p)? prevents the NULL (a void *) from being passed to sdev_printk.
Passing "" rather than NULL eliminates the compiler warnings.
There should probably be a () around p in the sdev_printk call, too.
---
Rob Elliott HP Server Storage
RE: [PATCH 04/24] scsi: introduce sdev_prefix_printk()
Quoted 56 lines · Robert (Server Storage)
Quoted 2 lines
-----Original Message-----
From: Hannes Reinecke [mailto:[email protected]]...
Quoted 19 lines
drivers/scsi/sd.h +4
-3@@ -103,9 +103,10 @@ static inline struct scsi_disk *scsi_disk(struct gendisk *disk) #define sd_printk(prefix, sdsk, fmt, a...) \ (sdsk)->disk ? \ - sdev_printk(prefix, (sdsk)->device, "[%s] " fmt, \ - (sdsk)->disk->disk_name, ##a) : \ - sdev_printk(prefix, (sdsk)->device, fmt, ##a) + sdev_prefix_printk(prefix, (sdsk)->device, \ + (sdsk)->disk->disk_name, fmt, ##a) : \ + sdev_prefix_printk(prefix, (sdsk)->device, \ + NULL, fmt, ##a) #define sd_first_printk(prefix, sdsk, fmt, a...) \do { \
...
Quoted 21 lines
include/scsi/scsi_device.h +9
-1@@ -244,6 +244,15 @@ struct scsi_dh_data { #define sdev_dbg(sdev, fmt, a...) \ dev_dbg(&(sdev)->sdev_gendev, fmt, ##a) +/* +* like scmd_printk, but the device name is passed in +* as a string pointer +*/ +#define sdev_prefix_printk(l, sdev, p, fmt, a...) \ + (p) ? \ + sdev_printk(l, sdev, "[%s] " fmt, p, ##a) : \ + sdev_printk(l, sdev, fmt, ##a) + #define scmd_printk(prefix, scmd, fmt, a...) \ (scmd)->request->rq_disk ? \ sdev_printk(prefix, (scmd)->device, "[%s] " fmt, \ --Quoted 1 line
1.8.5.2
This triggers lots of compiler warnings with gcc 4.4.7 like:
drivers/scsi/sd.c: In function 'sd_open':drivers/scsi/sd.c:1179: warning: reading through null pointer (argument 4)drivers/scsi/sd.c:1179: warning: format '%s' expects type 'char *', but argument 4 has type 'void *'That is from:
SCSI_LOG_HLQUEUE(3, sd_printk(KERN_INFO, sdkp, "sd_open\n"));
Quoted 7 lines
Since:
#define NULL ((void *)0)gcc probably doesn't realize the (p)? prevents the NULL (a void *) from being passed to
sdev_printk.Passing "" rather than NULL eliminates the compiler warnings.
It eliminates the warnings, but unfortunately we then get log messages that look like:
Oct 3 11:30:08 rhel-storage-01 kernel: sd 10:0:0:0: [sde] (null)FAILED Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE
^^^^^^
Changing it to (char *)NULL, like this:
#define sd_printk(prefix, sdsk, fmt, a...) \
(sdsk)->disk ? \
sdev_prefix_printk(prefix, (sdsk)->device, \
(sdsk)->disk->disk_name, fmt, ##a) : \
sdev_prefix_printk(prefix, (sdsk)->device, \
(char *)NULL, fmt, ##a)doesn't work either. The compiler gives an error:
drivers/scsi/sd.c: In function 'sd_open':drivers/scsi/sd.c:1158:2: error: reading through null pointer (argument 4) [-Werror=format=]
SCSI_LOG_HLQUEUE(3, sd_printk(KERN_INFO, sdkp, "sd_open\n"));
^
-Ewan
Quoted 13 lines
There should probably be a () around p in the
sdev_printkcall, too.---
Rob Elliott HP Server Storage--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 04/24] scsi: introduce sdev_prefix_printk()
Quoted 82 lines · Ewan Milne
Quoted 56 lines · Robert (Server Storage)
Quoted 2 lines
-----Original Message-----
From: Hannes Reinecke [mailto:[email protected]]...
Quoted 19 lines
drivers/scsi/sd.h +4
-3@@ -103,9 +103,10 @@ static inline struct scsi_disk *scsi_disk(struct gendisk *disk) #define sd_printk(prefix, sdsk, fmt, a...) \ (sdsk)->disk ? \ - sdev_printk(prefix, (sdsk)->device, "[%s] " fmt, \ - (sdsk)->disk->disk_name, ##a) : \ - sdev_printk(prefix, (sdsk)->device, fmt, ##a) + sdev_prefix_printk(prefix, (sdsk)->device, \ + (sdsk)->disk->disk_name, fmt, ##a) : \ + sdev_prefix_printk(prefix, (sdsk)->device, \ + NULL, fmt, ##a) #define sd_first_printk(prefix, sdsk, fmt, a...) \do { \
...
Quoted 21 lines
include/scsi/scsi_device.h +9
-1@@ -244,6 +244,15 @@ struct scsi_dh_data { #define sdev_dbg(sdev, fmt, a...) \ dev_dbg(&(sdev)->sdev_gendev, fmt, ##a) +/* +* like scmd_printk, but the device name is passed in +* as a string pointer +*/ +#define sdev_prefix_printk(l, sdev, p, fmt, a...) \ + (p) ? \ + sdev_printk(l, sdev, "[%s] " fmt, p, ##a) : \ + sdev_printk(l, sdev, fmt, ##a) + #define scmd_printk(prefix, scmd, fmt, a...) \ (scmd)->request->rq_disk ? \ sdev_printk(prefix, (scmd)->device, "[%s] " fmt, \ --Quoted 1 line
1.8.5.2
This triggers lots of compiler warnings with gcc 4.4.7 like:
drivers/scsi/sd.c: In function 'sd_open':drivers/scsi/sd.c:1179: warning: reading through null pointer (argument 4)drivers/scsi/sd.c:1179: warning: format '%s' expects type 'char *', but argument 4 has type 'void *'That is from:
SCSI_LOG_HLQUEUE(3, sd_printk(KERN_INFO, sdkp, "sd_open\n"));Quoted 7 lines
Since:
#define NULL ((void *)0)gcc probably doesn't realize the (p)? prevents the NULL (a void *) from being passed to
sdev_printk.Passing "" rather than NULL eliminates the compiler warnings.
It eliminates the warnings, but unfortunately we then get log messages that look like:
Oct 3 11:30:08 rhel-storage-01 kernel: sd 10:0:0:0: [sde] (null)FAILED Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE^^^^^^Changing it to (char *)NULL, like this:
#define sd_printk(prefix, sdsk, fmt, a...) \ (sdsk)->disk ? \ sdev_prefix_printk(prefix, (sdsk)->device, \ (sdsk)->disk->disk_name, fmt, ##a) : \ sdev_prefix_printk(prefix, (sdsk)->device, \ (char *)NULL, fmt, ##a)
Quoted 7 lines
doesn't work either. The compiler gives an error:
drivers/scsi/sd.c: In function 'sd_open':drivers/scsi/sd.c:1158:2: error: reading through null pointer (argument 4) [-Werror=format=]
SCSI_LOG_HLQUEUE(3, sd_printk(KERN_INFO, sdkp, "sd_open\n"));
^
I've fixed it up in my next iteration of the patchset. (By simply using 'sdev_printk' if the prefix argument is NULL ...)
Cheers,
Hannes
Signature · 8 lines
Dr. Hannes Reinecke zSeries & Storage [email protected] +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 [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH v5 5/24 scsi: Use sdev as argument for sense code printing
In reply to Hannes ReineckeWe should be using the standard dev_printk() variants for sense code printing.
Reviewed-by Christoph Hellwig <[email protected]>
Signed-off-by Hannes Reinecke <[email protected]>
--- drivers/scsi/53c700.c | 2 +- drivers/scsi/ch.c | 2 +- drivers/scsi/constants.c | 117 ++++++++++++++++++++++----------------------- drivers/scsi/osst.c | 8 ++-- drivers/scsi/scsi.c | 2 +- drivers/scsi/scsi_error.c | 2 +- drivers/scsi/scsi_ioctl.c | 2 +- drivers/scsi/scsi_lib.c | 4 +- drivers/scsi/sd.c | 9 ++-- drivers/scsi/sg.c | 2 +- drivers/scsi/sr_ioctl.c | 6 +-- drivers/scsi/st.c | 6 ++- drivers/scsi/storvsc_drv.c | 3 +- include/scsi/scsi_dbg.h | 17 ++++--- 14 files changed, 96 insertions(+), 86 deletions(-)
drivers/scsi/53c700.c +1 -1
@@ -602,7 +602,7 @@ NCR_700_scsi_done(struct NCR_700_Host_Parameters *hostdata,#ifdef NCR_700_DEBUG printk (" ORIGINAL CMD %p RETURNED %d, new return is %d sense is\n" , SCp, SCp->cmnd[7 ], result); -scsi_print_sense ("53c700" , SCp); +scsi_print_sense (SCp);#endif dma_unmap_single(hostdata->dev, slot->dma_handle,
drivers/scsi/ch.c +1 -1
@@ -206,7 +206,7 @@ ch_do_scsi(scsi_changer *ch, unsigned char *cmd,DPRINTK ("result: 0x%x\n" ,result);if (driver_byte (result) &DRIVER_SENSE ) {if (debug) -scsi_print_sense_hdr (ch->name, &sshdr); +scsi_print_sense_hdr (ch->device, ch->name, &sshdr); errno =ch_find_errno (&sshdr);switch (sshdr.sense_key) {
drivers/scsi/constants.c +58 -59
184 lines not on this page · raw message
drivers/scsi/osst.c +5 -3
@@ -259,9 +259,10 @@ static int osst_chk_result(struct osst_tape * STp, struct osst_request * SRpnt) SRpnt->cmd[0 ], SRpnt->cmd[1 ], SRpnt->cmd[2 ], SRpnt->cmd[3], SRpnt->cmd[4], SRpnt->cmd[5]);if (scode)printk (OSST_DEB_MSG "%s:D: Sense: %02x, ASC: %02x, ASCQ: %02x\n" , - name, scode, sense[12 ], sense[13 ]); + name, scode, sense[12 ], sense[13 ]);if (cmdstatp->have_sense) -__scsi_print_sense ("osst " , SRpnt->sense,SCSI_SENSE_BUFFERSIZE ); +__scsi_print_sense (STp->device, name, + SRpnt->sense,SCSI_SENSE_BUFFERSIZE ); } else#endif
@@ -275,7 +276,8 @@ static int osst_chk_result(struct osst_tape * STp, struct osst_request * SRpnt) SRpnt->cmd[0] != TEST_UNIT_READY)) { /* Abnormal conditions for tape */if (cmdstatp->have_sense) {printk (KERN_WARNING "%s:W: Command with sense data:\n" , name); -__scsi_print_sense ("osst " , SRpnt->sense,SCSI_SENSE_BUFFERSIZE ); +__scsi_print_sense (STp->device, name, + SRpnt->sense,SCSI_SENSE_BUFFERSIZE ); }else {static int notyetprinted =1 ;
drivers/scsi/scsi.c +1 -1
@@ -606,7 +606,7 @@ void scsi_log_completion(struct scsi_cmnd *cmd, int disposition)scsi_print_result (cmd);scsi_print_command (cmd);if (status_byte (cmd->result) &CHECK_CONDITION ) -scsi_print_sense ("" , cmd); +scsi_print_sense (cmd);if (level >3 ) scmd_printk(KERN_INFO, cmd, "scsi host busy %d failed %d\n",
drivers/scsi/scsi_error.c +1 -1
@@ -1180,7 +1180,7 @@ int scsi_eh_get_sense(struct list_head *work_q,SCSI_LOG_ERROR_RECOVERY (3 ,scmd_printk (KERN_INFO , scmd,"sense requested for %p result %x\n" , scmd, scmd->result)); -SCSI_LOG_ERROR_RECOVERY (3 ,scsi_print_sense ("bh" , scmd)); +SCSI_LOG_ERROR_RECOVERY (3 ,scsi_print_sense (scmd)); rtn =scsi_decide_disposition (scmd);
drivers/scsi/scsi_ioctl.c +1 -1
@@ -126,7 +126,7 @@ static int ioctl_internal_command(struct scsi_device *sdev, char *cmd,sdev_printk (KERN_INFO , sdev,"ioctl_internal_command return code = %x\n" , result); -scsi_print_sense_hdr (" " , &sshdr); +scsi_print_sense_hdr (sdev,NULL , &sshdr);break ; } }
drivers/scsi/scsi_lib.c +2 -2
@@ -912,7 +912,7 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)if ((sshdr.asc ==0x0 ) && (sshdr.ascq ==0x1d )) ;else if (!(req->cmd_flags &REQ_QUIET )) -scsi_print_sense ("" , cmd); +scsi_print_sense (cmd); result =0 ;/* BLOCK_PC may have set error */ error =0 ;
@@ -1041,7 +1041,7 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)if (!(req->cmd_flags &REQ_QUIET )) {scsi_print_result (cmd);if (driver_byte (result) &DRIVER_SENSE ) -scsi_print_sense ("" , cmd); +scsi_print_sense (cmd);scsi_print_command (cmd); }if (!scsi_end_request (req, error,blk_rq_err_bytes (req),0 ))
drivers/scsi/sd.c +5 -4
@@ -3321,10 +3321,11 @@ module_exit(exit_sd);static void sd_print_sense_hdr (struct scsi_disk *sdkp,struct scsi_sense_hdr *sshdr) { -sd_printk (KERN_INFO , sdkp," " ); -scsi_show_sense_hdr (sshdr); -sd_printk (KERN_INFO , sdkp," " ); -scsi_show_extd_sense (sshdr->asc, sshdr->ascq); +scsi_show_sense_hdr (sdkp->device, + sdkp->disk ? sdkp->disk->disk_name :NULL , sshdr); +scsi_show_extd_sense (sdkp->device, + sdkp->disk ? sdkp->disk->disk_name :NULL , + sshdr->asc, sshdr->ascq); }static void sd_print_result (struct scsi_disk *sdkp,int result)
drivers/scsi/sg.c +1 -1
@@ -1360,7 +1360,7 @@ sg_rq_end_io(struct request *rq, int uptodate)if ((sdp->sgdebug >0 ) && ((CHECK_CONDITION == srp->header.masked_status) || (COMMAND_TERMINATED == srp->header.masked_status))) -__scsi_print_sense (__func__, sense, +__scsi_print_sense (sdp->device, __func__, sense,SCSI_SENSE_BUFFERSIZE );/* Following if statement is a patch supplied by Eric Youngdale */
drivers/scsi/sr_ioctl.c +3 -3
@@ -246,7 +246,7 @@ int sr_do_ioctl(Scsi_CD *cd, struct packet_command *cgc)"CDROM not ready. Make sure there " "is a disc in the drive.\n");#ifdef DEBUG -scsi_print_sense_hdr ("sr" , &sshdr); +scsi_print_sense_hdr (cd->device, cd->cdi.name, &sshdr);#endif err = -ENOMEDIUM ;break ;
@@ -258,14 +258,14 @@ int sr_do_ioctl(Scsi_CD *cd, struct packet_command *cgc) err = -EDRIVE_CANT_DO_THIS ;#ifdef DEBUG __scsi_print_command (cgc->cmd); -scsi_print_sense_hdr ("sr" , &sshdr); +scsi_print_sense_hdr (cd->device, cd->cdi.name, &sshdr);#endif break ;default :sr_printk (KERN_ERR , cd,"CDROM (ioctl) error, command: " );__scsi_print_command (cgc->cmd); -scsi_print_sense_hdr ("sr" , &sshdr); +scsi_print_sense_hdr (cd->device, cd->cdi.name, &sshdr); err = -EIO ; } }
drivers/scsi/st.c +4 -2
@@ -373,7 +373,8 @@ static int st_chk_result(struct scsi_tape *STp, struct st_request * SRpnt) SRpnt->cmd[0 ], SRpnt->cmd[1 ], SRpnt->cmd[2 ], SRpnt->cmd[3], SRpnt->cmd[4], SRpnt->cmd[5]);if (cmdstatp->have_sense) -__scsi_print_sense (name, SRpnt->sense,SCSI_SENSE_BUFFERSIZE ); +__scsi_print_sense (STp->device, name, + SRpnt->sense,SCSI_SENSE_BUFFERSIZE ); } ) /* end DEB */if (!debugging) {/* Abnormal conditions for tape */ if (!cmdstatp->have_sense)
@@ -389,7 +390,8 @@ static int st_chk_result(struct scsi_tape *STp, struct st_request * SRpnt) SRpnt->cmd[0] != MODE_SENSE && SRpnt->cmd[0] != TEST_UNIT_READY) { -__scsi_print_sense (name, SRpnt->sense,SCSI_SENSE_BUFFERSIZE ); +__scsi_print_sense (STp->device, name, + SRpnt->sense,SCSI_SENSE_BUFFERSIZE ); } }
drivers/scsi/storvsc_drv.c +2 -1
@@ -1097,7 +1097,8 @@ static void storvsc_command_completion(struct storvsc_cmd_request *cmd_request)if (scmnd->result) {if (scsi_normalize_sense (scmnd->sense_buffer,SCSI_SENSE_BUFFERSIZE , &sense_hdr)) -scsi_print_sense_hdr ("storvsc" , &sense_hdr); +scsi_print_sense_hdr (scmnd->device,"storvsc" , + &sense_hdr); }if (vm_srb->srb_status !=SRB_STATUS_SUCCESS )
include/scsi/scsi_dbg.h +11 -6
@@ -2,21 +2,26 @@#define _SCSI_SCSI_DBG_Hstruct scsi_cmnd ; +struct scsi_device ;struct scsi_sense_hdr ;extern void scsi_print_command (struct scsi_cmnd *);extern void __scsi_print_command (unsigned char *); -extern void scsi_show_extd_sense (unsigned char ,unsigned char ); -extern void scsi_show_sense_hdr (struct scsi_sense_hdr *); -extern void scsi_print_sense_hdr (const char *,struct scsi_sense_hdr *); -extern void scsi_print_sense (char *,struct scsi_cmnd *); -extern void __scsi_print_sense (const char *name, +extern void scsi_show_extd_sense (struct scsi_device *,const char *, +unsigned char ,unsigned char ); +extern void scsi_show_sense_hdr (struct scsi_device *,const char *, +struct scsi_sense_hdr *); +extern void scsi_print_sense_hdr (struct scsi_device *,const char *, +struct scsi_sense_hdr *); +extern void scsi_print_sense (struct scsi_cmnd *); +extern void __scsi_print_sense (struct scsi_device *,const char *name,const unsigned char *sense_buffer,int sense_len);extern void scsi_show_result (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 ); +extern const char *scsi_extd_sense_format (unsigned char ,unsigned char , +const char **);#endif /* _SCSI_SCSI_DBG_H */
Signature · 1 line
1.8.5.2
RE: [PATCH 05/24] scsi: Use sdev as argument for sense code printing
Quoted 4 lines
-----Original Message-----
From: Hannes Reinecke [mailto:[email protected]]
...
drivers/scsi/constants.c scsi_extd_sense_format(unsigned char asc, unsigned · line 1377
Quoted 8 lines
drivers/scsi/constants.c +2
-1@@ -1369,49 +1372,53 @@ scsi_extd_sense_format(unsigned char asc, unsigned char ascq) {EXPORT_SYMBOL (scsi_extd_sense_format);void -scsi_show_extd_sense (unsigned char asc,unsigned char ascq) +scsi_show_extd_sense (struct scsi_device *sdev,const char *name, +unsigned char asc,unsigned char ascq)
Since what sdev points to is not modified, that could be const struct scsi_device *sdev.
(also adjust the function prototype in scsi_dbg.h to match)
Quoted 35 lines
drivers/scsi/constants.c +17
-15{ -const char *extd_sense_fmt =scsi_extd_sense_format (asc, ascq); +const char *extd_sense_fmt =NULL ; +const char *extd_sense_str =scsi_extd_sense_format (asc, ascq, + &extd_sense_fmt); + +if (extd_sense_str) { +if (extd_sense_fmt) +sdev_prefix_printk (KERN_INFO , sdev, name, +"Add. Sense: %s (%s%x)" , + extd_sense_str, extd_sense_fmt, + ascq); +else +sdev_prefix_printk (KERN_INFO , sdev, name, +"Add. Sense: %s" , extd_sense_str); -if (extd_sense_fmt) { -if (strstr (extd_sense_fmt,"%x" )) { -printk ("Add. Sense: " ); -printk (extd_sense_fmt, ascq); - }else -printk ("Add. Sense: %s" , extd_sense_fmt); }else { -if (asc >=0x80 ) -printk ("<<vendor>> ASC=0x%x ASCQ=0x%x" , asc, - ascq); -if (ascq >=0x80 ) -printk ("ASC=0x%x <<vendor>> ASCQ=0x%x" , asc, - ascq); -else -printk ("ASC=0x%x ASCQ=0x%x" , asc, ascq); +sdev_prefix_printk (KERN_INFO , sdev, name, +"%sASC=0x%x %sASCQ=0x%x\n" , + asc >=0x80 ?"<<vendor>> " :"" , asc, + ascq >=0x80 ?"<<vendor>> " :"" , ascq);
drivers/scsi/constants.c scsi_extd_sense_format(unsigned char asc, unsigned char ascq) { · line 1403
Quoted 10 lines
drivers/scsi/constants.c +2
-3} - -printk ("\n" ); }EXPORT_SYMBOL (scsi_show_extd_sense);void -scsi_show_sense_hdr (struct scsi_sense_hdr *sshdr) +scsi_show_sense_hdr (struct scsi_device *sdev,const char *name, +struct scsi_sense_hdr *sshdr)
Add const for *sdev and *sshdr.
The latter requires also adding const to the argument in scsi_sense_is_deferred() in scsi_eh.h.
...
drivers/scsi/constants.c EXPORT_SYMBOL(scsi_show_sense_hdr); · line 1430
Quoted 7 lines
drivers/scsi/constants.c +2
-1@@ -1419,12 +1426,11 @@ EXPORT_SYMBOL(scsi_show_sense_hdr);* Print normalized SCSI sense header with a prefix. */ void -scsi_print_sense_hdr (const char *name,struct scsi_sense_hdr *sshdr) +scsi_print_sense_hdr (struct scsi_device *sdev,const char *name, +struct scsi_sense_hdr *sshdr)
Add const for *sdev and *sshdr.
...
drivers/scsi/constants.c scsi_decode_sense_extras(const unsigned char · line 1524
Quoted 9 lines
drivers/scsi/constants.c +2
-2@@ -1513,33 +1519,26 @@ scsi_decode_sense_extras(const unsigned char *sense_buffer, int sense_len, }/* Normalize and print sense buffer with name prefix */ -void __scsi_print_sense (const char *name,const unsigned char *sense_buffer, -int sense_len) +void __scsi_print_sense (struct scsi_device *sdev,const char *name, +const unsigned char *sense_buffer,int sense_len)
Add const for *sdev.
Quoted 7 lines
drivers/scsi/constants.c +1
-2{struct scsi_sense_hdr sshdr; -printk (KERN_INFO "%s: " , name);scsi_decode_sense_buffer (sense_buffer, sense_len, &sshdr); -scsi_show_sense_hdr (&sshdr); +scsi_show_sense_hdr (sdev, name, &sshdr);
drivers/scsi/constants.c scsi_decode_sense_extras(const unsigned char *sense_buffer, int sense_len, · line 1535
Quoted 10 lines
drivers/scsi/constants.c +2
-3scsi_decode_sense_extras (sense_buffer, sense_len, &sshdr); -printk (KERN_INFO "%s: " , name); -scsi_show_extd_sense (sshdr.asc, sshdr.ascq); +scsi_show_extd_sense (sdev, name, sshdr.asc, sshdr.ascq); }EXPORT_SYMBOL (__scsi_print_sense);/* Normalize and print sense buffer in SCSI command */ -void scsi_print_sense (char *name,struct scsi_cmnd *cmd) +void scsi_print_sense (struct scsi_cmnd *cmd)
...
Add const for *cmd.
---
Rob Elliott HP Server Storage
PATCH v5 6/24 acornscsi: use scsi_print_command()
In reply to Hannes ReineckeUpdate acornscsi to use scsi_print_command() instead of the underscore version and use scmd_printk() in acornscsi_done(). This will add correct device annotations in the resulting message. And we should be using set_host_byte() for setting the final result.
Reviewed-by Christoph Hellwig <[email protected]>
Signed-off-by Hannes Reinecke <[email protected]>
--- drivers/scsi/arm/acornscsi.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
drivers/scsi/arm/acornscsi.c +6 -6
@@ -850,13 +850,13 @@ static void acornscsi_done(AS_Host *host, struct scsi_cmnd **SCpntp,break ;default : -printk (KERN_ERR "scsi%d.H: incomplete data transfer detected: result=%08X command=" , - host->host->host_no, SCpnt->result); -__scsi_print_command (SCpnt->cmnd); +scmd_printk (KERN_ERR , SCpnt, +"incomplete data transfer detected: " +"result=%08X" , SCpnt->result); +scsi_print_command (SCpnt);acornscsi_dumpdma (host,"done" ); -acornscsi_dumplog (host, SCpnt->device->id); - SCpnt->result &=0xffff ; - SCpnt->result |=DID_ERROR <<16 ; +acornscsi_dumplog (host, SCpnt->device->id); +set_host_byte (SCpnt,DID_ERROR ); } } }
Signature · 1 line
1.8.5.2
PATCH v5 7/24 fas216: Return DID_ERROR for incomplete data transfer
In reply to Hannes Reineckefas216 returns DID_BAD_TARGET for an incomplete data transfer. The midlayer uses DID_BAD_TARGET to signal a non-existing or not reachable target. So we should rather be using DID_ERROR here.
Signed-off-by Hannes Reinecke <[email protected]>
--- drivers/scsi/arm/fas216.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/scsi/arm/fas216.c +1 -2
@@ -2085,8 +2085,7 @@ fas216_std_done(FAS216_Info *info, struct scsi_cmnd *SCpnt, unsigned int result) SCpnt->result, info->scsi.SCp.ptr, info->scsi.SCp.this_residual);__scsi_print_command (SCpnt->cmnd); - SCpnt->result &= ~(255 <<16 ); - SCpnt->result |=DID_BAD_TARGET <<16 ; +set_host_byte (SCpnt,DID_ERROR ); goto request_sense; } }
Signature · 1 line
1.8.5.2
Re: [PATCH 07/24] fas216: Return DID_ERROR for incomplete data transfer
Quoted 6 lines · Hannes Reinecke
fas216 returns
DID_BAD_TARGETfor an incomplete data transfer. The midlayer usesDID_BAD_TARGETto signal a non-existing or not reachable target. So we should rather be usingDID_ERRORhere.Signed-off-by Hannes Reinecke <[email protected]>
Looks good,
Reviewed-by Christoph Hellwig <[email protected]>
PATCH v5 8/24 fas216: Update logging messages
In reply to Hannes ReineckeUpdate logging messages to use dev_printk() variants for correct device annotations.
Reviewed-by Christoph Hellwig <[email protected]>
Signed-off-by Hannes Reinecke <[email protected]>
--- drivers/scsi/arm/fas216.c | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-)
drivers/scsi/arm/fas216.c +13 -15
@@ -308,8 +308,7 @@ static void fas216_log_command(FAS216_Info *info, int level,fas216_do_log (info,'0' + SCpnt->device->id, fmt, args);va_end (args); -printk (" CDB: " ); -__scsi_print_command (SCpnt->cmnd); +scsi_print_command (SCpnt); } static void
@@ -2079,12 +2078,12 @@ fas216_std_done(FAS216_Info *info, struct scsi_cmnd *SCpnt, unsigned int result)break ;default : -printk (KERN_ERR "scsi%d.%c: incomplete data transfer " -"detected: res=%08X ptr=%p len=%X CDB: " , - info->host->host_no,'0' + SCpnt->device->id, - SCpnt->result, info->scsi.SCp.ptr, - info->scsi.SCp.this_residual); -__scsi_print_command (SCpnt->cmnd); +scmd_printk (KERN_ERR , SCpnt, +"incomplete data transfer " +"detected: res=%08X ptr=%p len=%X\n" , + SCpnt->result, info->scsi.SCp.ptr, + info->scsi.SCp.this_residual); +scsi_print_command (SCpnt);set_host_byte (SCpnt,DID_ERROR ); goto request_sense; }
@@ -2157,12 +2156,12 @@ static void fas216_done(FAS216_Info *info, unsigned int result)* to transfer, we should not have a valid pointer. */ if (info->scsi.SCp.ptr && info->scsi.SCp.this_residual ==0 ) { -printk ("scsi%d.%c: zero bytes left to transfer, but " -"buffer pointer still valid: ptr=%p len=%08x CDB: " , - info->host->host_no,'0' + SCpnt->device->id, - info->scsi.SCp.ptr, info->scsi.SCp.this_residual); +scmd_printk (KERN_INFO , SCpnt, +"zero bytes left to transfer, but " +"buffer pointer still valid: ptr=%p len=%08x\n" , + info->scsi.SCp.ptr, info->scsi.SCp.this_residual); info->scsi.SCp.ptr =NULL ; -__scsi_print_command (SCpnt->cmnd); +scsi_print_command (SCpnt); }/*
@@ -2663,8 +2662,7 @@ int fas216_eh_host_reset(struct scsi_cmnd *SCpnt)fas216_checkmagic (info); -printk ("scsi%d.%c: %s: resetting host\n" , - info->host->host_no,'0' + SCpnt->device->id, __func__); +fas216_log (info,LOG_ERROR ,"resetting host" );/* * Reset the SCSI chip.
Signature · 1 line
1.8.5.2
RE: [PATCH 08/24] fas216: Update logging messages
Quoted 4 lines
-----Original Message-----
From: Hannes Reinecke [mailto:[email protected]]
...
Quoted 4 lines
drivers/scsi/arm/fas216.c +0
-0@@ -2079,12 +2078,12 @@ fas216_std_done(FAS216_Info *info, struct scsi_cmnd *SCpnt, unsigned int result)break ;
drivers/scsi/arm/fas216.c fas216_std_done(FAS216_Info *info, struct scsi_cmnd · line 2084
Quoted 10 lines
drivers/scsi/arm/fas216.c +3
-6default : -printk (KERN_ERR "scsi%d.%c: incomplete data transfer " -"detected: res=%08X ptr=%p len=%X CDB: " , - info->host->host_no,'0' + SCpnt->device->id, - SCpnt->result, info->scsi.SCp.ptr, - info->scsi.SCp.this_residual); -__scsi_print_command (SCpnt->cmnd); + scmd_printk(KERN_ERR, SCpnt, + "incomplete data transfer " + "detected: res=%08X ptr=%p len=%X\n",
checkpatch lets you keep strings on one line even if the code exceeds 80 characters.
Quoted 8 lines
drivers/scsi/arm/fas216.c +3
-0+ SCpnt->result, info->scsi.SCp.ptr, + info->scsi.SCp.this_residual); +scsi_print_command (SCpnt);set_host_byte (SCpnt,DID_ERROR ); goto request_sense;drivers/scsi/arm/fas216.c +0
-0}@@ -2157,12 +2156,12 @@ static void fas216_done(FAS216_Info *info, unsigned int result)
drivers/scsi/arm/fas216.c static void fas216_done(FAS216_Info *info, unsigned · line 2162
Quoted 10 lines
drivers/scsi/arm/fas216.c +3
-4* to transfer, we should not have a valid pointer. */ if (info->scsi.SCp.ptr && info->scsi.SCp.this_residual ==0 ) { -printk ("scsi%d.%c: zero bytes left to transfer, but " -"buffer pointer still valid: ptr=%p len=%08x CDB: " , - info->host->host_no,'0' + SCpnt->device->id, - info->scsi.SCp.ptr, info->scsi.SCp.this_residual); + scmd_printk(KERN_INFO, SCpnt, + "zero bytes left to transfer, but " + "buffer pointer still valid: ptr=%p len=%08x\n",
checkpatch lets you keep strings on one line even if the code exceeds 80 characters.
---
Rob Elliott HP Server Storage
PATCH v5 9/24 53c700: remove scsi_print_sense() usage
In reply to Hannes ReineckeThe 53c700 driver would be using scsi_print_sense() in a debug statement, which was never compiled in. Plus the same information can get retrieved with logging. So remove it.
Reviewed-by Christoph Hellwig <[email protected]>
Signed-off-by Hannes Reinecke <[email protected]>
--- drivers/scsi/53c700.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-)
drivers/scsi/53c700.c +2 -7
@@ -592,19 +592,14 @@ NCR_700_scsi_done(struct NCR_700_Host_Parameters *hostdata, hostdata->cmd =NULL ;if (SCp !=NULL ) { -struct NCR_700_command_slot *slot = +struct NCR_700_command_slot *slot = (struct NCR_700_command_slot *)SCp->host_scribble; - +dma_unmap_single (hostdata->dev, slot->pCmd,MAX_COMMAND_SIZE ,DMA_TO_DEVICE );if (slot->flags ==NCR_700_FLAG_AUTOSENSE ) {char *cmnd =NCR_700_get_sense_cmnd (SCp->device); -#ifdef NCR_700_DEBUG -printk (" ORIGINAL CMD %p RETURNED %d, new return is %d sense is\n" , - SCp, SCp->cmnd[7 ], result); -scsi_print_sense (SCp); -#endif dma_unmap_single (hostdata->dev, slot->dma_handle,SCSI_SENSE_BUFFERSIZE ,DMA_FROM_DEVICE );/* restore the old result if the request sense was
Signature · 1 line
1.8.5.2
PATCH v5 10/24 scsi: stop decoding if scsi_normalize_sense() fails
In reply to Hannes ReineckeIf scsi_normalize_sense() fails we couldn't decode the sense buffer, and the scsi_sense_hdr fields are invalid. For those cases we should rather dump the sense buffer and not try to decode invalid fields.
Reviewed-by Christoph Hellwig <[email protected]>
Signed-off-by Hannes Reinecke <[email protected]>
--- drivers/scsi/constants.c | 34 ++++++++++++++++------------------ 1 file changed, 16 insertions(+), 18 deletions(-)
drivers/scsi/constants.c +16 -18
@@ -1435,26 +1435,21 @@ scsi_print_sense_hdr(struct scsi_device *sdev, const char *name,EXPORT_SYMBOL (scsi_print_sense_hdr);static void -scsi_decode_sense_buffer (const unsigned char *sense_buffer,int sense_len, -struct scsi_sense_hdr *sshdr) +scsi_dump_sense_buffer (const unsigned char *sense_buffer,int sense_len) { -int k, num, res; - - res =scsi_normalize_sense (sense_buffer, sense_len, sshdr); -if (0 == res) { -/* this may be SCSI-1 sense data */ - num = (sense_len <32 ) ? sense_len :32 ; -printk ("Unrecognized sense data (in hex):" ); -for (k =0 ; k < num; ++k) { -if (0 == (k %16 )) { -printk ("\n" ); -printk (KERN_INFO " " ); - } -printk ("%02x " , sense_buffer[k]); +int k, num; + + num = (sense_len <32 ) ? sense_len :32 ; +printk ("Unrecognized sense data (in hex):" ); +for (k =0 ; k < num; ++k) { +if (0 == (k %16 )) { +printk ("\n" ); +printk (KERN_INFO " " ); } -printk ("\n" ); -return ; +printk ("%02x " , sense_buffer[k]); } +printk ("\n" ); +return ; } static void
@@ -1524,7 +1519,10 @@ void __scsi_print_sense(struct scsi_device *sdev, const char *name, {struct scsi_sense_hdr sshdr; -scsi_decode_sense_buffer (sense_buffer, sense_len, &sshdr); +if (!scsi_normalize_sense (sense_buffer, sense_len, &sshdr)) { +scsi_dump_sense_buffer (sense_buffer, sense_len); +return ; + }scsi_show_sense_hdr (sdev, name, &sshdr);scsi_decode_sense_extras (sense_buffer, sense_len, &sshdr);scsi_show_extd_sense (sdev, name, sshdr.asc, sshdr.ascq);
Signature · 1 line
1.8.5.2
PATCH v5 11/24 scsi: do not decode sense extras
In reply to Hannes ReineckeCurrently we're only decoding sense extras for tape devices. And even there only for fixed format sense formats. As this is of rather limited use in the general case we should be stop trying to decode sense extras; the tape driver does its own decoding anyway.
Reviewed-by Christoph Hellwig <[email protected]>
Signed-off-by Hannes Reinecke <[email protected]>
--- drivers/scsi/constants.c | 62 ------------------------------------------------ 1 file changed, 62 deletions(-)
drivers/scsi/constants.c +0 -62
@@ -1452,67 +1452,6 @@ scsi_dump_sense_buffer(const unsigned char *sense_buffer, int sense_len)return ; } -static void -scsi_decode_sense_extras (const unsigned char *sense_buffer,int sense_len, -struct scsi_sense_hdr *sshdr) -{ -int k, num, res; - -if (sshdr->response_code <0x72 ) - { -/* only decode extras for "fixed" format now */ -char buff[80 ]; -int blen, fixed_valid; -unsigned int info; - - fixed_valid = sense_buffer[0 ] &0x80 ; - info = ((sense_buffer[3 ] <<24 ) | (sense_buffer[4 ] <<16 ) | - (sense_buffer[5 ] <<8 ) | sense_buffer[6 ]); - res =0 ; -memset (buff,0 ,sizeof (buff)); - blen =sizeof (buff) -1 ; -if (fixed_valid) - res +=snprintf (buff + res, blen - res, -"Info fld=0x%x" , info); -if (sense_buffer[2 ] &0x80 ) { -/* current command has read a filemark */ -if (res >0 ) - res +=snprintf (buff + res, blen - res,", " ); - res +=snprintf (buff + res, blen - res,"FMK" ); - } -if (sense_buffer[2 ] &0x40 ) { -/* end-of-medium condition exists */ -if (res >0 ) - res +=snprintf (buff + res, blen - res,", " ); - res +=snprintf (buff + res, blen - res,"EOM" ); - } -if (sense_buffer[2 ] &0x20 ) { -/* incorrect block length requested */ -if (res >0 ) - res +=snprintf (buff + res, blen - res,", " ); - res +=snprintf (buff + res, blen - res,"ILI" ); - } -if (res >0 ) -printk ("%s\n" , buff); - }else if (sshdr->additional_length >0 ) { -/* descriptor format with sense descriptors */ - num =8 + sshdr->additional_length; - num = (sense_len < num) ? sense_len : num; -printk ("Descriptor sense data with sense descriptors " -"(in hex):" ); -for (k =0 ; k < num; ++k) { -if (0 == (k %16 )) { -printk ("\n" ); -printk (KERN_INFO " " ); - } -printk ("%02x " , sense_buffer[k]); - } - -printk ("\n" ); - } - -} -/* Normalize and print sense buffer with name prefix */ void __scsi_print_sense (struct scsi_device *sdev,const char *name,const unsigned char *sense_buffer,int sense_len)
@@ -1524,7 +1463,6 @@ void __scsi_print_sense(struct scsi_device *sdev, const char *name,return ; }scsi_show_sense_hdr (sdev, name, &sshdr); -scsi_decode_sense_extras (sense_buffer, sense_len, &sshdr);scsi_show_extd_sense (sdev, name, sshdr.asc, sshdr.ascq); }EXPORT_SYMBOL (__scsi_print_sense);
Signature · 1 line
1.8.5.2
PATCH v5 12/24 scsi: use 'bool' as return value for scsi_normalize_sense()
In reply to Hannes ReineckeConvert scsi_normalize_sense() and frieds to return 'bool' instead of an integer.
Reviewed-by Yoshihiro Yunomae <[email protected]>
Reviewed-by Christoph Hellwig <[email protected]>
Signed-off-by Hannes Reinecke <[email protected]>
--- drivers/scsi/scsi_error.c | 14 +++++++------- drivers/scsi/scsi_lib.c | 2 +- include/scsi/scsi_eh.h | 14 +++++++------- 3 files changed, 15 insertions(+), 15 deletions(-)
drivers/scsi/scsi_error.c +7 -7
@@ -2410,20 +2410,20 @@ EXPORT_SYMBOL(scsi_reset_provider);* responded to a SCSI command with the CHECK_CONDITION status. * * Return value: -* 1 if valid sense data information found, else 0; +* true if valid sense data information found, else false; */ -int scsi_normalize_sense (const u8 *sense_buffer,int sb_len, -struct scsi_sense_hdr *sshdr) +bool scsi_normalize_sense (const u8 *sense_buffer,int sb_len, +struct scsi_sense_hdr *sshdr) {if (!sense_buffer || !sb_len) -return 0 ; +return false;memset (sshdr,0 ,sizeof (struct scsi_sense_hdr )); sshdr->response_code = (sense_buffer[0 ] &0x7f );if (!scsi_sense_valid (sshdr)) -return 0 ; +return false;if (sshdr->response_code >=0x72 ) {/*
@@ -2453,11 +2453,11 @@ int scsi_normalize_sense(const u8 *sense_buffer, int sb_len, } } -return 1 ; +return true; }EXPORT_SYMBOL (scsi_normalize_sense); -int scsi_command_normalize_sense (struct scsi_cmnd *cmd, +bool scsi_command_normalize_sense (struct scsi_cmnd *cmd,struct scsi_sense_hdr *sshdr) { return scsi_normalize_sense(cmd->sense_buffer,
drivers/scsi/scsi_lib.c +1 -1
@@ -831,7 +831,7 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)struct request *req = cmd->request;int error =0 ;struct scsi_sense_hdr sshdr; -int sense_valid =0 ; +bool sense_valid = false;int sense_deferred =0 ;enum {ACTION_FAIL ,ACTION_REPREP ,ACTION_RETRY ,ACTION_DELAYED_RETRY } action;
include/scsi/scsi_eh.h +7 -7
@@ -27,10 +27,10 @@ struct scsi_sense_hdr { /* See SPC-3 section 4.5 */u8 additional_length;/* always 0 for fixed sense format */ }; -static inline int scsi_sense_valid (struct scsi_sense_hdr *sshdr) +static inline bool scsi_sense_valid (struct scsi_sense_hdr *sshdr) {if (!sshdr) -return 0 ; +return false;return (sshdr->response_code &0x70 ) ==0x70 ; }
@@ -42,12 +42,12 @@ extern void scsi_eh_flush_done_q(struct list_head *done_q);extern void scsi_report_bus_reset (struct Scsi_Host *,int );extern void scsi_report_device_reset (struct Scsi_Host *,int ,int );extern int scsi_block_when_processing_errors (struct scsi_device *); -extern int scsi_normalize_sense (const u8 *sense_buffer,int sb_len, -struct scsi_sense_hdr *sshdr); -extern int scsi_command_normalize_sense (struct scsi_cmnd *cmd, -struct scsi_sense_hdr *sshdr); +extern bool scsi_normalize_sense (const u8 *sense_buffer,int sb_len, +struct scsi_sense_hdr *sshdr); +extern bool scsi_command_normalize_sense (struct scsi_cmnd *cmd, +struct scsi_sense_hdr *sshdr); -static inline int scsi_sense_is_deferred (struct scsi_sense_hdr *sshdr) +static inline bool scsi_sense_is_deferred (struct scsi_sense_hdr *sshdr) {return ((sshdr->response_code >=0x70 ) && (sshdr->response_code &1 )); }
Signature · 1 line
1.8.5.2
RE: [PATCH 12/24] scsi: use 'bool' as return value for scsi_normalize_sense()
Quoted 11 lines
-----Original Message-----
From: Hannes Reinecke [mailto:[email protected]]
Sent: Wednesday, 01 October, 2014 1:23 AM
To: James Bottomley
Cc: Christoph Hellwig; [email protected]; Elliott, Robert (Server Storage); Hannes Reinecke
Subject: [PATCH 12/24] scsi: use 'bool' as return value forscsi_normalize_sense()
Quoted 2 lines
Convert
scsi_normalize_sense()and frieds to return 'bool' instead of an integer.
frieds should be friends
...
drivers/scsi/scsi_error.c int scsi_normalize_sense(const u8 *sense_buffer, int sb_len, · line 2461
Quoted 3 lines
drivers/scsi/scsi_error.c +1
-1-int scsi_command_normalize_sense (struct scsi_cmnd *cmd, +bool scsi_command_normalize_sense (struct scsi_cmnd *cmd,struct scsi_sense_hdr *sshdr)
Add const for *cmd since what it points to is not
modified. (prototype in scsi_eh.h)
...
include/scsi/scsi_eh.h struct scsi_sense_hdr { /* See SPC-3 section 4.5 */ · line 36
Quoted 9 lines
include/scsi/scsi_eh.h +2
-2-static inline int scsi_sense_valid (struct scsi_sense_hdr *sshdr) +static inline bool scsi_sense_valid (struct scsi_sense_hdr *sshdr) {if (!sshdr) -return 0 ; +return false;return (sshdr->response_code &0x70 ) ==0x70 ; }
Add const to the argument, since what it points to is not changed by this function:
const struct scsi_sense_hdr *sshdr
include/scsi/scsi_eh.h extern void scsi_eh_flush_done_q(struct list_head *done_q); · line 53
Quoted 5 lines
include/scsi/scsi_eh.h +1
-1-static inline int scsi_sense_is_deferred (struct scsi_sense_hdr *sshdr) +static inline bool scsi_sense_is_deferred (struct scsi_sense_hdr *sshdr) {return ((sshdr->response_code >=0x70 ) && (sshdr->response_code &1 )); }
Add const to the argument, since what it points to is not changed by this function:
const struct scsi_sense_hdr *sshdr
(also mentioned in reply to PATCH 4)
---
Rob Elliott HP Server Storage
PATCH v5 13/24 scsi: remove scsi_print_status()
In reply to Hannes ReineckeLast caller is gone, so we can remove it.
Reviewed-by Christoph Hellwig <[email protected]>
Signed-off-by Hannes Reinecke <[email protected]>
--- drivers/scsi/constants.c | 35 ----------------------------------- include/scsi/scsi_dbg.h | 1 - 2 files changed, 36 deletions(-)
drivers/scsi/constants.c +0 -35
@@ -433,41 +433,6 @@ void scsi_print_command(struct scsi_cmnd *cmd) }EXPORT_SYMBOL (scsi_print_command); -/** -* scsi_print_status - print scsi status description -* @scsi_status: scsi status value -* -* If the status is recognized, the description is printed. -* Otherwise "Unknown status" is output. No trailing space. -* If CONFIG_SCSI_CONSTANTS is not set, then print status in hex -* (e.g. "0x2" for Check Condition). -**/ -void -scsi_print_status (unsigned char scsi_status) { -#ifdef CONFIG_SCSI_CONSTANTS -const char * ccp; - -switch (scsi_status) { -case 0 : ccp ="Good" ;break ; -case 0x2 : ccp ="Check Condition" ;break ; -case 0x4 : ccp ="Condition Met" ;break ; -case 0x8 : ccp ="Busy" ;break ; -case 0x10 : ccp ="Intermediate" ;break ; -case 0x14 : ccp ="Intermediate-Condition Met" ;break ; -case 0x18 : ccp ="Reservation Conflict" ;break ; -case 0x22 : ccp ="Command Terminated" ;break ;/* obsolete */ -case 0x28 : ccp ="Task set Full" ;break ;/* was: Queue Full */ -case 0x30 : ccp ="ACA Active" ;break ; -case 0x40 : ccp ="Task Aborted" ;break ; -default : ccp ="Unknown status" ; - } -printk (KERN_INFO "%s" , ccp); -#else -printk (KERN_INFO "0x%0x" , scsi_status); -#endif -} -EXPORT_SYMBOL (scsi_print_status); -#ifdef CONFIG_SCSI_CONSTANTS struct error_info {
include/scsi/scsi_dbg.h +0 -1
@@ -19,7 +19,6 @@ extern void __scsi_print_sense(struct scsi_device *, const char *name, int sense_len);extern void scsi_show_result (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 ,const char **);
Signature · 1 line
1.8.5.2
PATCH v5 14/24 Implement scsi_opcode_sa_name
In reply to Hannes ReineckeImplement a lookup array for SERVICE ACTION commands instead of hardcoding it in a large switch statement.
Reviewed-by Christoph Hellwig <[email protected]>
Signed-off-by Hannes Reinecke <[email protected]>
--- drivers/scsi/constants.c | 131 +++++++++++++++++++---------------------------- 1 file changed, 53 insertions(+), 78 deletions(-)
drivers/scsi/constants.c +53 -78
@@ -244,102 +244,76 @@ static const struct value_name_pair variable_length_arr[] = { };#define VARIABLE_LENGTH_SZ ARRAY_SIZE(variable_length_arr) -static const char *get_sa_name (const struct value_name_pair * arr, -int arr_sz,int service_action) +struct sa_name_list { +int cmd; +const struct value_name_pair *arr; +int arr_sz; +}; + +static struct sa_name_list sa_names_arr[] = { + {VARIABLE_LENGTH_CMD , variable_length_arr,VARIABLE_LENGTH_SZ }, + {MAINTENANCE_IN , maint_in_arr,MAINT_IN_SZ }, + {MAINTENANCE_OUT , maint_out_arr,MAINT_OUT_SZ }, + {PERSISTENT_RESERVE_IN , pr_in_arr,PR_IN_SZ }, + {PERSISTENT_RESERVE_OUT , pr_out_arr,PR_OUT_SZ }, + {SERVICE_ACTION_IN_12 , serv_in12_arr,SERV_IN12_SZ }, + {SERVICE_ACTION_OUT_12 , serv_out12_arr,SERV_OUT12_SZ }, + {SERVICE_ACTION_BIDIRECTIONAL , serv_bidi_arr,SERV_BIDI_SZ }, + {SERVICE_ACTION_IN_16 , serv_in16_arr,SERV_IN16_SZ }, + {SERVICE_ACTION_OUT_16 , serv_out16_arr,SERV_OUT16_SZ }, + {THIRD_PARTY_COPY_IN , tpc_in_arr,TPC_IN_SZ }, + {THIRD_PARTY_COPY_OUT , tpc_out_arr,TPC_OUT_SZ }, + {0 ,NULL ,0 }, +}; + +static bool scsi_opcode_sa_name (int cmd,int service_action, +const char **sa_name) { -int k; +struct sa_name_list *sa_name_ptr = sa_names_arr; +const struct value_name_pair *arr =NULL ; +int arr_sz, k; + +for (k =0 ; sa_name_ptr->arr; ++k, ++sa_name_ptr) { +if (sa_name_ptr->cmd == cmd) { + arr = sa_name_ptr->arr; + arr_sz = sa_name_ptr->arr_sz; +break ; + } + } +if (!arr) +return false;for (k =0 ; k < arr_sz; ++k, ++arr) {if (service_action == arr->value)break ; } -return (k < arr_sz) ? arr->name :NULL ; +if (k < arr_sz) + *sa_name = arr->name; + +return true; }/* attempt to guess cdb length if cdb_len==0 . No trailing linefeed. */ static void print_opcode_name (unsigned char * cdbp,int cdb_len) {int sa, len, cdb0; -int fin_name =0 ; -const char * name; +const char *name =NULL ; cdb0 = cdbp[0 ]; -switch (cdb0) { -case VARIABLE_LENGTH_CMD : +if (cdb0 ==VARIABLE_LENGTH_CMD ) { len =scsi_varlen_cdb_length (cdbp);if (len <10 ) {printk ("short variable length command, " "len=%d ext_len=%d" , len, cdb_len); -break ; +return ; } sa = (cdbp[8 ] <<8 ) + cdbp[9 ]; - name =get_sa_name (variable_length_arr,VARIABLE_LENGTH_SZ , - sa); -if (name) -printk ("%s" , name); -else -printk ("cdb[0]=0x%x, sa=0x%x" , cdb0, sa); - -if ((cdb_len >0 ) && (len != cdb_len)) -printk (", in_cdb_len=%d, ext_len=%d" , len, cdb_len); - -break ; -case MAINTENANCE_IN : - sa = cdbp[1 ] &0x1f ; - name =get_sa_name (maint_in_arr,MAINT_IN_SZ , sa); - fin_name =1 ; -break ; -case MAINTENANCE_OUT : - sa = cdbp[1 ] &0x1f ; - name =get_sa_name (maint_out_arr,MAINT_OUT_SZ , sa); - fin_name =1 ; -break ; -case PERSISTENT_RESERVE_IN : - sa = cdbp[1 ] &0x1f ; - name =get_sa_name (pr_in_arr,PR_IN_SZ , sa); - fin_name =1 ; -break ; -case PERSISTENT_RESERVE_OUT : - sa = cdbp[1 ] &0x1f ; - name =get_sa_name (pr_out_arr,PR_OUT_SZ , sa); - fin_name =1 ; -break ; -case SERVICE_ACTION_IN_12 : - sa = cdbp[1 ] &0x1f ; - name =get_sa_name (serv_in12_arr,SERV_IN12_SZ , sa); - fin_name =1 ; -break ; -case SERVICE_ACTION_OUT_12 : - sa = cdbp[1 ] &0x1f ; - name =get_sa_name (serv_out12_arr,SERV_OUT12_SZ , sa); - fin_name =1 ; -break ; -case SERVICE_ACTION_BIDIRECTIONAL : - sa = cdbp[1 ] &0x1f ; - name =get_sa_name (serv_bidi_arr,SERV_BIDI_SZ , sa); - fin_name =1 ; -break ; -case SERVICE_ACTION_IN_16 : - sa = cdbp[1 ] &0x1f ; - name =get_sa_name (serv_in16_arr,SERV_IN16_SZ , sa); - fin_name =1 ; -break ; -case SERVICE_ACTION_OUT_16 : - sa = cdbp[1 ] &0x1f ; - name =get_sa_name (serv_out16_arr,SERV_OUT16_SZ , sa); - fin_name =1 ; -break ; -case THIRD_PARTY_COPY_IN : - sa = cdbp[1 ] &0x1f ; - name =get_sa_name (tpc_in_arr,TPC_IN_SZ , sa); - fin_name =1 ; -break ; -case THIRD_PARTY_COPY_OUT : + }else { sa = cdbp[1 ] &0x1f ; - name =get_sa_name (tpc_out_arr,TPC_OUT_SZ , sa); - fin_name =1 ; -break ; -default : + len = cdb_len; + } + +if (!scsi_opcode_sa_name (cdb0, sa, &name)) {if (cdb0 <0xc0 ) { name = cdb_byte0_names[cdb0];if (name)
@@ -348,13 +322,14 @@ static void print_opcode_name(unsigned char * cdbp, int cdb_len)printk ("cdb[0]=0x%x (reserved)" , cdb0); }else printk ("cdb[0]=0x%x (vendor)" , cdb0); -break ; - } -if (fin_name) { + }else {if (name)printk ("%s" , name);else printk ("cdb[0]=0x%x, sa=0x%x" , cdb0, sa); + +if (cdb_len >0 && len != cdb_len) +printk (", in_cdb_len=%d, ext_len=%d" , len, cdb_len); } }
Signature · 1 line
1.8.5.2
RE: [PATCH 14/24] Implement scsi_opcode_sa_name
Quoted 4 lines
-----Original Message-----
From: Hannes Reinecke [mailto:[email protected]]
...
drivers/scsi/constants.c static const struct value_name_pair variable_length_arr[] = { · line 251
Quoted 5 lines
drivers/scsi/constants.c +5
-0+struct sa_name_list { +int cmd; +const struct value_name_pair *arr; +int arr_sz; +};
cmd usually refers to a whole structure and is usually a pointer variable. grep searches will be easier if you name that field "opcode".
Quoted 19 lines
drivers/scsi/constants.c +19
-0+ +static struct sa_name_list sa_names_arr[] = { + {VARIABLE_LENGTH_CMD , variable_length_arr,VARIABLE_LENGTH_SZ }, + {MAINTENANCE_IN , maint_in_arr,MAINT_IN_SZ }, + {MAINTENANCE_OUT , maint_out_arr,MAINT_OUT_SZ }, + {PERSISTENT_RESERVE_IN , pr_in_arr,PR_IN_SZ }, + {PERSISTENT_RESERVE_OUT , pr_out_arr,PR_OUT_SZ }, + {SERVICE_ACTION_IN_12 , serv_in12_arr,SERV_IN12_SZ }, + {SERVICE_ACTION_OUT_12 , serv_out12_arr,SERV_OUT12_SZ }, + {SERVICE_ACTION_BIDIRECTIONAL , serv_bidi_arr,SERV_BIDI_SZ }, + {SERVICE_ACTION_IN_16 , serv_in16_arr,SERV_IN16_SZ }, + {SERVICE_ACTION_OUT_16 , serv_out16_arr,SERV_OUT16_SZ }, + {THIRD_PARTY_COPY_IN , tpc_in_arr,TPC_IN_SZ }, + {THIRD_PARTY_COPY_OUT , tpc_out_arr,TPC_OUT_SZ }, + {0 ,NULL ,0 }, +}; + +static bool scsi_opcode_sa_name (int cmd,int service_action, +const char **sa_name)
...
PATCH v5 15/24 scsi: merge print_opcode_name()
In reply to Hannes ReineckeInstead of having two versions of print_opcode_name() we should be consolidating them into one version.
Reviewed-by Christoph Hellwig <[email protected]>
Signed-off-by Hannes Reinecke <[email protected]>
--- drivers/scsi/constants.c | 90 ++++++++++++++++++------------------------------ 1 file changed, 34 insertions(+), 56 deletions(-)
drivers/scsi/constants.c +34 -56
@@ -30,6 +30,11 @@#define THIRD_PARTY_COPY_IN 0x84 +struct sa_name_list { +int cmd; +const struct value_name_pair *arr; +int arr_sz; +};#ifdef CONFIG_SCSI_CONSTANTS static const char * cdb_byte0_names[] = {
@@ -244,12 +249,6 @@ static const struct value_name_pair variable_length_arr[] = { };#define VARIABLE_LENGTH_SZ ARRAY_SIZE(variable_length_arr) -struct sa_name_list { -int cmd; -const struct value_name_pair *arr; -int arr_sz; -}; -static struct sa_name_list sa_names_arr[] = { {VARIABLE_LENGTH_CMD , variable_length_arr,VARIABLE_LENGTH_SZ }, {MAINTENANCE_IN , maint_in_arr,MAINT_IN_SZ },
@@ -266,6 +265,26 @@ static struct sa_name_list sa_names_arr[] = { {0, NULL, 0}, }; +#else /* ifndef CONFIG_SCSI_CONSTANTS */ +static const char *cdb_byte0_names[]; + +static struct sa_name_list sa_names_arr[] = { + {VARIABLE_LENGTH_CMD, NULL, 0}, + {MAINTENANCE_IN, NULL, 0}, + {MAINTENANCE_OUT, NULL, 0}, + {PERSISTENT_RESERVE_IN, NULL, 0}, + {PERSISTENT_RESERVE_OUT, NULL, 0}, + {SERVICE_ACTION_IN_12, NULL, 0}, + {SERVICE_ACTION_OUT_12, NULL, 0}, + {SERVICE_ACTION_BIDIRECTIONAL, NULL, 0}, + {SERVICE_ACTION_IN_16, NULL, 0}, + {SERVICE_ACTION_OUT_16, NULL, 0}, + {THIRD_PARTY_COPY_IN, NULL, 0}, + {THIRD_PARTY_COPY_OUT, NULL, 0}, + {0, NULL, 0}, +}; +#endif /* CONFIG_SCSI_CONSTANTS */ + static bool scsi_opcode_sa_name(int cmd, int service_action, const char **sa_name) {
@@ -273,7 +292,7 @@ static bool scsi_opcode_sa_name(int cmd, int service_action,const struct value_name_pair *arr =NULL ;int arr_sz, k; -for (k =0 ; sa_name_ptr->arr; ++k, ++sa_name_ptr) { +for (k =0 ; k <ARRAY_SIZE (sa_names_arr); ++k, ++sa_name_ptr) {if (sa_name_ptr->cmd == cmd) { arr = sa_name_ptr->arr; arr_sz = sa_name_ptr->arr_sz;
@@ -315,11 +334,14 @@ static void print_opcode_name(unsigned char * cdbp, int cdb_len)if (!scsi_opcode_sa_name (cdb0, sa, &name)) {if (cdb0 <0xc0 ) { - name = cdb_byte0_names[cdb0]; -if (name) -printk ("%s" , name); -else -printk ("cdb[0]=0x%x (reserved)" , cdb0); +if (ARRAY_SIZE (cdb_byte0_names)) { + name = cdb_byte0_names[cdb0]; +if (name) +printk ("%s" , name); +else +printk ("cdb[0]=0x%x (reserved)" , cdb0); + }else +printk ("cdb[0]=0x%x" , cdb0); }else printk ("cdb[0]=0x%x (vendor)" , cdb0); }else {
@@ -333,50 +355,6 @@ static void print_opcode_name(unsigned char * cdbp, int cdb_len) } } -#else /* ifndef CONFIG_SCSI_CONSTANTS */ - -static void print_opcode_name (unsigned char * cdbp,int cdb_len) -{ -int sa, len, cdb0; - - cdb0 = cdbp[0 ]; -switch (cdb0) { -case VARIABLE_LENGTH_CMD : - len =scsi_varlen_cdb_length (cdbp); -if (len <10 ) { -printk ("short opcode=0x%x command, len=%d " -"ext_len=%d" , cdb0, len, cdb_len); -break ; - } - sa = (cdbp[8 ] <<8 ) + cdbp[9 ]; -printk ("cdb[0]=0x%x, sa=0x%x" , cdb0, sa); -if (len != cdb_len) -printk (", in_cdb_len=%d, ext_len=%d" , len, cdb_len); -break ; -case MAINTENANCE_IN : -case MAINTENANCE_OUT : -case PERSISTENT_RESERVE_IN : -case PERSISTENT_RESERVE_OUT : -case SERVICE_ACTION_IN_12 : -case SERVICE_ACTION_OUT_12 : -case SERVICE_ACTION_BIDIRECTIONAL : -case SERVICE_ACTION_IN_16 : -case SERVICE_ACTION_OUT_16 : -case THIRD_PARTY_COPY_IN : -case THIRD_PARTY_COPY_OUT : - sa = cdbp[1 ] &0x1f ; -printk ("cdb[0]=0x%x, sa=0x%x" , cdb0, sa); -break ; -default : -if (cdb0 <0xc0 ) -printk ("cdb[0]=0x%x" , cdb0); -else -printk ("cdb[0]=0x%x (vendor)" , cdb0); -break ; - } -} -#endif -void __scsi_print_command (unsigned char *cdb) {int k, len;
Signature · 1 line
1.8.5.2
RE: [PATCH 15/24] scsi: merge print_opcode_name()
drivers/scsi/constants.c line 37
Quoted 9 lines
-----Original Message-----
From: Hannes Reinecke [mailto:[email protected]]drivers/scsi/constants.c +5
-0+struct sa_name_list { +int cmd; +const struct value_name_pair *arr; +int arr_sz; +};
The suggestion to rename cmd to opcode in patch 14 would follow the movements here.
...
Quoted 18 lines
drivers/scsi/constants.c +1
-1@@ -273,7 +292,7 @@ static bool scsi_opcode_sa_name(int cmd, int service_action, const struct value_name_pair *arr = NULL;int arr_sz, k; -for (k =0 ; sa_name_ptr->arr; ++k, ++sa_name_ptr) { +for (k =0 ; k <ARRAY_SIZE (sa_names_arr); ++k, ++sa_name_ptr) {if (sa_name_ptr->cmd == cmd) { arr = sa_name_ptr->arr;drivers/scsi/constants.c +0
-3arr_sz = sa_name_ptr->arr_sz;@@ -315,11 +334,14 @@ static void print_opcode_name(unsigned char * cdbp, int cdb_len)if (!scsi_opcode_sa_name (cdb0, sa, &name)) {if (cdb0 <0xc0 ) { - name = cdb_byte0_names[cdb0]; -if (name) -printk ("%s" , name);
drivers/scsi/constants.c static void print_opcode_name(unsigned char * cdbp, int · line 345
Quoted 10 lines
drivers/scsi/constants.c +8
-2-else -printk ("cdb[0]=0x%x (reserved)" , cdb0); +if (ARRAY_SIZE (cdb_byte0_names)) { + name = cdb_byte0_names[cdb0]; +if (name) +printk ("%s" , name); +else +printk ("cdb[0]=0x%x (reserved)" , cdb0); + }else +printk ("cdb[0]=0x%x" , cdb0);
Just ARRAY_SIZE(cdb_byte0_names) is not right - that array is always defined and the statement is always true.
Patch 16 replaces that block of code and eliminates that issue.
---
Rob Elliott HP Server Storage
PATCH v5 16/24 scsi: consolidate opcode lookup in scsi_opcode_sa_name()
In reply to Hannes ReineckeConsolidate the CDB opcode lookup in scsi_opcode_sa_name(), so that we don't have to call several functions to figure out the CDB opcode string.
Reviewed-by Christoph Hellweg <[email protected]>
Signed-off-by Hannes Reinecke <[email protected]>
--- drivers/scsi/constants.c | 37 ++++++++++++++++++++++--------------- 1 file changed, 22 insertions(+), 15 deletions(-)
drivers/scsi/constants.c +22 -15
@@ -29,6 +29,7 @@#define THIRD_PARTY_COPY_OUT 0x83#define THIRD_PARTY_COPY_IN 0x84 +#define VENDOR_SPECIFIC_CDB 0xc0struct sa_name_list {int cmd;
@@ -286,12 +287,19 @@ static struct sa_name_list sa_names_arr[] = {#endif /* CONFIG_SCSI_CONSTANTS */ static bool scsi_opcode_sa_name (int cmd,int service_action, -const char **sa_name) +const char **cdb_name,const char **sa_name) {struct sa_name_list *sa_name_ptr = sa_names_arr;const struct value_name_pair *arr =NULL ;int arr_sz, k; + *cdb_name =NULL ; +if (cmd >=VENDOR_SPECIFIC_CDB ) +return false; + +if (cmd <ARRAY_SIZE (cdb_byte0_names)) + *cdb_name = cdb_byte0_names[cmd]; +for (k =0 ; k <ARRAY_SIZE (sa_names_arr); ++k, ++sa_name_ptr) {if (sa_name_ptr->cmd == cmd) { arr = sa_name_ptr->arr;
@@ -316,7 +324,7 @@ static bool scsi_opcode_sa_name(int cmd, int service_action,static void print_opcode_name (unsigned char * cdbp,int cdb_len) {int sa, len, cdb0; -const char *name =NULL ; +const char *cdb_name =NULL , *sa_name =NULL ; cdb0 = cdbp[0 ];if (cdb0 ==VARIABLE_LENGTH_CMD ) {
@@ -332,21 +340,20 @@ static void print_opcode_name(unsigned char * cdbp, int cdb_len) len = cdb_len; } -if (!scsi_opcode_sa_name (cdb0, sa, &name)) { -if (cdb0 <0xc0 ) { -if (ARRAY_SIZE (cdb_byte0_names)) { - name = cdb_byte0_names[cdb0]; -if (name) -printk ("%s" , name); -else -printk ("cdb[0]=0x%x (reserved)" , cdb0); - }else -printk ("cdb[0]=0x%x" , cdb0); - }else +if (!scsi_opcode_sa_name (cdb0, sa, &cdb_name, &sa_name)) { +if (cdb_name) +printk ("%s" , cdb_name); +else if (cdb0 >VENDOR_SPECIFIC_CDB )printk ("cdb[0]=0x%x (vendor)" , cdb0); +else if (cdb0 >0x60 && cdb0 <0x7e ) +printk ("cdb[0]=0x%x (reserved)" , cdb0); +else +printk ("cdb[0]=0x%x" , cdb0); }else { -if (name) -printk ("%s" , name); +if (sa_name) +printk ("%s" , sa_name); +else if (cdb_name) +printk ("%s, sa=0x%x" , cdb_name, sa);else printk ("cdb[0]=0x%x, sa=0x%x" , cdb0, sa);
Signature · 1 line
1.8.5.2
RE: [PATCH 16/24] scsi: consolidate opcode lookup in scsi_opcode_sa_name()
Quoted 4 lines
-----Original Message-----
From: Hannes Reinecke [mailto:[email protected]]
...
Quoted 1 line
drivers/scsi/constants.c +0
-0
...
Quoted 15 lines
drivers/scsi/constants.c +0
-10@@ -332,21 +340,20 @@ static void print_opcode_name(unsigned char * cdbp, int cdb_len) len = cdb_len; } -if (!scsi_opcode_sa_name (cdb0, sa, &name)) { -if (cdb0 <0xc0 ) { -if (ARRAY_SIZE (cdb_byte0_names)) { - name = cdb_byte0_names[cdb0]; -if (name) -printk ("%s" , name); -else -printk ("cdb[0]=0x%x (reserved)" , cdb0); - }else -printk ("cdb[0]=0x%x" , cdb0);
drivers/scsi/constants.c static void print_opcode_name(unsigned char * cdbp, int · line 352
Quoted 10 lines
drivers/scsi/constants.c +8
-1- }else +if (!scsi_opcode_sa_name (cdb0, sa, &cdb_name, &sa_name)) { +if (cdb_name) +printk ("%s" , cdb_name); +else if (cdb0 >VENDOR_SPECIFIC_CDB )printk ("cdb[0]=0x%x (vendor)" , cdb0); +else if (cdb0 >0x60 && cdb0 <0x7e ) +printk ("cdb[0]=0x%x (reserved)" , cdb0); +else +printk ("cdb[0]=0x%x" , cdb0);
Both of those > need to be >=, since the reserved
range starts at and includes 0x60, and the vendor
specific range starts at and includes 0xc0.
---
Rob Elliott HP Server Storage
PATCH v5 17/24 scsi: remove last argument from print_opcode_name()
In reply to Hannes Reineckeprint_opcode_name() was only ever called with a '0' argument from LLDDs and ULDs which were _not_ supporting variable length CDBs, so the 'if' clause was never triggered.
Reviewed-by Christoph Hellwig <[email protected]>
Signed-off-by Hannes Reinecke <[email protected]>
--- drivers/scsi/constants.c | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-)
drivers/scsi/constants.c +7 -14
@@ -320,25 +320,21 @@ static bool scsi_opcode_sa_name(int cmd, int service_action,return true; } -/* attempt to guess cdb length if cdb_len==0 . No trailing linefeed. */ -static void print_opcode_name (unsigned char * cdbp,int cdb_len) +static void print_opcode_name (unsigned char *cdbp) { -int sa, len, cdb0; +int sa, cdb0;const char *cdb_name =NULL , *sa_name =NULL ; cdb0 = cdbp[0 ];if (cdb0 ==VARIABLE_LENGTH_CMD ) { - len =scsi_varlen_cdb_length (cdbp); +int len =scsi_varlen_cdb_length (cdbp);if (len <10 ) { -printk ("short variable length command, " -"len=%d ext_len=%d" , len, cdb_len); +printk ("short variable length command, len=%d" , len);return ; } sa = (cdbp[8 ] <<8 ) + cdbp[9 ]; - }else { + }else sa = cdbp[1 ] &0x1f ; - len = cdb_len; - }if (!scsi_opcode_sa_name (cdb0, sa, &cdb_name, &sa_name)) {if (cdb_name)
@@ -356,9 +352,6 @@ static void print_opcode_name(unsigned char * cdbp, int cdb_len)printk ("%s, sa=0x%x" , cdb_name, sa); else printk("cdb[0]=0x%x, sa=0x%x", cdb0, sa); - -if (cdb_len >0 && len != cdb_len) -printk (", in_cdb_len=%d, ext_len=%d" , len, cdb_len); } }
@@ -366,7 +359,7 @@ void __scsi_print_command(unsigned char *cdb) {int k, len; -print_opcode_name (cdb,0 ); +print_opcode_name (cdb); len =scsi_command_size (cdb);/* print out all bytes in cdb */ for (k =0 ; k < len; ++k)
@@ -383,7 +376,7 @@ void scsi_print_command(struct scsi_cmnd *cmd)return ;scmd_printk (KERN_INFO , cmd,"CDB: " ); -print_opcode_name (cmd->cmnd, cmd->cmd_len); +print_opcode_name (cmd->cmnd);/* print out all bytes in cdb */ printk (":" );
Signature · 1 line
1.8.5.2
RE: [PATCH 17/24] scsi: remove last argument from print_opcode_name()
Quoted 25 lines
-----Original Message-----
From: Hannes Reinecke [mailto:[email protected]]
Sent: Wednesday, 01 October, 2014 1:23 AM
To: James Bottomley
Cc: Christoph Hellwig; [email protected]; Elliott, Robert (Server Storage); Hannes Reinecke
Subject: [PATCH 17/24] scsi: remove last argument fromprint_opcode_name()
print_opcode_name()was only ever called with a '0' argument from LLDDs and ULDs which were _not_ supporting variable length CDBs, so the 'if' clause was never triggered.Reviewed-by Christoph Hellwig <[email protected]>Signed-off-by Hannes Reinecke <[email protected]>--- drivers/scsi/constants.c | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-)drivers/scsi/constants.c +0
-0
drivers/scsi/constants.c static bool scsi_opcode_sa_name(int cmd, int · line 324
Quoted 7 lines
drivers/scsi/constants.c +1
-2@@ -320,25 +320,21 @@ static bool scsi_opcode_sa_name(int cmd, int service_action, return true; } -/* attempt to guess cdb length if cdb_len==0 . No trailing linefeed. */ -static void print_opcode_name (unsigned char * cdbp,int cdb_len) +static void print_opcode_name (unsigned char *cdbp)
Add const since what cdbp points to is not modified.
drivers/scsi/constants.c static bool scsi_opcode_sa_name(int cmd, int service_action, · line 330
Quoted 9 lines
drivers/scsi/constants.c +2
-2{ -int sa, len, cdb0; +int sa, cdb0;const char *cdb_name =NULL , *sa_name =NULL ; cdb0 = cdbp[0 ];if (cdb0 ==VARIABLE_LENGTH_CMD ) { - len =scsi_varlen_cdb_length (cdbp); +int len =scsi_varlen_cdb_length (cdbp);
cdbp must point to a buffer containing at least 8 valid bytes for that to be safe. Is that guaranteed?
drivers/scsi/constants.c static bool scsi_opcode_sa_name(int cmd, int service_action, · line 337
Quoted 10 lines
drivers/scsi/constants.c +2
-3if (len <10 ) { -printk ("short variable length command, " -"len=%d ext_len=%d" , len, cdb_len); +printk ("short variable length command, len=%d" , len);return ; } sa = (cdbp[8 ] <<8 ) + cdbp[9 ]; - }else { + }else sa = cdbp[1 ] &0x1f ;
cdbp must point to a buffer containing at least 2 valid bytes for that to be safe. Is that guaranteed?
Quoted 17 lines
drivers/scsi/constants.c +0
-2- len = cdb_len; - }if (!scsi_opcode_sa_name (cdb0, sa, &cdb_name, &sa_name)) {drivers/scsi/constants.c +0
-3if (cdb_name)@@ -356,9 +352,6 @@ static void print_opcode_name(unsigned char * cdbp, int cdb_len) printk("%s, sa=0x%x", cdb_name, sa); else printk("cdb[0]=0x%x, sa=0x%x", cdb0, sa); - -if (cdb_len >0 && len != cdb_len) -printk (", in_cdb_len=%d, ext_len=%d" , len, cdb_len); } }@@ -366,7 +359,7 @@ void __scsi_print_command(unsigned char *cdb)
...
drivers/scsi/constants.c void scsi_print_command(struct scsi_cmnd *cmd)
Quoted 1 line
drivers/scsi/constants.c +0
-0@@ -383,7 +376,7 @@ void scsi_print_command(struct scsi_cmnd *cmd)
Add const to each of those, since what cdbp and cmd point to is not modified.
---
Rob Elliott HP Server Storage
PATCH v5 18/24 scsi: Remove scsi_print_command when calling abort
In reply to Hannes ReineckeCalling scsi_print_command should not be necessary during abort; if the information is required one should enable scsi logging.
Reviewed-by Christoph Hellwig <[email protected]>
Signed-off-by Hannes Reinecke <[email protected]>
--- drivers/scsi/53c700.c | 4 +--- drivers/scsi/NCR5380.c | 5 ++--- drivers/scsi/arm/fas216.c | 10 +++------- drivers/scsi/atari_NCR5380.c | 3 +-- drivers/scsi/ps3rom.c | 4 ---- drivers/scsi/stex.c | 9 +++------ drivers/scsi/sun3_NCR5380.c | 3 +-- 7 files changed, 11 insertions(+), 27 deletions(-)
drivers/scsi/53c700.c +1 -3
@@ -1906,9 +1906,7 @@ NCR_700_abort(struct scsi_cmnd * SCp) {struct NCR_700_command_slot *slot; -scmd_printk (KERN_INFO , SCp, -"New error handler wants to abort command\n\t" ); -scsi_print_command (SCp); +scmd_printk (KERN_INFO , SCp,"abort command\n" ); slot = (struct NCR_700_command_slot *)SCp->host_scribble;
drivers/scsi/NCR5380.c +2 -3
@@ -2666,9 +2666,8 @@ static int NCR5380_abort(Scsi_Cmnd * cmd) {struct Scsi_Host *instance = cmd->device->host;struct NCR5380_hostdata *hostdata = (struct NCR5380_hostdata *) instance->hostdata;Scsi_Cmnd *tmp, **prev; - -printk (KERN_WARNING "scsi%d : aborting command\n" , instance->host_no); -scsi_print_command (cmd); + +scmd_printk (KERN_WARNING , cmd,"aborting command\n" );NCR5380_print_status (instance);
drivers/scsi/arm/fas216.c +3 -7
@@ -2425,14 +2425,11 @@ int fas216_eh_abort(struct scsi_cmnd *SCpnt) info->stats.aborts +=1 ; -printk (KERN_WARNING "scsi%d: abort command " , info->host->host_no); -__scsi_print_command (SCpnt->cmnd); +sdev_printk (KERN_WARNING , SCpnt,"abort command\n" );print_debug_list ();fas216_dumpstate (info); -printk (KERN_WARNING "scsi%d: abort %p " , info->host->host_no, SCpnt); -switch (fas216_find_command (info, SCpnt)) {/* * We found the command, and cleared it out. Either
@@ -2440,7 +2437,7 @@ int fas216_eh_abort(struct scsi_cmnd *SCpnt)* target, or the busylun bit is not set. */ case res_success: -printk ("success\n" ); +sdev_printk (KERN_WARNING , SCpnt,"abort %p success\n" , SCpnt); result =SUCCESS ;break ;
@@ -2450,14 +2447,13 @@ int fas216_eh_abort(struct scsi_cmnd *SCpnt)* if the bus is free. */ case res_hw_abort: -/* * We are unable to abort the command for some reason. */ default :case res_failed: -printk ("failed\n" ); +sdev_printk (KERN_WARNING , SCpnt,"abort %p failed\n" , SCpnt);break ; }
drivers/scsi/atari_NCR5380.c +1 -2
@@ -2623,8 +2623,7 @@ int NCR5380_abort(Scsi_Cmnd *cmd)Scsi_Cmnd *tmp, **prev;unsigned long flags; -printk (KERN_NOTICE "scsi%d: aborting command\n" ,HOSTNO ); -scsi_print_command (cmd); +sdev_printk (KERN_NOTICE , cmd,"aborting command\n" );NCR5380_print_status (instance);
drivers/scsi/ps3rom.c +0 -4
@@ -220,10 +220,6 @@ static int ps3rom_queuecommand_lck(struct scsi_cmnd *cmd,unsigned char opcode;int res; -#ifdef DEBUG -scsi_print_command (cmd); -#endif - priv->curr_cmd = cmd; cmd->scsi_done = done;
drivers/scsi/stex.c +3 -6
@@ -1162,9 +1162,7 @@ static int stex_abort(struct scsi_cmnd *cmd)int result =SUCCESS ;unsigned long flags; - printk(KERN_INFO DRV_NAME - "(%s): aborting command\n", pci_name(hba->pdev)); -scsi_print_command (cmd); +scmd_printk (KERN_INFO , cmd,"aborting command\n" ); base = hba->mmio_base;spin_lock_irqsave (host->host_lock, flags);
@@ -1352,9 +1350,8 @@ static int stex_reset(struct scsi_cmnd *cmd) hba = (struct st_hba *) &cmd->device->host->hostdata[0 ]; - printk(KERN_INFO DRV_NAME - "(%s): resetting host\n", pci_name(hba->pdev)); -scsi_print_command (cmd); +shost_printk (KERN_INFO , cmd->device->host, +"resetting host\n" );return stex_do_reset (hba) ?FAILED :SUCCESS ; }
drivers/scsi/sun3_NCR5380.c +1 -2
@@ -2608,8 +2608,7 @@ static int NCR5380_abort(struct scsi_cmnd *cmd)struct scsi_cmnd *tmp, **prev;unsigned long flags; -printk (KERN_NOTICE "scsi%d: aborting command\n" ,HOSTNO ); -scsi_print_command (cmd); +sdev_printk (KERN_NOTICE , cmd,"aborting command\n" );NCR5380_print_status (instance);
Signature · 1 line
1.8.5.2
RE: [PATCH 18/24] scsi: Remove scsi_print_command when calling abort
Quoted 13 lines
-----Original Message-----
From: Hannes Reinecke [mailto:[email protected]]
Sent: Wednesday, 01 October, 2014 1:23 AM
To: James Bottomley
Cc: Christoph Hellwig; [email protected]; Elliott, Robert (Server Storage); Hannes Reinecke
Subject: [PATCH 18/24] scsi: Removescsi_print_commandwhen calling abortCalling
scsi_print_commandshould not be necessary during abort; if the information is required one should enable scsi logging.
...
Quoted 22 lines
drivers/scsi/NCR5380.c +2
-3@@ -2666,9 +2666,8 @@ static int NCR5380_abort(Scsi_Cmnd * cmd) {struct Scsi_Host *instance = cmd->device->host;struct NCR5380_hostdata *hostdata = (struct NCR5380_hostdata *) instance->hostdata;Scsi_Cmnd *tmp, **prev; - -printk (KERN_WARNING "scsi%d : aborting command\n" , instance->host_no); -scsi_print_command (cmd); + +scmd_printk (KERN_WARNING , cmd,"aborting command\n" );NCR5380_print_status (instance);drivers/scsi/arm/fas216.c +0
-0
drivers/scsi/arm/fas216.c int fas216_eh_abort(struct scsi_cmnd *SCpnt) · line 2428
Quoted 6 lines
drivers/scsi/arm/fas216.c +1
-2@@ -2425,14 +2425,11 @@ int fas216_eh_abort(struct scsi_cmnd *SCpnt) info->stats.aborts +=1 ; -printk (KERN_WARNING "scsi%d: abort command " , info->host->host_no); -__scsi_print_command (SCpnt->cmnd); +sdev_printk (KERN_WARNING , SCpnt,"abort command\n" );
That should be scmd_printk.
It'd be nice if the s*_printk macros could typecheck that pointer.
Quoted 10 lines
drivers/scsi/arm/fas216.c +0
-2print_debug_list ();fas216_dumpstate (info); -printk (KERN_WARNING "scsi%d: abort %p " , info->host->host_no, SCpnt); -switch (fas216_find_command (info, SCpnt)) {/* * We found the command, and cleared it out. Either
drivers/scsi/arm/fas216.c int fas216_eh_abort(struct scsi_cmnd *SCpnt) · line 2440
Quoted 5 lines
drivers/scsi/arm/fas216.c +1
-1@@ -2440,7 +2437,7 @@ int fas216_eh_abort(struct scsi_cmnd *SCpnt)* target, or the busylun bit is not set. */ case res_success: -printk ("success\n" ); +sdev_printk (KERN_WARNING , SCpnt,"abort %p success\n" , SCpnt);
That should be scmd_printk, dropping the %p and the extra SCpnt argument.
Quoted 6 lines
drivers/scsi/arm/fas216.c +0
-0result =SUCCESS ;break ;@@ -2450,14 +2447,13 @@ int fas216_eh_abort(struct scsi_cmnd *SCpnt)* if the bus is free. */
drivers/scsi/arm/fas216.c int fas216_eh_abort(struct scsi_cmnd *SCpnt) · line 2456
Quoted 10 lines
drivers/scsi/arm/fas216.c +1
-2case res_hw_abort: -/* * We are unable to abort the command for some reason. */ default :case res_failed: -printk ("failed\n" ); +sdev_printk (KERN_WARNING , SCpnt,"abort %p failed\n" , SCpnt);
That should be scmd_printk, dropping the %p and the extra SCpnt argument.
Quoted 8 lines
drivers/scsi/arm/fas216.c +0
-0break ; }drivers/scsi/atari_NCR5380.c +0
-0
drivers/scsi/atari_NCR5380.c int NCR5380_abort(Scsi_Cmnd *cmd) · line 2626
Quoted 6 lines
drivers/scsi/atari_NCR5380.c +1
-2@@ -2623,8 +2623,7 @@ int NCR5380_abort(Scsi_Cmnd *cmd)Scsi_Cmnd *tmp, **prev;unsigned long flags; -printk (KERN_NOTICE "scsi%d: aborting command\n" ,HOSTNO ); -scsi_print_command (cmd); +sdev_printk (KERN_NOTICE , cmd,"aborting command\n" );
That should be scmd_printk.
...
drivers/scsi/sun3_NCR5380.c static int NCR5380_abort(struct scsi_cmnd *cmd) · line 2611
Quoted 11 lines
drivers/scsi/sun3_NCR5380.c +1
-2@@ -2608,8 +2608,7 @@ static int NCR5380_abort(struct scsi_cmnd *cmd)struct scsi_cmnd *tmp, **prev;unsigned long flags; -printk (KERN_NOTICE "scsi%d: aborting command\n" ,HOSTNO ); -scsi_print_command (cmd); +sdev_printk (KERN_NOTICE , cmd,"aborting command\n" );
That should be scmd_printk.
---
Rob Elliott HP Server Storage
PATCH v5 19/24 scsi: separate out scsi_(host|driver)byte_string()
In reply to Hannes ReineckeExport functions for later use.
Reviewed-by Christoph Hellwig <[email protected]>
Signed-off-by Hannes Reinecke <[email protected]>
--- drivers/scsi/constants.c | 58 ++++++++++++++++++++++++++++++++++++------------ include/scsi/scsi_dbg.h | 2 ++ 2 files changed, 46 insertions(+), 14 deletions(-)
drivers/scsi/constants.c +44 -14
@@ -1404,38 +1404,68 @@ static const char * const hostbyte_table[]={"DID_PASSTHROUGH" ,"DID_SOFT_ERROR" ,"DID_IMM_RETRY" ,"DID_REQUEUE" ,"DID_TRANSPORT_DISRUPTED" ,"DID_TRANSPORT_FAILFAST" ,"DID_TARGET_FAILURE" , "DID_NEXUS_FAILURE" }; -#define NUM_HOSTBYTE_STRS ARRAY_SIZE(hostbyte_table)static const char *const driverbyte_table[]={"DRIVER_OK" ,"DRIVER_BUSY" ,"DRIVER_SOFT" ,"DRIVER_MEDIA" ,"DRIVER_ERROR" ,"DRIVER_INVALID" ,"DRIVER_TIMEOUT" ,"DRIVER_HARD" ,"DRIVER_SENSE" }; -#define NUM_DRIVERBYTE_STRS ARRAY_SIZE(driverbyte_table) -void scsi_show_result (int result) +#endif + +const char *scsi_hostbyte_string (int result) { +const char *hb_string =NULL ; +#ifdef CONFIG_SCSI_CONSTANTS int hb =host_byte (result); -int db =driver_byte (result); -printk ("Result: hostbyte=%s driverbyte=%s\n" , - (hb <NUM_HOSTBYTE_STRS ? hostbyte_table[hb] :"invalid" ), - (db <NUM_DRIVERBYTE_STRS ? driverbyte_table[db] :"invalid" )); +if (hb <ARRAY_SIZE (hostbyte_table)) + hb_string = hostbyte_table[hb]; +#endif +return hb_string; } +EXPORT_SYMBOL (scsi_hostbyte_string); -#else +const char *scsi_driverbyte_string (int result) +{ +const char *db_string =NULL ; +#ifdef CONFIG_SCSI_CONSTANTS +int db =driver_byte (result); + +if (db <ARRAY_SIZE (driverbyte_table)) + db_string = driverbyte_table[db]; +#endif +return db_string; +} +EXPORT_SYMBOL (scsi_driverbyte_string);void scsi_show_result (int result) { -printk ("Result: hostbyte=0x%02x driverbyte=0x%02x\n" , -host_byte (result),driver_byte (result)); -} +const char *hb_string =scsi_hostbyte_string (result); +const char *db_string =scsi_driverbyte_string (result); -#endif +if (hb_string || db_string) +printk ("Result: hostbyte=%s driverbyte=%s\n" , + hb_string ? hb_string :"invalid" , + db_string ? db_string :"invalid" ); +else +printk ("Result: hostbyte=0x%02x driverbyte=0x%02x\n" , +host_byte (result),driver_byte (result)); +}EXPORT_SYMBOL (scsi_show_result);void scsi_print_result (struct scsi_cmnd *cmd) { -scmd_printk (KERN_INFO , cmd," " ); -scsi_show_result (cmd->result); +const char *hb_string =scsi_hostbyte_string (cmd->result); +const char *db_string =scsi_driverbyte_string (cmd->result); + +if (hb_string || db_string) +scmd_printk (KERN_INFO , cmd, +"Result: hostbyte=%s driverbyte=%s" , + hb_string ? hb_string :"invalid" , + db_string ? db_string :"invalid" ); +else +scmd_printk (KERN_INFO , cmd, +"Result: hostbyte=0x%02x driverbyte=0x%02x" , +host_byte (cmd->result),driver_byte (cmd->result)); }EXPORT_SYMBOL (scsi_print_result);
include/scsi/scsi_dbg.h +2 -0
@@ -19,6 +19,8 @@ extern void __scsi_print_sense(struct scsi_device *, const char *name, int sense_len);extern void scsi_show_result (int );extern void scsi_print_result (struct scsi_cmnd *); +extern const char *scsi_hostbyte_string (int ); +extern const char *scsi_driverbyte_string (int );extern const char *scsi_sense_key_string (unsigned char );extern const char *scsi_extd_sense_format (unsigned char ,unsigned char ,const char **);
Signature · 1 line
1.8.5.2
PATCH v5 20/24 sd: Cleanup logging
In reply to Hannes ReineckeOpen-code scsi_print_result in sd.c, and cleanup logging to not print duplicate informations.
With that we can remove scsi_show_result in constants.c
Reviewed-by Christoph Hellwig <[email protected]>
Signed-off-by Hannes Reinecke <[email protected]>
--- drivers/scsi/constants.c | 16 ---------------- drivers/scsi/sd.c | 45 +++++++++++++++++++++++---------------------- include/scsi/scsi_dbg.h | 1 - 3 files changed, 23 insertions(+), 39 deletions(-)
drivers/scsi/constants.c +0 -16
@@ -1437,22 +1437,6 @@ const char *scsi_driverbyte_string(int result) }EXPORT_SYMBOL (scsi_driverbyte_string); -void scsi_show_result (int result) -{ -const char *hb_string =scsi_hostbyte_string (result); -const char *db_string =scsi_driverbyte_string (result); - -if (hb_string || db_string) -printk ("Result: hostbyte=%s driverbyte=%s\n" , - hb_string ? hb_string :"invalid" , - db_string ? db_string :"invalid" ); -else -printk ("Result: hostbyte=0x%02x driverbyte=0x%02x\n" , -host_byte (result),driver_byte (result)); -} -EXPORT_SYMBOL (scsi_show_result); - -void scsi_print_result (struct scsi_cmnd *cmd) {const char *hb_string =scsi_hostbyte_string (cmd->result);
drivers/scsi/sd.c +23 -22
90 lines not on this page · raw message
include/scsi/scsi_dbg.h +0 -1
@@ -17,7 +17,6 @@ extern void scsi_print_sense(struct scsi_cmnd *);extern void __scsi_print_sense (struct scsi_device *,const char *name,const unsigned char *sense_buffer,int sense_len); -extern void scsi_show_result (int );extern void scsi_print_result (struct scsi_cmnd *);extern const char *scsi_hostbyte_string (int );extern const char *scsi_driverbyte_string (int );
Signature · 1 line
1.8.5.2
RE: [PATCH 20/24] sd: Cleanup logging
Quoted 1 line
From: Hannes Reinecke [mailto:[email protected]]
...
Quoted 14 lines
drivers/scsi/sd.c +1
-1@@ -116,7 +116,7 @@ static int sd_eh_action(struct scsi_cmnd *, int);static void sd_read_capacity (struct scsi_disk *sdkp,unsigned char *buffer);static void scsi_disk_release (struct device *cdev);static void sd_print_sense_hdr (struct scsi_disk *,struct scsi_sense_hdr *); -static void sd_print_result (struct scsi_disk *,int ); +static void sd_print_result (struct scsi_disk *,const char *,int );static DEFINE_SPINLOCK (sd_index_lock);static DEFINE_IDA (sd_index_ida);
drivers/scsi/sd.c static int sd_sync_cache(struct scsi_disk *sdkp) · line 1482
Quoted 5 lines
drivers/scsi/sd.c +1
-1@@ -1479,7 +1479,7 @@ static int sd_sync_cache(struct scsi_disk *sdkp) }if (res) { -sd_print_result (sdkp, res); +sd_print_result (sdkp,"SYNCHRONIZE_CACHE failed" , res);
The cdb_byte0_names[] array in constants.c capitalizes the candidates as:
Synchronize Cache(10)
Synchronize cache(16)
Here, it's specifically the 10 byte version (so far), so
Synchronize Cache(10) failed
might be a better string. Or, if following SCSI standards,
SYNCHRONIZE CACHE(10)
without the _ would be better.
Consider changing cdb_byte0_names to Synchronize Cache(16) too. (it has a lot of inconsistent capitalization)
Quoted 11 lines
drivers/scsi/sd.c +0
-4if (driver_byte (res) &DRIVER_SENSE )sd_print_sense_hdr (sdkp, &sshdr);@@ -1700,17 +1700,9 @@ static int sd_done(struct scsi_cmnd *SCpnt)if (sense_valid) sense_deferred =scsi_sense_is_deferred (&sshdr); } -#ifdef CONFIG_SCSI_LOGGING -SCSI_LOG_HLCOMPLETE (1 ,scsi_print_result (SCpnt)); -if (sense_valid) { - SCSI_LOG_HLCOMPLETE(1, scmd_printk(KERN_INFO, SCpnt,
drivers/scsi/sd.c static int sd_done(struct scsi_cmnd *SCpnt) · line 1705
Quoted 10 lines
drivers/scsi/sd.c +3
-7-"sd_done: sb[respc,sk,asc," -"ascq]=%x,%x,%x,%x\n" , - sshdr.response_code, - sshdr.sense_key, sshdr.asc, - sshdr.ascq)); - } -#endif +SCSI_LOG_HLCOMPLETE (1 ,scmd_printk (KERN_INFO , SCpnt, +"sd_done: completed %d bytes\n" , + good_bytes));
Printing good_bytes here is misleading, because it is subsequently changed from 0 to nonzero in these cases:
* HARDWARE ERROR sense key
* MEDIUM ERROR sense key
* RECOVERED ERROR sense key
* ABORTED COMMAND sense key for ASC 0x10
* ILLEGAL REQUEST sense key for ASC 0x10
* ILLEGAL REQUEST sense key for ASC/ASCQ 0x20/0x24 for WRITE SAME without the UNMAP bit
Suggestions:
1. Move the print down after the out: label, when good_bytes has reached its final value.
2. Indicate if the command was able to transfer all the bytes by adding:
"%d of %d bytes" good_bytes, scsi_bufflen(SCpnt)
drivers/scsi/sd.c sd_spinup_disk(struct scsi_disk *sdkp) · line 1816
Quoted 11 lines
drivers/scsi/sd.c +2
-2sdkp->medium_access_timed_out =0 ; if (driver_byte(result) != DRIVER_SENSE &&@@ -1820,12 +1812,12 @@ sd_spinup_disk(struct scsi_disk *sdkp)/* no sense, TUR either succeeded or failed * with a status error */ if (!spintime && !scsi_status_is_good (the_result)) { -sd_printk (KERN_NOTICE , sdkp,"Unit Not Ready\n" ); -sd_print_result (sdkp, the_result); +sd_print_result (sdkp,"Unit Not Ready" , + the_result);
"unit" by itself is not a real SCSI term. "Logical Unit Not Ready" would be better, but this is printed for all non-GOOD status values, not just CHECK CONDITION status with a sense key of NOT READY and one of many additional sense codes with "LOGICAL UNIT NOT READY" in their names.
Following the other prints in this patch, how about this?
"TEST UNIT READY failed"
Quoted 9 lines
drivers/scsi/sd.c +1
-1}break ; } - +/* * The device does not want the automatic start to be issued. */
drivers/scsi/sd.c static void read_capacity_error(struct scsi_disk *sdkp, · line 1937
Quoted 6 lines
drivers/scsi/sd.c +1
-1@@ -1941,7 +1933,7 @@ static void read_capacity_error(struct scsi_disk *sdkp,struct scsi_device *sdp, struct scsi_sense_hdr *sshdr, int sense_valid, int the_result) { -sd_print_result (sdkp ,the_result ); + sd_print_result(sdkp, "READ CAPACITY failed", the_result);
The cdb_byte0_names[] array in constants.c capitalizes the candidates as:
Read Capacity(10)
Read capacity(16)
The two callers to this function already print the opcode name, using all caps and not printing "(10)", at the KERN_NOTICE level:
sd_printk(KERN_NOTICE, sdkp, "READ CAPACITY(16) failed\n");
read_capacity_error(sdkp, sdp, &sshdr, sense_valid, the_result);
sd_printk(KERN_NOTICE, sdkp, "READ CAPACITY failed\n");
read_capacity_error(sdkp, sdp, &sshdr, sense_valid, the_result);sd_print_result uses the KERN_INFO level.
To follow cdb_byte0_names, use "Read Capacity(10)" and "Read capacity(16)" in the callers.
To follow SCSI standards, add "(10)" in the second caller.
Consider changing cdb_byte0_names to "Read Capacity(16)" too.
drivers/scsi/sd.c static int sd_start_stop_device(struct scsi_disk *sdkp, · line 3122
Quoted 11 lines
drivers/scsi/sd.c +0
-0if (driver_byte (the_result) &DRIVER_SENSE )sd_print_sense_hdr (sdkp, sshdr);drivers/scsi/sd.c +1
-2else @@ -3126,8 +3118,7 @@ static int sd_start_stop_device(struct scsi_disk *sdkp, int start) res =scsi_execute_req_flags (sdp, cmd,DMA_NONE ,NULL ,0 , &sshdr,SD_TIMEOUT ,SD_MAX_RETRIES ,NULL ,REQ_PM );if (res) { -sd_printk (KERN_WARNING , sdkp,"START_STOP FAILED\n" ); -sd_print_result (sdkp, res); +sd_print_result (sdkp,"START_STOP failed" , res);
To follow cdb_byte0_names, use "Start/Stop Unit".
To follow SCSI standards, use "START STOP UNIT".
Since this command can request start, stop, or various power conditions, it might be helpful to include:
start=%d pwr_cond=0x%x", cmd[4] & 0x1, cmd[4] >> 4 & 0xf
drivers/scsi/sd.c static void sd_print_sense_hdr(struct scsi_disk *sdkp, · line 3323
Quoted 10 lines
drivers/scsi/sd.c +0
-0if (driver_byte (res) &DRIVER_SENSE )sd_print_sense_hdr (sdkp, &sshdr);drivers/scsi/sd.c +2
-1if (scsi_sense_valid(&sshdr) &&@@ -3328,9 +3319,19 @@ static void sd_print_sense_hdr(struct scsi_disk *sdkp, sshdr->asc, sshdr->ascq); } -static void sd_print_result (struct scsi_disk *sdkp,int result) +static void sd_print_result (struct scsi_disk *sdkp,const char *msg, +int result)
...
Since what sdkp points to is not modified, the first argument could be:
const struct scsi_disk *sdkp
---
Rob Elliott HP Server Storage
RE: [PATCH 20/24] sd: Cleanup logging
In reply to Hannes ReineckeQuoted 4 lines
-----Original Message-----
From: Hannes Reinecke [mailto:[email protected]]
...
Quoted 1 line
drivers/scsi/sd.c +0
-0
...
Quoted 11 lines
drivers/scsi/sd.c +3
-3@@ -3328,9 +3319,19 @@ static void sd_print_sense_hdr(struct scsi_disk *sdkp, sshdr->asc, sshdr->ascq); } -static void sd_print_result (struct scsi_disk *sdkp,int result) +static void sd_print_result (struct scsi_disk *sdkp,const char *msg, +int result) { -sd_printk (KERN_INFO , sdkp," " ); -scsi_show_result (result); +const char *hb_string =scsi_hostbyte_string (result);
drivers/scsi/sd.c static void sd_print_sense_hdr(struct scsi_disk *sdkp, · line 3335
Quoted 10 lines
drivers/scsi/sd.c +10
-0+const char *db_string =scsi_driverbyte_string (result); + +if (hb_string && db_string) +sd_printk (KERN_INFO , sdkp, +"%s: Result: hostbyte=%s driverbyte=%s\n" , + msg, hb_string, db_string); +else +sd_printk (KERN_INFO , sdkp, +"%s: Result: hostbyte=0x%02x driverbyte=0x%02x\n" , + msg,host_byte (result),driver_byte (result));
In a similar construct, patch 19 used || in scsi_print_result to choose the string path, and printed "invalid" if one of the strings was NULL. They should probably match.
---
Rob Elliott HP Server Storage
PATCH v5 21/24 scsi: simplify scsi_log_(send|completion)
In reply to Hannes ReineckeSimplify scsi_log_(send|completion) by externalizing scsi_mlreturn_string() and always print the command address.
Reviewed-by Christoph Hellwig <[email protected]>
Signed-off-by Hannes Reinecke <[email protected]>
--- drivers/scsi/constants.c | 39 ++++++++++++++++++++++++++++++++++++--- drivers/scsi/scsi.c | 43 ++++++------------------------------------- drivers/scsi/scsi_lib.c | 13 ++++++++++--- include/scsi/scsi_dbg.h | 3 ++- 4 files changed, 54 insertions(+), 44 deletions(-)
drivers/scsi/constants.c +36 -3
@@ -1437,19 +1437,52 @@ const char *scsi_driverbyte_string(int result) }EXPORT_SYMBOL (scsi_driverbyte_string); -void scsi_print_result (struct scsi_cmnd *cmd) +#ifdef CONFIG_SCSI_CONSTANTS +#define scsi_mlreturn_name (result) { result, #result } +static const struct value_name_pair scsi_mlreturn_arr[] = { +scsi_mlreturn_name (NEEDS_RETRY ), +scsi_mlreturn_name (SUCCESS ), +scsi_mlreturn_name (FAILED ), +scsi_mlreturn_name (QUEUED ), +scsi_mlreturn_name (SOFT_ERROR ), +scsi_mlreturn_name (ADD_TO_MLQUEUE ), +scsi_mlreturn_name (TIMEOUT_ERROR ), +scsi_mlreturn_name (SCSI_RETURN_NOT_HANDLED ), +scsi_mlreturn_name (FAST_IO_FAIL ) +}; +#endif + +const char *scsi_mlreturn_string (int result) +{ +#ifdef CONFIG_SCSI_CONSTANTS +const struct value_name_pair *arr = scsi_mlreturn_arr; +int k; + +for (k =0 ; k <ARRAY_SIZE (scsi_mlreturn_arr); ++k, ++arr) { +if (result == arr->value) +return arr->name; + } +#endif +return NULL ; +} +EXPORT_SYMBOL (scsi_mlreturn_string); + +void scsi_print_result (struct scsi_cmnd *cmd,const char *msg,int disposition) { +const char *mlret_string =scsi_mlreturn_string (disposition);const char *hb_string =scsi_hostbyte_string (cmd->result);const char *db_string =scsi_driverbyte_string (cmd->result);if (hb_string || db_string)scmd_printk (KERN_INFO , cmd, -"Result: hostbyte=%s driverbyte=%s" , +"%s%s Result: hostbyte=%s driverbyte=%s" , + msg, mlret_string ? mlret_string :"UNKNOWN" , hb_string ? hb_string :"invalid" , db_string ? db_string :"invalid" );else scmd_printk (KERN_INFO , cmd, -"Result: hostbyte=0x%02x driverbyte=0x%02x" , +"%s%s Result: hostbyte=0x%02x driverbyte=0x%02x" , + msg, mlret_string ? mlret_string :"UNKNOWN" ,host_byte (cmd->result),driver_byte (cmd->result)); }EXPORT_SYMBOL (scsi_print_result);
drivers/scsi/scsi.c +6 -37
68 lines not on this page · raw message
drivers/scsi/scsi_lib.c +10 -3
@@ -832,7 +832,7 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)int error =0 ;struct scsi_sense_hdr sshdr;bool sense_valid = false; -int sense_deferred =0 ; +int sense_deferred =0 , level =0 ;enum {ACTION_FAIL ,ACTION_REPREP ,ACTION_RETRY ,ACTION_DELAYED_RETRY } action;unsigned long wait_for = (cmd->allowed +1 ) * req->timeout;
@@ -1038,8 +1038,15 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)switch (action) {case ACTION_FAIL :/* Give up and fail the remainder of the request */ -if (!(req->cmd_flags &REQ_QUIET )) { -scsi_print_result (cmd); +if (unlikely (scsi_logging_level)) + level =SCSI_LOG_LEVEL (SCSI_LOG_MLQUEUE_SHIFT , +SCSI_LOG_MLQUEUE_BITS ); +/* +* if logging is enabled the failure will be printed +* in scsi_log_completion(), so avoid duplicate messages +*/ +if (!level && !(req->cmd_flags &REQ_QUIET )) { +scsi_print_result (cmd,NULL ,FAILED );if (driver_byte (result) &DRIVER_SENSE )scsi_print_sense (cmd);scsi_print_command (cmd);
include/scsi/scsi_dbg.h +2 -1
@@ -17,9 +17,10 @@ extern void scsi_print_sense(struct scsi_cmnd *);extern void __scsi_print_sense (struct scsi_device *,const char *name,const unsigned char *sense_buffer,int sense_len); -extern void scsi_print_result (struct scsi_cmnd *); +extern void scsi_print_result (struct scsi_cmnd *,const char *,int );extern const char *scsi_hostbyte_string (int );extern const char *scsi_driverbyte_string (int ); +extern const char *scsi_mlreturn_string (int );extern const char *scsi_sense_key_string (unsigned char );extern const char *scsi_extd_sense_format (unsigned char ,unsigned char ,const char **);
Signature · 1 line
1.8.5.2
RE: [PATCH 21/24] scsi: simplify scsi_log_(send|completion)
Quoted 34 lines
-----Original Message-----
From: Hannes Reinecke [mailto:[email protected]]
Sent: Wednesday, 01 October, 2014 1:23 AM
To: James Bottomley
Cc: Christoph Hellwig; [email protected]; Elliott, Robert (Server Storage); Hannes Reinecke
Subject: [PATCH 21/24] scsi: simplifyscsi_log_(send|completion)Simplify
scsi_log_(send|completion)by externalizingscsi_mlreturn_string()and always print the command address.Reviewed-by Christoph Hellwig <[email protected]>Signed-off-by Hannes Reinecke <[email protected]>--- drivers/scsi/constants.c | 39 ++++++++++++++++++++++++++++++++++++--- drivers/scsi/scsi.c | 43 ++++++------------------------------------- drivers/scsi/scsi_lib.c | 13 ++++++++++--- include/scsi/scsi_dbg.h | 3 ++- 4 files changed, 54 insertions(+), 44 deletions(-)drivers/scsi/constants.c +3
-1@@ -1437,19 +1437,52 @@ const char *scsi_driverbyte_string(int result) }EXPORT_SYMBOL (scsi_driverbyte_string); -void scsi_print_result (struct scsi_cmnd *cmd) +#ifdef CONFIG_SCSI_CONSTANTS +#define scsi_mlreturn_name (result) { result, #result } +static const struct value_name_pair scsi_mlreturn_arr[] = {
drivers/scsi/constants.c const char *scsi_driverbyte_string(int result) · line 1452
Quoted 10 lines
drivers/scsi/constants.c +10
-0+scsi_mlreturn_name (NEEDS_RETRY ), +scsi_mlreturn_name (SUCCESS ), +scsi_mlreturn_name (FAILED ), +scsi_mlreturn_name (QUEUED ), +scsi_mlreturn_name (SOFT_ERROR ), +scsi_mlreturn_name (ADD_TO_MLQUEUE ), +scsi_mlreturn_name (TIMEOUT_ERROR ), +scsi_mlreturn_name (SCSI_RETURN_NOT_HANDLED ), +scsi_mlreturn_name (FAST_IO_FAIL ) +};
SUCCESS is a misleading name to print on commands that were really not successful. Example:
[ 5978.573297] sd 2:0:0:8: [sdz] tag#209 Done: SUCCESS Result: hostbyte=DID_OK driverbyte=DRIVER_OK [ 5978.576335] sd 2:0:0:8: [sdz] tag#209 CDB: Test Unit Ready 00 00 00 00 00 00 [ 5978.578721] sd 2:0:0:8: [sdz] tag#209 Sense Key : Unit Attention [current] [ 5978.581177] sd 2:0:0:8: [sdz] tag#209 Add. Sense: Bus device reset function occurred
Maybe that mlreturn value should be printed as "COMPLETE" since it is similar to the SAM service responses called COMMAND COMPLETE and FUNCTION COMPLETE. That is a more neutral term.
---
Rob Elliott HP Server Storage
Re: [PATCH 21/24] scsi: simplify scsi_log_(send|completion)
Quoted 52 lines · Robert (Server Storage)
Quoted 34 lines
-----Original Message-----
From: Hannes Reinecke [mailto:[email protected]]
Sent: Wednesday, 01 October, 2014 1:23 AM
To: James Bottomley
Cc: Christoph Hellwig; [email protected]; Elliott, Robert (Server Storage); Hannes Reinecke
Subject: [PATCH 21/24] scsi: simplifyscsi_log_(send|completion)Simplify
scsi_log_(send|completion)by externalizingscsi_mlreturn_string()and always print the command address.Reviewed-by Christoph Hellwig <[email protected]>Signed-off-by Hannes Reinecke <[email protected]>--- drivers/scsi/constants.c | 39 ++++++++++++++++++++++++++++++++++++--- drivers/scsi/scsi.c | 43 ++++++------------------------------------- drivers/scsi/scsi_lib.c | 13 ++++++++++--- include/scsi/scsi_dbg.h | 3 ++- 4 files changed, 54 insertions(+), 44 deletions(-)drivers/scsi/constants.c +3
-1@@ -1437,19 +1437,52 @@ const char *scsi_driverbyte_string(int result) }EXPORT_SYMBOL (scsi_driverbyte_string); -void scsi_print_result (struct scsi_cmnd *cmd) +#ifdef CONFIG_SCSI_CONSTANTS +#define scsi_mlreturn_name (result) { result, #result } +static const struct value_name_pair scsi_mlreturn_arr[] = {drivers/scsi/constants.c const char *scsi_driverbyte_string(int result) · line 1452
Quoted 10 lines
drivers/scsi/constants.c +10
-0+scsi_mlreturn_name (NEEDS_RETRY ), +scsi_mlreturn_name (SUCCESS ), +scsi_mlreturn_name (FAILED ), +scsi_mlreturn_name (QUEUED ), +scsi_mlreturn_name (SOFT_ERROR ), +scsi_mlreturn_name (ADD_TO_MLQUEUE ), +scsi_mlreturn_name (TIMEOUT_ERROR ), +scsi_mlreturn_name (SCSI_RETURN_NOT_HANDLED ), +scsi_mlreturn_name (FAST_IO_FAIL ) +};SUCCESS is a misleading name to print on commands that were really not successful. Example:
[ 5978.573297] sd 2:0:0:8: [sdz] tag#209 Done: SUCCESS Result: hostbyte=DID_OK driverbyte=DRIVER_OK [ 5978.576335] sd 2:0:0:8: [sdz] tag#209 CDB: Test Unit Ready 00 00 00 00 00 00 [ 5978.578721] sd 2:0:0:8: [sdz] tag#209 Sense Key : Unit Attention [current] [ 5978.581177] sd 2:0:0:8: [sdz] tag#209 Add. Sense: Bus device reset function occurred
Quoted 5 lines
Maybe that mlreturn value should be printed as "COMPLETE" since it is similar to the SAM service responses called COMMAND COMPLETE and FUNCTION COMPLETE. That is a more neutral term.
I do agree that SUCCESS is rather misleading here. However, the midlayer always used 'SUCCESS' here, and changing that would be a different patch as it's bound to induce quite some discussion.
I'm happy to send such a patch, but preferably _after_ this patchset is in.
Cheers,
Hannes
Signature · 8 lines
Dr. Hannes Reinecke zSeries & Storage [email protected] +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 [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH v5 22/24 scsi: fixup logging messages in scsi_error.c
In reply to Hannes ReineckeUse the matching scope for logging messages to allow for better command tracing.
Suggested-by Robert Elliott <[email protected]>
Reviewed-by Christoph Hellwig <[email protected]>
Signed-off-by Hannes Reinecke <[email protected]>
--- drivers/scsi/hosts.c | 4 +-- drivers/scsi/scsi_error.c | 82 +++++++++++++++++++++++++---------------------- 2 files changed, 45 insertions(+), 41 deletions(-)
drivers/scsi/hosts.c +2 -2
@@ -485,8 +485,8 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *sht, int privsize)WQ_UNBOUND |WQ_MEM_RECLAIM , 1, shost->host_no);if (!shost->tmf_work_q) { -printk (KERN_WARNING "scsi%d: failed to create tmf workq\n" , - shost->host_no); +shost_printk (KERN_WARNING , shost, +"failed to create tmf workq\n" ); goto fail_kthread; }scsi_proc_hostdir_add (shost->hostt);
drivers/scsi/scsi_error.c +43 -39
161 lines not on this page · raw message
Signature · 1 line
1.8.5.2
Re: [PATCH 22/24] scsi: fixup logging messages in scsi_error.c
Quoted 10 lines · Hannes Reinecke
Use the matching scope for logging messages to allow for better command tracing.
Suggested-by Robert Elliott <[email protected]>Reviewed-by Christoph Hellwig <[email protected]>Signed-off-by Hannes Reinecke <[email protected]>--- drivers/scsi/hosts.c | 4 +-- drivers/scsi/scsi_error.c | 82 +++++++++++++++++++++++++---------------------- 2 files changed, 45 insertions(+), 41 deletions(-)
See below. I think there is another line that needs to be changed.
Reviewed-by Ewan D. Milne <[email protected]>
Quoted 57 lines
drivers/scsi/hosts.c +2
-2@@ -485,8 +485,8 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *sht, int privsize)WQ_UNBOUND |WQ_MEM_RECLAIM , 1, shost->host_no);if (!shost->tmf_work_q) { -printk (KERN_WARNING "scsi%d: failed to create tmf workq\n" , - shost->host_no); +shost_printk (KERN_WARNING , shost, +"failed to create tmf workq\n" ); goto fail_kthread; }scsi_proc_hostdir_add (shost->hostt);drivers/scsi/scsi_error.c +10
-9@@ -1156,9 +1156,10 @@ int scsi_eh_get_sense(struct list_head *work_q, shost = scmd->device->host;if (scsi_host_eh_past_deadline (shost)) {SCSI_LOG_ERROR_RECOVERY (3 , -shost_printk (KERN_INFO , shost, -"skip %s, past eh deadline\n" , - __func__)); +scmd_printk (KERN_INFO , scmd, +"%s: skip request sense, " +"past eh deadline\n" , + current->comm));break ; }if (status_byte (scmd->result) !=CHECK_CONDITION )@@ -1265,9 +1266,10 @@ static int scsi_eh_test_devices(struct list_head *cmd_list,/* Push items back onto work_q */ list_splice_init (cmd_list, work_q);SCSI_LOG_ERROR_RECOVERY (3 , -shost_printk (KERN_INFO , sdev->host, -"skip %s, past eh deadline" , - __func__)); +sdev_printk (KERN_INFO , sdev, +"%s: skip test device, " +"past eh deadline" , + current->comm));break ; } }@@ -1318,21 +1320,22 @@ static int scsi_eh_abort_cmds(struct list_head *work_q,if (scsi_host_eh_past_deadline (shost)) {list_splice_init (&check_list, work_q); SCSI_LOG_ERROR_RECOVERY(3, -shost_printk (KERN_INFO , shost, -"skip %s, past eh deadline\n" , - __func__)); + scmd_printk(KERN_INFO, scmd, + "%s: skip aborting cmd, "
drivers/scsi/scsi_error.c static int scsi_eh_abort_cmds(struct list_head *work_q, · line 1332
Quoted 10 lines
drivers/scsi/scsi_error.c +4
-2+"past eh deadline\n" , + current->comm));return list_empty (work_q); }SCSI_LOG_ERROR_RECOVERY (3 , -shost_printk (KERN_INFO , shost, -"%s: aborting cmd: 0x%p\n" , +scmd_printk (KERN_INFO , scmd, +"%s: aborting cmd\n" , current->comm, scmd));
I got a compile error on this, I think you need to change the above line
to be " current->comm));", removing scmd
Quoted 126 lines
drivers/scsi/scsi_error.c +29
-28rtn =scsi_try_to_abort_cmd (shost->hostt, scmd);if (rtn ==FAILED ) {SCSI_LOG_ERROR_RECOVERY (3 , -shost_printk (KERN_INFO , shost, -"%s: aborting cmd failed: 0x%p\n" , - current->comm, scmd)); +scmd_printk (KERN_INFO , scmd, +"%s: aborting cmd failed\n" , + current->comm));list_splice_init (&check_list, work_q);return list_empty (work_q); }@@ -1390,9 +1393,10 @@ static int scsi_eh_stu(struct Scsi_Host *shost,shost_for_each_device (sdev, shost) {if (scsi_host_eh_past_deadline (shost)) {SCSI_LOG_ERROR_RECOVERY (3 , -shost_printk (KERN_INFO , shost, -"skip %s, past eh deadline\n" , - __func__)); +sdev_printk (KERN_INFO , sdev, +"%s: skip START_UNIT, " +"past eh deadline\n" , + current->comm));break ; } stu_scmd =NULL ;@@ -1407,9 +1411,9 @@ static int scsi_eh_stu(struct Scsi_Host *shost,continue ;SCSI_LOG_ERROR_RECOVERY (3 , -shost_printk (KERN_INFO , shost, -"%s: Sending START_UNIT to sdev: 0x%p\n" , - current->comm, sdev)); +sdev_printk (KERN_INFO , sdev, +"%s: Sending START_UNIT\n" , + current->comm));if (!scsi_eh_try_stu (stu_scmd)) { if (!scsi_device_online(sdev) ||@@ -1423,9 +1427,9 @@ static int scsi_eh_stu(struct Scsi_Host *shost, } }else {SCSI_LOG_ERROR_RECOVERY (3 , -shost_printk (KERN_INFO , shost, -"%s: START_UNIT failed to sdev:" -" 0x%p\n" , current->comm, sdev)); +sdev_printk (KERN_INFO , sdev, +"%s: START_UNIT failed\n" , + current->comm)); } }@@ -1456,9 +1460,9 @@ static int scsi_eh_bus_device_reset(struct Scsi_Host *shost,shost_for_each_device (sdev, shost) {if (scsi_host_eh_past_deadline (shost)) {SCSI_LOG_ERROR_RECOVERY (3 , -shost_printk (KERN_INFO , shost, -"skip %s, past eh deadline\n" , - __func__)); +sdev_printk (KERN_INFO , sdev, +"%s: skip BDR, past eh deadline\n" , + current->comm));break ; } bdr_scmd =NULL ;@@ -1472,9 +1476,8 @@ static int scsi_eh_bus_device_reset(struct Scsi_Host *shost,continue ;SCSI_LOG_ERROR_RECOVERY (3 , -shost_printk (KERN_INFO , shost, -"%s: Sending BDR sdev: 0x%p\n" , - current->comm, sdev)); +sdev_printk (KERN_INFO , sdev, +"%s: Sending BDR\n" , current->comm)); rtn =scsi_try_bus_device_reset (bdr_scmd);if (rtn ==SUCCESS || rtn ==FAST_IO_FAIL ) { if (!scsi_device_online(sdev) ||@@ -1490,9 +1493,8 @@ static int scsi_eh_bus_device_reset(struct Scsi_Host *shost, } }else {SCSI_LOG_ERROR_RECOVERY (3 , -shost_printk (KERN_INFO , shost, -"%s: BDR failed sdev: 0x%p\n" , - current->comm, sdev)); +sdev_printk (KERN_INFO , sdev, +"%s: BDR failed\n" , current->comm)); } }@@ -1528,8 +1530,9 @@ static int scsi_eh_target_reset(struct Scsi_Host *shost,list_splice_init (&tmp_list, work_q);SCSI_LOG_ERROR_RECOVERY (3 ,shost_printk (KERN_INFO , shost, -"skip %s, past eh deadline\n" , - __func__)); +"%s: Skip target reset, " +"past eh deadline\n" , + current->comm));return list_empty (work_q); }@@ -1591,8 +1594,8 @@ static int scsi_eh_bus_reset(struct Scsi_Host *shost,list_splice_init (&check_list, work_q);SCSI_LOG_ERROR_RECOVERY (3 ,shost_printk (KERN_INFO , shost, -"skip %s, past eh deadline\n" , - __func__)); +"%s: skip BRST, past eh deadline\n" , + current->comm));return list_empty (work_q); }@@ -2191,9 +2194,10 @@ int scsi_error_handler(void *data)*/ if (!shost->eh_noresume &&scsi_autopm_get_host (shost) !=0 ) {SCSI_LOG_ERROR_RECOVERY (1 , -printk (KERN_ERR "Error handler scsi_eh_%d " -"unable to autoresume\n" , - shost->host_no)); +shost_printk (KERN_ERR , shost, +"scsi_eh_%d: " +"unable to autoresume\n" , + shost->host_no));continue ; }
RE: [PATCH 22/24] scsi: fixup logging messages in scsi_error.c
In reply to Hannes ReineckeQuoted 4 lines
-----Original Message-----
From: Hannes Reinecke [mailto:[email protected]]
...
Quoted 5 lines
drivers/scsi/scsi_error.c +0
-0
drivers/scsi/scsi_error.c int scsi_eh_get_sense(struct list_head *work_q, · line 1161
Quoted 9 lines
drivers/scsi/scsi_error.c +3
-3@@ -1156,9 +1156,10 @@ int scsi_eh_get_sense(struct list_head *work_q, shost = scmd->device->host;if (scsi_host_eh_past_deadline (shost)) { SCSI_LOG_ERROR_RECOVERY(3, -shost_printk (KERN_INFO , shost, -"skip %s, past eh deadline\n" , - __func__)); + scmd_printk(KERN_INFO, scmd, + "%s: skip request sense, " + "past eh deadline\n",
checkpatch lets you keep strings on one line.
Quoted 5 lines
drivers/scsi/scsi_error.c +1
-0+ current->comm));break ; }if (status_byte (scmd->result) !=CHECK_CONDITION )
drivers/scsi/scsi_error.c static int scsi_eh_test_devices(struct list_head · line 1272
Quoted 10 lines
drivers/scsi/scsi_error.c +3
-3@@ -1265,9 +1266,10 @@ static int scsi_eh_test_devices(struct list_head *cmd_list,/* Push items back onto work_q */ list_splice_init (cmd_list, work_q); SCSI_LOG_ERROR_RECOVERY(3, -shost_printk (KERN_INFO , sdev->host, -"skip %s, past eh deadline" , - __func__)); + sdev_printk(KERN_INFO, sdev, + "%s: skip test device, " + "past eh deadline",
checkpatch lets you keep strings on one line.
Quoted 5 lines
drivers/scsi/scsi_error.c +1
-0+ current->comm));break ; }drivers/scsi/scsi_error.c +0
-0}
drivers/scsi/scsi_error.c static int scsi_eh_abort_cmds(struct list_head · line 1326
Quoted 10 lines
drivers/scsi/scsi_error.c +3
-3@@ -1318,21 +1320,22 @@ static int scsi_eh_abort_cmds(struct list_head *work_q, if (scsi_host_eh_past_deadline(shost)) {list_splice_init (&check_list, work_q); SCSI_LOG_ERROR_RECOVERY(3, -shost_printk (KERN_INFO , shost, -"skip %s, past eh deadline\n" , - __func__)); + scmd_printk(KERN_INFO, scmd, + "%s: skip aborting cmd, " + "past eh deadline\n",
checkpatch lets you keep strings on one line.
drivers/scsi/scsi_error.c static int scsi_eh_abort_cmds(struct list_head *work_q, · line 1332
Quoted 9 lines
drivers/scsi/scsi_error.c +3
-2+ current->comm));return list_empty (work_q); }SCSI_LOG_ERROR_RECOVERY (3 , -shost_printk (KERN_INFO , shost, -"%s: aborting cmd: 0x%p\n" , +scmd_printk (KERN_INFO , scmd, +"%s: aborting cmd\n" , current->comm, scmd));
As noted by Ewan, the extra scmd argument causes compiler warnings.
...
drivers/scsi/scsi_error.c static int scsi_eh_stu(struct Scsi_Host *shost, · line 1398
Quoted 10 lines
drivers/scsi/scsi_error.c +3
-3@@ -1390,9 +1393,10 @@ static int scsi_eh_stu(struct Scsi_Host *shost,shost_for_each_device (sdev, shost) {if (scsi_host_eh_past_deadline (shost)) { SCSI_LOG_ERROR_RECOVERY(3, -shost_printk (KERN_INFO , shost, -"skip %s, past eh deadline\n" , - __func__)); + sdev_printk(KERN_INFO, sdev, + "%s: skip START_UNIT, " + "past eh deadline\n",
checkpatch lets you keep strings on one line.
Quoted 4 lines
drivers/scsi/scsi_error.c +1
-0+ current->comm));break ; } stu_scmd =NULL ;
...
drivers/scsi/scsi_error.c static int scsi_eh_target_reset(struct Scsi_Host · line 1535
Quoted 9 lines
drivers/scsi/scsi_error.c +2
-2@@ -1528,8 +1530,9 @@ static int scsi_eh_target_reset(struct Scsi_Host *shost,list_splice_init (&tmp_list, work_q); SCSI_LOG_ERROR_RECOVERY(3, shost_printk(KERN_INFO, shost, -"skip %s, past eh deadline\n" , - __func__)); + "%s: Skip target reset, " + "past eh deadline\n",
checkpatch lets you keep strings on one line.
...
drivers/scsi/scsi_error.c int scsi_error_handler(void *data) · line 2199
Quoted 10 lines
drivers/scsi/scsi_error.c +3
-3@@ -2191,9 +2194,10 @@ int scsi_error_handler(void *data)*/ if (!shost->eh_noresume &&scsi_autopm_get_host (shost) !=0 ) { SCSI_LOG_ERROR_RECOVERY(1, -printk (KERN_ERR "Error handler scsi_eh_%d " -"unable to autoresume\n" , - shost->host_no)); + shost_printk(KERN_ERR, shost, + "scsi_eh_%d: " + "unable to autoresume\n",
checkpatch lets you keep strings on one line.
---
Rob Elliott HP Server Storage
PATCH v5 23/24 scsi: use shost argument in scsi_eh_prt_fail_stats
In reply to Hannes ReineckeThe EH statistics are per host, so we should be using shost_printk() here.
Suggested-by Robert Elliott <[email protected]>
Reviewed-by Christoph Hellwig <[email protected]>
Signed-off-by Hannes Reinecke <[email protected]>
--- drivers/scsi/scsi_error.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/scsi/scsi_error.c +1 -1
@@ -355,7 +355,7 @@ static inline void scsi_eh_prt_fail_stats(struct Scsi_Host *shost,if (cmd_cancel || cmd_failed) {SCSI_LOG_ERROR_RECOVERY (3 , -sdev_printk (KERN_INFO , sdev, +shost_printk (KERN_INFO , shost,"%s: cmds failed: %d, cancel: %d\n" , __func__, cmd_failed, cmd_cancel));
Signature · 1 line
1.8.5.2
PATCH v5 24/24 scsi_error: document scsi_try_to_abort_cmd
In reply to Hannes Reineckescsi_try_to_abort_cmd() should only return SUCCESS or FAILED. So document that in the function description and simplify the logging message.
Suggested-by Christoph Hellwig <[email protected]>
Cc Robert Elliott <[email protected]>
Signed-off-by Hannes Reinecke <[email protected]>
--- drivers/scsi/scsi_error.c | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-)
drivers/scsi/scsi_error.c +17 -3
@@ -157,8 +157,7 @@ scmd_eh_abort_handler(struct work_struct *work) }else {SCSI_LOG_ERROR_RECOVERY (3 ,scmd_printk (KERN_INFO , scmd, -"scmd %p abort failed, rtn %d\n" , - scmd, rtn)); +"scmd %p abort failed\n" , scmd)); } }
@@ -869,7 +868,22 @@ static int scsi_try_bus_device_reset(struct scsi_cmnd *scmd)return rtn; } -static int scsi_try_to_abort_cmd (struct scsi_host_template *hostt,struct scsi_cmnd *scmd) +/** +* scsi_try_to_abort_cmd - Ask host to abort a SCSI command +* @scmd: SCSI cmd used to send a target reset +* +* Return value: +* SUCCESS or FAILED +* +* Notes: +* SUCCESS does not necessarily indicate that the command +* has been aborted; it only indicates that the LLDDs +* has cleared all references to that command. +* LLDDs should return FAILED only if an abort was required +* but could not be executed. +*/ +static int scsi_try_to_abort_cmd (struct scsi_host_template *hostt, +struct scsi_cmnd *scmd) {if (!hostt->eh_abort_handler)return FAILED ;
Signature · 1 line
1.8.5.2
Re: [PATCH 24/24] scsi_error: document scsi_try_to_abort_cmd
Quoted 8 lines · Hannes Reinecke
scsi_try_to_abort_cmd()should only return SUCCESS or FAILED. So document that in the function description and simplify the logging message.Suggested-by Christoph Hellwig <[email protected]>Cc Robert Elliott <[email protected]>Signed-off-by Hannes Reinecke <[email protected]>
Looks good,
Reviewed-by Christoph Hellwig <[email protected]>
RE: [PATCH 24/24] scsi_error: document scsi_try_to_abort_cmd
In reply to Hannes ReineckeQuoted 60 lines
-----Original Message-----
From: Hannes Reinecke [mailto:[email protected]]
Sent: Wednesday, 01 October, 2014 1:23 AM
To: James Bottomley
Cc: Christoph Hellwig; [email protected]; Elliott, Robert (Server Storage); Hannes Reinecke
Subject: [PATCH 24/24]scsi_error: documentscsi_try_to_abort_cmd
scsi_try_to_abort_cmd()should only return SUCCESS or FAILED. So document that in the function description and simplify the logging message.Suggested-by Christoph Hellwig <[email protected]>Cc Robert Elliott <[email protected]>Signed-off-by Hannes Reinecke <[email protected]>--- drivers/scsi/scsi_error.c | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-)drivers/scsi/scsi_error.c +17
-3@@ -157,8 +157,7 @@ scmd_eh_abort_handler(struct work_struct *work) }else {SCSI_LOG_ERROR_RECOVERY (3 ,scmd_printk (KERN_INFO , scmd, -"scmd %p abort failed, rtn %d\n" , - scmd, rtn)); +"scmd %p abort failed\n" , scmd)); } }@@ -869,7 +868,22 @@ static int scsi_try_bus_device_reset(struct scsi_cmnd *scmd)return rtn; } -static int scsi_try_to_abort_cmd (struct scsi_host_template *hostt,struct scsi_cmnd *scmd) +/** +* scsi_try_to_abort_cmd - Ask host to abort a SCSI command +* @scmd: SCSI cmd used to send a target reset +* +* Return value: +* SUCCESS or FAILED +* +* Notes: +* SUCCESS does not necessarily indicate that the command +* has been aborted; it only indicates that the LLDDs +* has cleared all references to that command. +* LLDDs should return FAILED only if an abort was required +* but could not be executed. +*/ +static int scsi_try_to_abort_cmd (struct scsi_host_template *hostt, +struct scsi_cmnd *scmd) {
drivers/scsi/scsi_error.c static int scsi_try_bus_device_reset(struct scsi_cmnd *scmd) · line 889
Quoted 2 lines
drivers/scsi/scsi_error.c +0
-0if (!hostt->eh_abort_handler)return FAILED ;
Since the rest of that function is just:
return hostt->eh_abort_handler(scmd);
this relies on the LLD to use only those values:
#define SUCCESS 0x2002
#define FAILED 0x2003
which is hard to ensure.
Randomly picking on one, megaraid.c registers this function:
.eh_abort_handler = megaraid_abort,
megaraid_abort returns the return code from megaraid_abort_and_reset, which returns TRUE or FALSE, not SUCCESS and FAILED.
Printing the return value helps expose such problems (if the code path is exercised).
---
Rob Elliott HP Server Storage
Quoted 40 lines
-----Original Message-----
From: Hannes Reinecke [mailto:[email protected]]
Sent: Wednesday, 01 October, 2014 1:23 AM
To: James Bottomley
Cc: Christoph Hellwig; [email protected]; Elliott, Robert (Server Storage); Hannes Reinecke
Subject: [PATCHv5 00/24] scsi logging update (the boring part)This is the next iteration of my scsi logging patchset. The patchset just contains some logging updates, code reshuffling and sanity fixes. Nothing major.
The second part of this patchset (the printk bit) has been vetoed for the moment, so I'll have to work on that. But in any case this patchset is pretty much independent.
Difference to v4:
- Add 'Reviewed-by' tags from hch where applicable
- Split off the fas216 patch into two different patches
- Added a patch to documentscsi_try_to_abort_cmdHannes Reinecke (24):
Removescsi_cmd_print_sense_hdr()
sd: Removescsi_print_sense()insd_done()
aha152x: Debug output update and whitespace cleanup
scsi: introducesdev_prefix_printk()
scsi: Use sdev as argument for sense code printing
acornscsi: usescsi_print_command()
fas216: ReturnDID_ERRORfor incomplete data transfer
fas216: Update logging messages
53c700: removescsi_print_sense()usage
scsi: stop decoding ifscsi_normalize_sense()fails
scsi: do not decode sense extras
scsi: use 'bool' as return value forscsi_normalize_sense()
scsi: removescsi_print_status()
Implementscsi_opcode_sa_name
scsi: mergeprint_opcode_name()
scsi: consolidate opcode lookup inscsi_opcode_sa_name()
Quoted 8 lines
scsi: remove last argument from
print_opcode_name()
scsi: Removescsi_print_commandwhen calling abort
scsi: separate out scsi_(host|driver)byte_string()
sd: Cleanup logging
scsi: simplifyscsi_log_(send|completion)
scsi: fixup logging messages inscsi_error.c
scsi: use shost argument inscsi_eh_prt_fail_stats
scsi_error: documentscsi_try_to_abort_cmd
With this 24-part series applied to 3.17rc7, with the kernel set to print timestamps (e.g., printk.time=y on the kernel command line), the prints still end up trickling out a byte at a time, interleaved from different threads, when the system is overloaded.
Maybe that's addressed by the part that is deferred?
Example serial log excerpt:
[74465.705193] sd 2:0:0:0: [sdr] CDB:
[74465.705194] :
[74465.705196] 0
[74465.705196] sd 2:0:0:0: [sdr] (null)FAILED Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE
[74465.705197] 00
[74465.705198] :
[74465.705198] :
[74465.705199] 00
[74465.705200] Read(10)
[74465.705201] 28
[74465.705202] f4
[74465.705202] sd 2:0:0:0: [sdr] Sense Key : Hardware Error [current]
[74465.705204] b2
[74465.705204] 28
[74465.705205] 28
[74465.705206] 01
[74465.705207] :
[74465.705207] 00
[74465.705208] 70
[74465.705209] f8
[74465.705210] sd 2:0:0:0: [sdr] Add. Sense: Logical unit failure
[74465.705211] 00
[74465.705211] 00
[74465.705212] 2f
[74465.705213] 28
[74465.705214] 00
[74465.705215] 00
[74465.705215] 00
[74465.705217] 00
[74465.705217] sd 2:0:0:0: [sdr] CDB:
[74465.705218] 00
[74465.705219] 70
[74465.705219] 00
[74465.705220] 00
[74465.705221] 00
[74465.705221] 00
[74465.705222] 01
[74465.705223] Read(10)
[74465.705223] 01
[74465.705224] 00
[74465.705225] 00
[74465.705226] 9c
[74465.705227] 08
[74465.705227] 08
[74465.705228] 81
[74465.705229] :
[74465.705229] 9d
[74465.705230] 00
[74465.705231] 01
[74465.705232] 70
[74465.705232] 00
[74465.705233] 00
[74465.705234] 50
[74465.705234] 28
[74465.705235] e8
[74465.705236] 08
[74465.705237] 9f
[74465.705237] 00
[74465.705238]
[74465.705238]
[74465.705239] 00
[74465.705240] 00
[74465.705240] 00
[74465.705241] 00
[74465.705242] 68
[74465.705243] 00
[74465.705243] 00
[74465.705244] 00
[74465.705245] 00
[74465.705245]
[74465.705246] 00
[74465.705247] 08
[74465.705247] 08
[74465.705248] 01
[74465.705249] 08
[74465.705250] 00
[74465.705250] 00
[74465.705251] 00
[74465.705253] 33
[74465.705253] sd 2:0:0:0: [sdr] Done: SUCCESS Result: hostbyte=DID_TARGET_FAILURE driverbyte=DRIVER_OKQuoted 49 lines · Robert (Server Storage)
Quoted 40 lines
-----Original Message-----
From: Hannes Reinecke [mailto:[email protected]]
Sent: Wednesday, 01 October, 2014 1:23 AM
To: James Bottomley
Cc: Christoph Hellwig; [email protected]; Elliott, Robert (Server Storage); Hannes Reinecke
Subject: [PATCHv5 00/24] scsi logging update (the boring part)This is the next iteration of my scsi logging patchset. The patchset just contains some logging updates, code reshuffling and sanity fixes. Nothing major.
The second part of this patchset (the printk bit) has been vetoed for the moment, so I'll have to work on that. But in any case this patchset is pretty much independent.
Difference to v4:
- Add 'Reviewed-by' tags from hch where applicable
- Split off the fas216 patch into two different patches
- Added a patch to documentscsi_try_to_abort_cmdHannes Reinecke (24):
Removescsi_cmd_print_sense_hdr()
sd: Removescsi_print_sense()insd_done()
aha152x: Debug output update and whitespace cleanup
scsi: introducesdev_prefix_printk()
scsi: Use sdev as argument for sense code printing
acornscsi: usescsi_print_command()
fas216: ReturnDID_ERRORfor incomplete data transfer
fas216: Update logging messages
53c700: removescsi_print_sense()usage
scsi: stop decoding ifscsi_normalize_sense()fails
scsi: do not decode sense extras
scsi: use 'bool' as return value forscsi_normalize_sense()
scsi: removescsi_print_status()
Implementscsi_opcode_sa_name
scsi: mergeprint_opcode_name()
scsi: consolidate opcode lookup inscsi_opcode_sa_name()Quoted 8 lines
scsi: remove last argument from
print_opcode_name()
scsi: Removescsi_print_commandwhen calling abort
scsi: separate out scsi_(host|driver)byte_string()
sd: Cleanup logging
scsi: simplifyscsi_log_(send|completion)
scsi: fixup logging messages inscsi_error.c
scsi: use shost argument inscsi_eh_prt_fail_stats
scsi_error: documentscsi_try_to_abort_cmd
Quoted 8 lines
With this 24-part series applied to 3.17rc7, with the kernel set to print timestamps (e.g., printk.time=y on the kernel command line), the prints still end up trickling out a byte at a time, interleaved from different threads, when the system is overloaded.
Maybe that's addressed by the part that is deferred?
Yes, unfortunately. I just wanted to get the main bulk of patches in now so that I can concentrate on the real issue and don't have to carry around tons of patches.
Cheers,
Hannes
Signature · 8 lines
Dr. Hannes Reinecke zSeries & Storage [email protected] +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 [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html