All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCHv6 00/26] scsi logging update (the boring part)
@ 2014-10-06  9:01 Hannes Reinecke
  2014-10-06  9:01 ` [PATCH 01/26] Remove scsi_cmd_print_sense_hdr() Hannes Reinecke
                   ` (25 more replies)
  0 siblings, 26 replies; 30+ messages in thread
From: Hannes Reinecke @ 2014-10-06  9:01 UTC (permalink / raw)
  To: James Bottomley
  Cc: Christoph Hellwig, linux-scsi, Robert Elliott, Hannes Reinecke

Hi all,

here is now the sixth iteration of my scsi logging update.
For this I've fixed all issues noticed by Robert Elliott
(thanks, Robert!) and Ewan Milne. And added the 'Reviewed-by:'
tags where appropriate.

Hannes Reinecke (26):
  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: repurpose the 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
  scsi: check for corrent return code in scsi_eh_abort_cmds()
  scsi: correct return values for .eh_abort_handler implementations

 drivers/scsi/53c700.c             |  13 +-
 drivers/scsi/NCR5380.c            |   7 +-
 drivers/scsi/aha152x.c            | 994 +++++++++-----------------------------
 drivers/scsi/aha1740.c            |   2 +-
 drivers/scsi/arm/acornscsi.c      |  12 +-
 drivers/scsi/arm/fas216.c         |  39 +-
 drivers/scsi/atari_NCR5380.c      |   3 +-
 drivers/scsi/ch.c                 |  29 +-
 drivers/scsi/constants.c          | 576 +++++++++-------------
 drivers/scsi/esas2r/esas2r_main.c |   2 +-
 drivers/scsi/hosts.c              |   4 +-
 drivers/scsi/megaraid.c           |   8 +-
 drivers/scsi/osst.c               |   8 +-
 drivers/scsi/ps3rom.c             |   4 -
 drivers/scsi/scsi.c               |  45 +-
 drivers/scsi/scsi_error.c         | 123 ++---
 drivers/scsi/scsi_ioctl.c         |   2 +-
 drivers/scsi/scsi_lib.c           |  19 +-
 drivers/scsi/sd.c                 |  56 +--
 drivers/scsi/sd.h                 |   6 +-
 drivers/scsi/sg.c                 |   6 +-
 drivers/scsi/sr.h                 |   3 +-
 drivers/scsi/sr_ioctl.c           |  10 +-
 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           |  28 +-
 include/scsi/scsi_device.h        |   9 +
 include/scsi/scsi_eh.h            |  14 +-
 30 files changed, 686 insertions(+), 1360 deletions(-)

-- 
1.8.5.2


^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 01/26] Remove scsi_cmd_print_sense_hdr()
  2014-10-06  9:01 [PATCHv6 00/26] scsi logging update (the boring part) Hannes Reinecke
@ 2014-10-06  9:01 ` Hannes Reinecke
  2014-10-06  9:01 ` [PATCH 02/26] sd: Remove scsi_print_sense() in sd_done() Hannes Reinecke
                   ` (24 subsequent siblings)
  25 siblings, 0 replies; 30+ messages in thread
From: Hannes Reinecke @ 2014-10-06  9:01 UTC (permalink / raw)
  To: James Bottomley
  Cc: Christoph Hellwig, linux-scsi, Robert Elliott, Hannes Reinecke

Unused.

Reviewed-by: Robert Elliott <elliott@hp.com>
Reviewed-by: Christoph Hellwig <hch@infradead.org>
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/constants.c | 14 --------------
 include/scsi/scsi_dbg.h  |  2 --
 2 files changed, 16 deletions(-)

diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
index d35a5d6..2f44707 100644
--- a/drivers/scsi/constants.c
+++ b/drivers/scsi/constants.c
@@ -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)
diff --git a/include/scsi/scsi_dbg.h b/include/scsi/scsi_dbg.h
index e89844c..5a43a4c 100644
--- a/include/scsi/scsi_dbg.h
+++ b/include/scsi/scsi_dbg.h
@@ -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,
-- 
1.8.5.2


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 02/26] sd: Remove scsi_print_sense() in sd_done()
  2014-10-06  9:01 [PATCHv6 00/26] scsi logging update (the boring part) Hannes Reinecke
  2014-10-06  9:01 ` [PATCH 01/26] Remove scsi_cmd_print_sense_hdr() Hannes Reinecke
@ 2014-10-06  9:01 ` Hannes Reinecke
  2014-10-06  9:01 ` [PATCH 03/26] aha152x: Debug output update and whitespace cleanup Hannes Reinecke
                   ` (23 subsequent siblings)
  25 siblings, 0 replies; 30+ messages in thread
From: Hannes Reinecke @ 2014-10-06  9:01 UTC (permalink / raw)
  To: James Bottomley
  Cc: Christoph Hellwig, linux-scsi, Robert Elliott, Hannes Reinecke

sd_done() was calling scsi_print_sense() for a sense code
of 'NO_SENSE'.

Reviewed-by: Robert Elliott <elliott@hp.com>
Reviewed-by: Christoph Hellwig <hch@infradead.org>
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/sd.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 0cb5c9f..0ccb459 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -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;
-- 
1.8.5.2


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 03/26] aha152x: Debug output update and whitespace cleanup
  2014-10-06  9:01 [PATCHv6 00/26] scsi logging update (the boring part) Hannes Reinecke
  2014-10-06  9:01 ` [PATCH 01/26] Remove scsi_cmd_print_sense_hdr() Hannes Reinecke
  2014-10-06  9:01 ` [PATCH 02/26] sd: Remove scsi_print_sense() in sd_done() Hannes Reinecke
@ 2014-10-06  9:01 ` Hannes Reinecke
  2014-10-06  9:01 ` [PATCH 04/26] scsi: introduce sdev_prefix_printk() Hannes Reinecke
                   ` (22 subsequent siblings)
  25 siblings, 0 replies; 30+ messages in thread
From: Hannes Reinecke @ 2014-10-06  9:01 UTC (permalink / raw)
  To: James Bottomley
  Cc: Christoph Hellwig, linux-scsi, Robert Elliott, Hannes Reinecke

Remove 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: Robert Elliott <elliott@hp.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/aha152x.c | 994 +++++++++++--------------------------------------
 1 file changed, 224 insertions(+), 770 deletions(-)

diff --git a/drivers/scsi/aha152x.c b/drivers/scsi/aha152x.c
index e77b72f..2b960b3 100644
--- a/drivers/scsi/aha152x.c
+++ b/drivers/scsi/aha152x.c
@@ -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 being 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_WARNING, 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_WARNING, 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_command(ptr);
+	scmd_printk(KERN_DEBUG, ptr,
+		    "request_bufflen=%d; resid=%d; "
+		    "phase |%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;
-- 
1.8.5.2


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 04/26] scsi: introduce sdev_prefix_printk()
  2014-10-06  9:01 [PATCHv6 00/26] scsi logging update (the boring part) Hannes Reinecke
                   ` (2 preceding siblings ...)
  2014-10-06  9:01 ` [PATCH 03/26] aha152x: Debug output update and whitespace cleanup Hannes Reinecke
@ 2014-10-06  9:01 ` Hannes Reinecke
  2014-10-06  9:01 ` [PATCH 05/26] scsi: Use sdev as argument for sense code printing Hannes Reinecke
                   ` (21 subsequent siblings)
  25 siblings, 0 replies; 30+ messages in thread
From: Hannes Reinecke @ 2014-10-06  9:01 UTC (permalink / raw)
  To: James Bottomley
  Cc: Christoph Hellwig, linux-scsi, Robert Elliott, Hannes Reinecke

Like 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: Robert Elliott <elliott@hp.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/ch.c          | 3 +--
 drivers/scsi/sd.h          | 6 +++---
 drivers/scsi/sg.c          | 4 ++--
 drivers/scsi/sr.h          | 3 +--
 drivers/scsi/st.c          | 3 +--
 include/scsi/scsi_device.h | 9 +++++++++
 6 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/drivers/scsi/ch.c b/drivers/scsi/ch.c
index ef5ae0d..52060e7 100644
--- a/drivers/scsi/ch.c
+++ b/drivers/scsi/ch.c
@@ -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 {									\
diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h
index 4c3ab83..19d20af 100644
--- a/drivers/scsi/sd.h
+++ b/drivers/scsi/sd.h
@@ -103,9 +103,9 @@ 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_printk(prefix, (sdsk)->device, fmt, ##a)
 
 #define sd_first_printk(prefix, sdsk, fmt, a...)			\
 	do {								\
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 01cf888..87304c8 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -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)
 {
diff --git a/drivers/scsi/sr.h b/drivers/scsi/sr.h
index 1d1f6f4..1de3371 100644
--- a/drivers/scsi/sr.h
+++ b/drivers/scsi/sr.h
@@ -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 *);
 
diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
index d3fd6e8..e479343 100644
--- a/drivers/scsi/st.c
+++ b/drivers/scsi/st.c
@@ -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 ); }
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 27ecee7..0b18a09 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -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,		\
-- 
1.8.5.2


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 05/26] scsi: Use sdev as argument for sense code printing
  2014-10-06  9:01 [PATCHv6 00/26] scsi logging update (the boring part) Hannes Reinecke
                   ` (3 preceding siblings ...)
  2014-10-06  9:01 ` [PATCH 04/26] scsi: introduce sdev_prefix_printk() Hannes Reinecke
@ 2014-10-06  9:01 ` Hannes Reinecke
  2014-10-06  9:01 ` [PATCH 06/26] acornscsi: use scsi_print_command() Hannes Reinecke
                   ` (20 subsequent siblings)
  25 siblings, 0 replies; 30+ messages in thread
From: Hannes Reinecke @ 2014-10-06  9:01 UTC (permalink / raw)
  To: James Bottomley
  Cc: Christoph Hellwig, linux-scsi, Robert Elliott, Hannes Reinecke

We should be using the standard dev_printk() variants for
sense code printing.

Reviewed-by: Robert Elliott <elliott@hp.com>
Reviewed-by: Christoph Hellwig <hch@infradead.org>
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 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 ++++---
 include/scsi/scsi_eh.h     |   2 +-
 15 files changed, 97 insertions(+), 87 deletions(-)

diff --git a/drivers/scsi/53c700.c b/drivers/scsi/53c700.c
index fabd4be..68bf423 100644
--- a/drivers/scsi/53c700.c
+++ b/drivers/scsi/53c700.c
@@ -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,
diff --git a/drivers/scsi/ch.c b/drivers/scsi/ch.c
index 52060e7..53621a3 100644
--- a/drivers/scsi/ch.c
+++ b/drivers/scsi/ch.c
@@ -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) {
diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
index 2f44707..f9e2317 100644
--- a/drivers/scsi/constants.c
+++ b/drivers/scsi/constants.c
@@ -1292,18 +1292,19 @@ static const struct error_info additional[] =
 
 struct error_info2 {
 	unsigned char code1, code2_min, code2_max;
+	const char * str;
 	const char * fmt;
 };
 
 static const struct error_info2 additional2[] =
 {
-	{0x40, 0x00, 0x7f, "Ram failure (%x)"},
-	{0x40, 0x80, 0xff, "Diagnostic failure on component (%x)"},
-	{0x41, 0x00, 0xff, "Data path failure (%x)"},
-	{0x42, 0x00, 0xff, "Power-on or self-test failure (%x)"},
-	{0x4D, 0x00, 0xff, "Tagged overlapped commands (task tag %x)"},
-	{0x70, 0x00, 0xff, "Decompression exception short algorithm id of %x"},
-	{0, 0, 0, NULL}
+	{0x40, 0x00, 0x7f, "Ram failure", ""},
+	{0x40, 0x80, 0xff, "Diagnostic failure on component", ""},
+	{0x41, 0x00, 0xff, "Data path failure", ""},
+	{0x42, 0x00, 0xff, "Power-on or self-test failure", ""},
+	{0x4D, 0x00, 0xff, "Tagged overlapped commands", "task tag "},
+	{0x70, 0x00, 0xff, "Decompression exception", "short algorithm id of "},
+	{0, 0, 0, NULL, NULL}
 };
 
 /* description of the sense key values */
@@ -1349,7 +1350,8 @@ EXPORT_SYMBOL(scsi_sense_key_string);
  * This string may contain a "%x" and should be printed with ascq as arg.
  */
 const char *
-scsi_extd_sense_format(unsigned char asc, unsigned char ascq) {
+scsi_extd_sense_format(unsigned char asc, unsigned char ascq, const char **fmt)
+{
 #ifdef CONFIG_SCSI_CONSTANTS
 	int i;
 	unsigned short code = ((asc << 8) | ascq);
@@ -1361,7 +1363,8 @@ scsi_extd_sense_format(unsigned char asc, unsigned char ascq) {
 		if (additional2[i].code1 == asc &&
 		    ascq >= additional2[i].code2_min &&
 		    ascq <= additional2[i].code2_max)
-			return additional2[i].fmt;
+			*fmt = additional2[i].fmt;
+			return additional2[i].str;
 	}
 #endif
 	return NULL;
@@ -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(const struct scsi_device *sdev, const char *name,
+		     unsigned char asc, unsigned char ascq)
 {
-        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);
 	}
-
-	printk("\n");
 }
 EXPORT_SYMBOL(scsi_show_extd_sense);
 
 void
-scsi_show_sense_hdr(struct scsi_sense_hdr *sshdr)
+scsi_show_sense_hdr(const struct scsi_device *sdev, const char *name,
+		    const struct scsi_sense_hdr *sshdr)
 {
 	const char *sense_txt;
 
 	sense_txt = scsi_sense_key_string(sshdr->sense_key);
 	if (sense_txt)
-		printk("Sense Key : %s ", sense_txt);
+		sdev_prefix_printk(KERN_INFO, sdev, name,
+				   "Sense Key : %s [%s]%s\n", sense_txt,
+				   scsi_sense_is_deferred(sshdr) ?
+				   "deferred" : "current",
+				   sshdr->response_code >= 0x72 ?
+				   " [descriptor]" : "");
 	else
-		printk("Sense Key : 0x%x ", sshdr->sense_key);
-
-	printk("%s", scsi_sense_is_deferred(sshdr) ? "[deferred] " :
-	       "[current] ");
-
-	if (sshdr->response_code >= 0x72)
-		printk("[descriptor]");
-
-	printk("\n");
+		sdev_prefix_printk(KERN_INFO, sdev, name,
+				   "Sense Key : 0x%x [%s]%s", sshdr->sense_key,
+				   scsi_sense_is_deferred(sshdr) ?
+				   "deferred" : "current",
+				   sshdr->response_code >= 0x72 ?
+				   " [descriptor]" : "");
 }
 EXPORT_SYMBOL(scsi_show_sense_hdr);
 
@@ -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(const struct scsi_device *sdev, const char *name,
+		     const struct scsi_sense_hdr *sshdr)
 {
-	printk(KERN_INFO "%s: ", name);
-	scsi_show_sense_hdr(sshdr);
-	printk(KERN_INFO "%s: ", name);
-	scsi_show_extd_sense(sshdr->asc, sshdr->ascq);
+	scsi_show_sense_hdr(sdev, name, sshdr);
+	scsi_show_extd_sense(sdev, name, sshdr->asc, sshdr->ascq);
 }
 EXPORT_SYMBOL(scsi_print_sense_hdr);
 
@@ -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(const struct scsi_device *sdev, const char *name,
+			const unsigned char *sense_buffer, int sense_len)
 {
 	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);
 	scsi_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(const struct scsi_cmnd *cmd)
 {
-	struct scsi_sense_hdr sshdr;
+	struct gendisk *disk = cmd->request->rq_disk;
+	const char *disk_name = disk ? disk->disk_name : NULL;
 
-	scmd_printk(KERN_INFO, cmd, " ");
-	scsi_decode_sense_buffer(cmd->sense_buffer, SCSI_SENSE_BUFFERSIZE,
-				 &sshdr);
-	scsi_show_sense_hdr(&sshdr);
-	scsi_decode_sense_extras(cmd->sense_buffer, SCSI_SENSE_BUFFERSIZE,
-				 &sshdr);
-	scmd_printk(KERN_INFO, cmd, " ");
-	scsi_show_extd_sense(sshdr.asc, sshdr.ascq);
+	__scsi_print_sense(cmd->device, disk_name, cmd->sense_buffer,
+			   SCSI_SENSE_BUFFERSIZE);
 }
 EXPORT_SYMBOL(scsi_print_sense);
 
diff --git a/drivers/scsi/osst.c b/drivers/scsi/osst.c
index 0727ea7..4ba2a9f 100644
--- a/drivers/scsi/osst.c
+++ b/drivers/scsi/osst.c
@@ -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;
diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index 1423cb1..61aeaf1 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -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",
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index 6b20ef3..6c99624 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -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);
 
diff --git a/drivers/scsi/scsi_ioctl.c b/drivers/scsi/scsi_ioctl.c
index 1aaaf43..7d2de08 100644
--- a/drivers/scsi/scsi_ioctl.c
+++ b/drivers/scsi/scsi_ioctl.c
@@ -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;
 		}
 	}
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index b9a8ddd..c3220b7 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -911,7 +911,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;
@@ -1040,7 +1040,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))
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 0ccb459..848b17d 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -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)
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 87304c8..fde1f18 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -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 */
diff --git a/drivers/scsi/sr_ioctl.c b/drivers/scsi/sr_ioctl.c
index 6389fcf..17e0c2b 100644
--- a/drivers/scsi/sr_ioctl.c
+++ b/drivers/scsi/sr_ioctl.c
@@ -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;
 		}
 	}
diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
index e479343..052d1f9 100644
--- a/drivers/scsi/st.c
+++ b/drivers/scsi/st.c
@@ -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);
 		}
 	}
 
diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index fecac5d0..9168d1b 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -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)
diff --git a/include/scsi/scsi_dbg.h b/include/scsi/scsi_dbg.h
index 5a43a4c..6cbd179 100644
--- a/include/scsi/scsi_dbg.h
+++ b/include/scsi/scsi_dbg.h
@@ -2,21 +2,26 @@
 #define _SCSI_SCSI_DBG_H
 
 struct 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(const struct scsi_device *, const char *,
+				 unsigned char, unsigned char);
+extern void scsi_show_sense_hdr(const struct scsi_device *, const char *,
+				const struct scsi_sense_hdr *);
+extern void scsi_print_sense_hdr(const struct scsi_device *, const char *,
+				 const struct scsi_sense_hdr *);
+extern void scsi_print_sense(const struct scsi_cmnd *);
+extern void __scsi_print_sense(const 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 */
diff --git a/include/scsi/scsi_eh.h b/include/scsi/scsi_eh.h
index 06a8790..3d720ca 100644
--- a/include/scsi/scsi_eh.h
+++ b/include/scsi/scsi_eh.h
@@ -47,7 +47,7 @@ extern int scsi_normalize_sense(const u8 *sense_buffer, int sb_len,
 extern int 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 int scsi_sense_is_deferred(const struct scsi_sense_hdr *sshdr)
 {
 	return ((sshdr->response_code >= 0x70) && (sshdr->response_code & 1));
 }
-- 
1.8.5.2


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 06/26] acornscsi: use scsi_print_command()
  2014-10-06  9:01 [PATCHv6 00/26] scsi logging update (the boring part) Hannes Reinecke
                   ` (4 preceding siblings ...)
  2014-10-06  9:01 ` [PATCH 05/26] scsi: Use sdev as argument for sense code printing Hannes Reinecke
@ 2014-10-06  9:01 ` Hannes Reinecke
  2014-10-06  9:01 ` [PATCH 07/26] fas216: Return DID_ERROR for incomplete data transfer Hannes Reinecke
                   ` (19 subsequent siblings)
  25 siblings, 0 replies; 30+ messages in thread
From: Hannes Reinecke @ 2014-10-06  9:01 UTC (permalink / raw)
  To: James Bottomley
  Cc: Christoph Hellwig, linux-scsi, Robert Elliott, Hannes Reinecke

Update 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: Robert Elliott <elliott@hp.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/arm/acornscsi.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/arm/acornscsi.c b/drivers/scsi/arm/acornscsi.c
index d89b9b4..deaaf84 100644
--- a/drivers/scsi/arm/acornscsi.c
+++ b/drivers/scsi/arm/acornscsi.c
@@ -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);
 		    }
 		}
 	}
-- 
1.8.5.2


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 07/26] fas216: Return DID_ERROR for incomplete data transfer
  2014-10-06  9:01 [PATCHv6 00/26] scsi logging update (the boring part) Hannes Reinecke
                   ` (5 preceding siblings ...)
  2014-10-06  9:01 ` [PATCH 06/26] acornscsi: use scsi_print_command() Hannes Reinecke
@ 2014-10-06  9:01 ` Hannes Reinecke
  2014-10-06  9:01 ` [PATCH 08/26] fas216: Update logging messages Hannes Reinecke
                   ` (18 subsequent siblings)
  25 siblings, 0 replies; 30+ messages in thread
From: Hannes Reinecke @ 2014-10-06  9:01 UTC (permalink / raw)
  To: James Bottomley
  Cc: Christoph Hellwig, linux-scsi, Robert Elliott, Hannes Reinecke

fas216 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.

Reviewed-by: Robert Elliott <elliott@hp.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/arm/fas216.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/scsi/arm/fas216.c b/drivers/scsi/arm/fas216.c
index 71cfb1e..7fc6fd3 100644
--- a/drivers/scsi/arm/fas216.c
+++ b/drivers/scsi/arm/fas216.c
@@ -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;
 		}
 	}
-- 
1.8.5.2


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 08/26] fas216: Update logging messages
  2014-10-06  9:01 [PATCHv6 00/26] scsi logging update (the boring part) Hannes Reinecke
                   ` (6 preceding siblings ...)
  2014-10-06  9:01 ` [PATCH 07/26] fas216: Return DID_ERROR for incomplete data transfer Hannes Reinecke
@ 2014-10-06  9:01 ` Hannes Reinecke
  2014-10-06  9:01 ` [PATCH 09/26] 53c700: remove scsi_print_sense() usage Hannes Reinecke
                   ` (17 subsequent siblings)
  25 siblings, 0 replies; 30+ messages in thread
From: Hannes Reinecke @ 2014-10-06  9:01 UTC (permalink / raw)
  To: James Bottomley
  Cc: Christoph Hellwig, linux-scsi, Robert Elliott, Hannes Reinecke

Update logging messages to use dev_printk() variants for correct
device annotations.

Reviewed-by: Robert Elliott <elliott@hp.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/arm/fas216.c | 26 +++++++++++---------------
 1 file changed, 11 insertions(+), 15 deletions(-)

diff --git a/drivers/scsi/arm/fas216.c b/drivers/scsi/arm/fas216.c
index 7fc6fd3..cea3463 100644
--- a/drivers/scsi/arm/fas216.c
+++ b/drivers/scsi/arm/fas216.c
@@ -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,11 @@ 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 +2155,11 @@ 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 +2660,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.
-- 
1.8.5.2


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 09/26] 53c700: remove scsi_print_sense() usage
  2014-10-06  9:01 [PATCHv6 00/26] scsi logging update (the boring part) Hannes Reinecke
                   ` (7 preceding siblings ...)
  2014-10-06  9:01 ` [PATCH 08/26] fas216: Update logging messages Hannes Reinecke
@ 2014-10-06  9:01 ` Hannes Reinecke
  2014-10-06  9:01 ` [PATCH 10/26] scsi: stop decoding if scsi_normalize_sense() fails Hannes Reinecke
                   ` (16 subsequent siblings)
  25 siblings, 0 replies; 30+ messages in thread
From: Hannes Reinecke @ 2014-10-06  9:01 UTC (permalink / raw)
  To: James Bottomley
  Cc: Christoph Hellwig, linux-scsi, Robert Elliott, Hannes Reinecke

The 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: Robert Elliott <elliott@hp.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/53c700.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/53c700.c b/drivers/scsi/53c700.c
index 68bf423..179a24e 100644
--- a/drivers/scsi/53c700.c
+++ b/drivers/scsi/53c700.c
@@ -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
-- 
1.8.5.2


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 10/26] scsi: stop decoding if scsi_normalize_sense() fails
  2014-10-06  9:01 [PATCHv6 00/26] scsi logging update (the boring part) Hannes Reinecke
                   ` (8 preceding siblings ...)
  2014-10-06  9:01 ` [PATCH 09/26] 53c700: remove scsi_print_sense() usage Hannes Reinecke
@ 2014-10-06  9:01 ` Hannes Reinecke
  2014-10-06  9:01 ` [PATCH 11/26] scsi: do not decode sense extras Hannes Reinecke
                   ` (15 subsequent siblings)
  25 siblings, 0 replies; 30+ messages in thread
From: Hannes Reinecke @ 2014-10-06  9:01 UTC (permalink / raw)
  To: James Bottomley
  Cc: Christoph Hellwig, linux-scsi, Robert Elliott, Hannes Reinecke

If 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: Robert Elliott <elliott@hp.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/constants.c | 34 ++++++++++++++++------------------
 1 file changed, 16 insertions(+), 18 deletions(-)

diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
index f9e2317..0b74e94 100644
--- a/drivers/scsi/constants.c
+++ b/drivers/scsi/constants.c
@@ -1435,26 +1435,21 @@ scsi_print_sense_hdr(const 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(const 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);
-- 
1.8.5.2


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 11/26] scsi: do not decode sense extras
  2014-10-06  9:01 [PATCHv6 00/26] scsi logging update (the boring part) Hannes Reinecke
                   ` (9 preceding siblings ...)
  2014-10-06  9:01 ` [PATCH 10/26] scsi: stop decoding if scsi_normalize_sense() fails Hannes Reinecke
@ 2014-10-06  9:01 ` Hannes Reinecke
  2014-10-06  9:01 ` [PATCH 12/26] scsi: use 'bool' as return value for scsi_normalize_sense() Hannes Reinecke
                   ` (14 subsequent siblings)
  25 siblings, 0 replies; 30+ messages in thread
From: Hannes Reinecke @ 2014-10-06  9:01 UTC (permalink / raw)
  To: James Bottomley
  Cc: Christoph Hellwig, linux-scsi, Robert Elliott, Hannes Reinecke

Currently 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: Robert Elliott <elliott@hp.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/constants.c | 62 ------------------------------------------------
 1 file changed, 62 deletions(-)

diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
index 0b74e94..ec69ec8 100644
--- a/drivers/scsi/constants.c
+++ b/drivers/scsi/constants.c
@@ -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(const struct scsi_device *sdev, const char *name,
 			const unsigned char *sense_buffer, int sense_len)
@@ -1524,7 +1463,6 @@ void __scsi_print_sense(const 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);
-- 
1.8.5.2


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 12/26] scsi: use 'bool' as return value for scsi_normalize_sense()
  2014-10-06  9:01 [PATCHv6 00/26] scsi logging update (the boring part) Hannes Reinecke
                   ` (10 preceding siblings ...)
  2014-10-06  9:01 ` [PATCH 11/26] scsi: do not decode sense extras Hannes Reinecke
@ 2014-10-06  9:01 ` Hannes Reinecke
  2014-10-06  9:01 ` [PATCH 13/26] scsi: remove scsi_print_status() Hannes Reinecke
                   ` (13 subsequent siblings)
  25 siblings, 0 replies; 30+ messages in thread
From: Hannes Reinecke @ 2014-10-06  9:01 UTC (permalink / raw)
  To: James Bottomley
  Cc: Christoph Hellwig, linux-scsi, Robert Elliott, Hannes Reinecke

Convert scsi_normalize_sense() and frieds to return 'bool'
instead of an integer.

Reviewed-by: Robert Elliott <elliott@hp.com>
Reviewed-by: Yoshihiro Yunomae <yoshihiro.yunomae.ez@hitachi.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/scsi_error.c | 16 ++++++++--------
 drivers/scsi/scsi_lib.c   |  2 +-
 include/scsi/scsi_eh.h    | 14 +++++++-------
 3 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index 6c99624..a113e99 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -2408,20 +2408,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) {
 		/*
@@ -2451,12 +2451,12 @@ 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,
-				 struct scsi_sense_hdr *sshdr)
+bool scsi_command_normalize_sense(const struct scsi_cmnd *cmd,
+				  struct scsi_sense_hdr *sshdr)
 {
 	return scsi_normalize_sense(cmd->sense_buffer,
 			SCSI_SENSE_BUFFERSIZE, sshdr);
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index c3220b7..b853659 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -830,7 +830,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;
diff --git a/include/scsi/scsi_eh.h b/include/scsi/scsi_eh.h
index 3d720ca..1d3c254 100644
--- a/include/scsi/scsi_eh.h
+++ b/include/scsi/scsi_eh.h
@@ -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(const 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(const struct scsi_cmnd *cmd,
+					 struct scsi_sense_hdr *sshdr);
 
-static inline int scsi_sense_is_deferred(const struct scsi_sense_hdr *sshdr)
+static inline bool scsi_sense_is_deferred(const struct scsi_sense_hdr *sshdr)
 {
 	return ((sshdr->response_code >= 0x70) && (sshdr->response_code & 1));
 }
-- 
1.8.5.2


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 13/26] scsi: remove scsi_print_status()
  2014-10-06  9:01 [PATCHv6 00/26] scsi logging update (the boring part) Hannes Reinecke
                   ` (11 preceding siblings ...)
  2014-10-06  9:01 ` [PATCH 12/26] scsi: use 'bool' as return value for scsi_normalize_sense() Hannes Reinecke
@ 2014-10-06  9:01 ` Hannes Reinecke
  2014-10-06  9:01 ` [PATCH 14/26] Implement scsi_opcode_sa_name Hannes Reinecke
                   ` (12 subsequent siblings)
  25 siblings, 0 replies; 30+ messages in thread
From: Hannes Reinecke @ 2014-10-06  9:01 UTC (permalink / raw)
  To: James Bottomley
  Cc: Christoph Hellwig, linux-scsi, Robert Elliott, Hannes Reinecke

Last caller is gone, so we can remove it.

Reviewed-bt: Robert Elliott <elliott@hp.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/constants.c | 35 -----------------------------------
 include/scsi/scsi_dbg.h  |  1 -
 2 files changed, 36 deletions(-)

diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
index ec69ec8..8253f7b 100644
--- a/drivers/scsi/constants.c
+++ b/drivers/scsi/constants.c
@@ -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 {
diff --git a/include/scsi/scsi_dbg.h b/include/scsi/scsi_dbg.h
index 6cbd179..386474e 100644
--- a/include/scsi/scsi_dbg.h
+++ b/include/scsi/scsi_dbg.h
@@ -19,7 +19,6 @@ extern void __scsi_print_sense(const 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 **);
-- 
1.8.5.2


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 14/26] Implement scsi_opcode_sa_name
  2014-10-06  9:01 [PATCHv6 00/26] scsi logging update (the boring part) Hannes Reinecke
                   ` (12 preceding siblings ...)
  2014-10-06  9:01 ` [PATCH 13/26] scsi: remove scsi_print_status() Hannes Reinecke
@ 2014-10-06  9:01 ` Hannes Reinecke
  2014-10-06  9:01 ` [PATCH 15/26] scsi: merge print_opcode_name() Hannes Reinecke
                   ` (11 subsequent siblings)
  25 siblings, 0 replies; 30+ messages in thread
From: Hannes Reinecke @ 2014-10-06  9:01 UTC (permalink / raw)
  To: James Bottomley
  Cc: Christoph Hellwig, linux-scsi, Robert Elliott, Hannes Reinecke

Implement a lookup array for SERVICE ACTION commands instead
of hardcoding it in a large switch statement.

Reviewed-by: Robert Elliott <elliott@hp.com>
Reviewed-by: Christoph Hellwig <hch@infradead.org>
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/constants.c | 131 +++++++++++++++++++----------------------------
 1 file changed, 53 insertions(+), 78 deletions(-)

diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
index 8253f7b..1a1ddfb 100644
--- a/drivers/scsi/constants.c
+++ b/drivers/scsi/constants.c
@@ -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 opcode;
+	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 opcode, 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->opcode == opcode) {
+			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);
 	}
 }
 
-- 
1.8.5.2


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 15/26] scsi: merge print_opcode_name()
  2014-10-06  9:01 [PATCHv6 00/26] scsi logging update (the boring part) Hannes Reinecke
                   ` (13 preceding siblings ...)
  2014-10-06  9:01 ` [PATCH 14/26] Implement scsi_opcode_sa_name Hannes Reinecke
@ 2014-10-06  9:01 ` Hannes Reinecke
  2014-10-06  9:01 ` [PATCH 16/26] scsi: consolidate opcode lookup in scsi_opcode_sa_name() Hannes Reinecke
                   ` (10 subsequent siblings)
  25 siblings, 0 replies; 30+ messages in thread
From: Hannes Reinecke @ 2014-10-06  9:01 UTC (permalink / raw)
  To: James Bottomley
  Cc: Christoph Hellwig, linux-scsi, Robert Elliott, Hannes Reinecke

Instead of having two versions of print_opcode_name() we
should be consolidating them into one version.

Reviewed-by: Robert Elliott <elliott@hp.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/constants.c | 90 ++++++++++++++++++------------------------------
 1 file changed, 34 insertions(+), 56 deletions(-)

diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
index 1a1ddfb..3b22740 100644
--- a/drivers/scsi/constants.c
+++ b/drivers/scsi/constants.c
@@ -30,6 +30,11 @@
 #define THIRD_PARTY_COPY_IN 0x84
 
 
+struct sa_name_list {
+	int opcode;
+	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 opcode;
-	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 opcode, int service_action,
 				const char **sa_name)
 {
@@ -273,7 +292,7 @@ static bool scsi_opcode_sa_name(int opcode, 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; sa_name_ptr->opcode; ++k, ++sa_name_ptr) {
 		if (sa_name_ptr->opcode == opcode) {
 			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) > 1) {
+				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;
-- 
1.8.5.2


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 16/26] scsi: consolidate opcode lookup in scsi_opcode_sa_name()
  2014-10-06  9:01 [PATCHv6 00/26] scsi logging update (the boring part) Hannes Reinecke
                   ` (14 preceding siblings ...)
  2014-10-06  9:01 ` [PATCH 15/26] scsi: merge print_opcode_name() Hannes Reinecke
@ 2014-10-06  9:01 ` Hannes Reinecke
  2014-10-06  9:01 ` [PATCH 17/26] scsi: repurpose the last argument from print_opcode_name() Hannes Reinecke
                   ` (9 subsequent siblings)
  25 siblings, 0 replies; 30+ messages in thread
From: Hannes Reinecke @ 2014-10-06  9:01 UTC (permalink / raw)
  To: James Bottomley
  Cc: Christoph Hellwig, linux-scsi, Robert Elliott, Hannes Reinecke

Consolidate 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: Robert Elliott <elliott@hp.com>
Reviewed-by: Christoph Hellweg <hch@lst.de>
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/constants.c | 37 ++++++++++++++++++++++---------------
 1 file changed, 22 insertions(+), 15 deletions(-)

diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
index 3b22740..6d34d1f 100644
--- a/drivers/scsi/constants.c
+++ b/drivers/scsi/constants.c
@@ -29,6 +29,7 @@
 #define THIRD_PARTY_COPY_OUT 0x83
 #define THIRD_PARTY_COPY_IN 0x84
 
+#define VENDOR_SPECIFIC_CDB 0xc0
 
 struct sa_name_list {
 	int opcode;
@@ -286,12 +287,19 @@ static struct sa_name_list sa_names_arr[] = {
 #endif /* CONFIG_SCSI_CONSTANTS */
 
 static bool scsi_opcode_sa_name(int opcode, 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 (opcode >= VENDOR_SPECIFIC_CDB)
+		return false;
+
+	if (opcode < ARRAY_SIZE(cdb_byte0_names))
+		*cdb_name = cdb_byte0_names[opcode];
+
 	for (k = 0; sa_name_ptr->opcode; ++k, ++sa_name_ptr) {
 		if (sa_name_ptr->opcode == opcode) {
 			arr = sa_name_ptr->arr;
@@ -316,7 +324,7 @@ static bool scsi_opcode_sa_name(int opcode, 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) > 1) {
-				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);
 
-- 
1.8.5.2


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 17/26] scsi: repurpose the last argument from print_opcode_name()
  2014-10-06  9:01 [PATCHv6 00/26] scsi logging update (the boring part) Hannes Reinecke
                   ` (15 preceding siblings ...)
  2014-10-06  9:01 ` [PATCH 16/26] scsi: consolidate opcode lookup in scsi_opcode_sa_name() Hannes Reinecke
@ 2014-10-06  9:01 ` Hannes Reinecke
  2014-10-06  9:01 ` [PATCH 18/26] scsi: Remove scsi_print_command when calling abort Hannes Reinecke
                   ` (8 subsequent siblings)
  25 siblings, 0 replies; 30+ messages in thread
From: Hannes Reinecke @ 2014-10-06  9:01 UTC (permalink / raw)
  To: James Bottomley
  Cc: Christoph Hellwig, linux-scsi, Robert Elliott, Hannes Reinecke

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.
Instead we should be using the last argument to specify
the cdb length to avoid accidental overflow when reading
the cdb buffer.

Reviewed-by: Robert Elliott <elliott@hp.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/arm/fas216.c |  2 +-
 drivers/scsi/ch.c         | 24 +++++++++++++-----------
 drivers/scsi/constants.c  | 25 ++++++++++---------------
 drivers/scsi/sr_ioctl.c   |  4 ++--
 include/scsi/scsi_dbg.h   |  2 +-
 5 files changed, 27 insertions(+), 30 deletions(-)

diff --git a/drivers/scsi/arm/fas216.c b/drivers/scsi/arm/fas216.c
index cea3463..d2581cb 100644
--- a/drivers/scsi/arm/fas216.c
+++ b/drivers/scsi/arm/fas216.c
@@ -2424,7 +2424,7 @@ 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);
+	__scsi_print_command(SCpnt->cmnd, SCpnt->cmd_len);
 
 	print_debug_list();
 	fas216_dumpstate(info);
diff --git a/drivers/scsi/ch.c b/drivers/scsi/ch.c
index 53621a3..226ef77 100644
--- a/drivers/scsi/ch.c
+++ b/drivers/scsi/ch.c
@@ -182,7 +182,7 @@ static int ch_find_errno(struct scsi_sense_hdr *sshdr)
 }
 
 static int
-ch_do_scsi(scsi_changer *ch, unsigned char *cmd,
+ch_do_scsi(scsi_changer *ch, unsigned char *cmd, int cmd_len,
 	   void *buffer, unsigned buflength,
 	   enum dma_data_direction direction)
 {
@@ -196,7 +196,7 @@ ch_do_scsi(scsi_changer *ch, unsigned char *cmd,
 	errno = 0;
 	if (debug) {
 		DPRINTK("command: ");
-		__scsi_print_command(cmd);
+		__scsi_print_command(cmd, cmd_len);
 	}
 
 	result = scsi_execute_req(ch->device, cmd, direction, buffer,
@@ -257,7 +257,8 @@ ch_read_element_status(scsi_changer *ch, u_int elem, char *data)
 	cmd[3] = elem        & 0xff;
 	cmd[5] = 1;
 	cmd[9] = 255;
-	if (0 == (result = ch_do_scsi(ch, cmd, buffer, 256, DMA_FROM_DEVICE))) {
+	if (0 == (result = ch_do_scsi(ch, cmd, 12,
+				      buffer, 256, DMA_FROM_DEVICE))) {
 		if (((buffer[16] << 8) | buffer[17]) != elem) {
 			DPRINTK("asked for element 0x%02x, got 0x%02x\n",
 				elem,(buffer[16] << 8) | buffer[17]);
@@ -287,7 +288,7 @@ ch_init_elem(scsi_changer *ch)
 	memset(cmd,0,sizeof(cmd));
 	cmd[0] = INITIALIZE_ELEMENT_STATUS;
 	cmd[1] = (ch->device->lun & 0x7) << 5;
-	err = ch_do_scsi(ch, cmd, NULL, 0, DMA_NONE);
+	err = ch_do_scsi(ch, cmd, 6, NULL, 0, DMA_NONE);
 	VPRINTK(KERN_INFO, "... finished\n");
 	return err;
 }
@@ -309,10 +310,10 @@ ch_readconfig(scsi_changer *ch)
 	cmd[1] = (ch->device->lun & 0x7) << 5;
 	cmd[2] = 0x1d;
 	cmd[4] = 255;
-	result = ch_do_scsi(ch, cmd, buffer, 255, DMA_FROM_DEVICE);
+	result = ch_do_scsi(ch, cmd, 10, buffer, 255, DMA_FROM_DEVICE);
 	if (0 != result) {
 		cmd[1] |= (1<<3);
-		result  = ch_do_scsi(ch, cmd, buffer, 255, DMA_FROM_DEVICE);
+		result  = ch_do_scsi(ch, cmd, 10, buffer, 255, DMA_FROM_DEVICE);
 	}
 	if (0 == result) {
 		ch->firsts[CHET_MT] =
@@ -437,7 +438,7 @@ ch_position(scsi_changer *ch, u_int trans, u_int elem, int rotate)
 	cmd[4]  = (elem  >> 8) & 0xff;
 	cmd[5]  =  elem        & 0xff;
 	cmd[8]  = rotate ? 1 : 0;
-	return ch_do_scsi(ch, cmd, NULL, 0, DMA_NONE);
+	return ch_do_scsi(ch, cmd, 10, NULL, 0, DMA_NONE);
 }
 
 static int
@@ -458,7 +459,7 @@ ch_move(scsi_changer *ch, u_int trans, u_int src, u_int dest, int rotate)
 	cmd[6]  = (dest  >> 8) & 0xff;
 	cmd[7]  =  dest        & 0xff;
 	cmd[10] = rotate ? 1 : 0;
-	return ch_do_scsi(ch, cmd, NULL,0, DMA_NONE);
+	return ch_do_scsi(ch, cmd, 12, NULL,0, DMA_NONE);
 }
 
 static int
@@ -484,7 +485,7 @@ ch_exchange(scsi_changer *ch, u_int trans, u_int src,
 	cmd[9]  =  dest2       & 0xff;
 	cmd[10] = (rotate1 ? 1 : 0) | (rotate2 ? 2 : 0);
 
-	return ch_do_scsi(ch, cmd, NULL,0, DMA_NONE);
+	return ch_do_scsi(ch, cmd, 12, NULL, 0, DMA_NONE);
 }
 
 static void
@@ -534,7 +535,7 @@ ch_set_voltag(scsi_changer *ch, u_int elem,
 	memcpy(buffer,tag,32);
 	ch_check_voltag(buffer);
 
-	result = ch_do_scsi(ch, cmd, buffer, 256, DMA_TO_DEVICE);
+	result = ch_do_scsi(ch, cmd, 12, buffer, 256, DMA_TO_DEVICE);
 	kfree(buffer);
 	return result;
 }
@@ -765,7 +766,8 @@ static long ch_ioctl(struct file *file,
 		ch_cmd[5] = 1;
 		ch_cmd[9] = 255;
 
-		result = ch_do_scsi(ch, ch_cmd, buffer, 256, DMA_FROM_DEVICE);
+		result = ch_do_scsi(ch, ch_cmd, 12,
+				    buffer, 256, DMA_FROM_DEVICE);
 		if (!result) {
 			cge.cge_status = buffer[18];
 			cge.cge_flags = 0;
diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
index 6d34d1f..84b2614 100644
--- a/drivers/scsi/constants.c
+++ b/drivers/scsi/constants.c
@@ -320,25 +320,21 @@ static bool scsi_opcode_sa_name(int opcode, 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(const unsigned char *cdbp, size_t cdb_len)
 {
-	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);
-		if (len < 10) {
-			printk("short variable length command, "
-			       "len=%d ext_len=%d", len, cdb_len);
+		if (cdb_len < 10) {
+			printk("short variable length command, len=%zu",
+			       cdb_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,18 +352,17 @@ 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);
 	}
 }
 
-void __scsi_print_command(unsigned char *cdb)
+void __scsi_print_command(const unsigned char *cdb, size_t cdb_len)
 {
 	int k, len;
 
-	print_opcode_name(cdb, 0);
+	print_opcode_name(cdb, cdb_len);
 	len = scsi_command_size(cdb);
+	if (cdb_len < len)
+		len = cdb_len;
 	/* print out all bytes in cdb */
 	for (k = 0; k < len; ++k)
 		printk(" %02x", cdb[k]);
diff --git a/drivers/scsi/sr_ioctl.c b/drivers/scsi/sr_ioctl.c
index 17e0c2b..fb929fa 100644
--- a/drivers/scsi/sr_ioctl.c
+++ b/drivers/scsi/sr_ioctl.c
@@ -257,14 +257,14 @@ int sr_do_ioctl(Scsi_CD *cd, struct packet_command *cgc)
 				/* sense: Invalid command operation code */
 				err = -EDRIVE_CANT_DO_THIS;
 #ifdef DEBUG
-			__scsi_print_command(cgc->cmd);
+			__scsi_print_command(cgc->cmd, CDROM_PACKET_SIZE);
 			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_command(cgc->cmd, CDROM_PACKET_SIZE);
 			scsi_print_sense_hdr(cd->device, cd->cdi.name, &sshdr);
 			err = -EIO;
 		}
diff --git a/include/scsi/scsi_dbg.h b/include/scsi/scsi_dbg.h
index 386474e..81d0418 100644
--- a/include/scsi/scsi_dbg.h
+++ b/include/scsi/scsi_dbg.h
@@ -6,7 +6,7 @@ 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_print_command(const unsigned char *, size_t);
 extern void scsi_show_extd_sense(const struct scsi_device *, const char *,
 				 unsigned char, unsigned char);
 extern void scsi_show_sense_hdr(const struct scsi_device *, const char *,
-- 
1.8.5.2


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 18/26] scsi: Remove scsi_print_command when calling abort
  2014-10-06  9:01 [PATCHv6 00/26] scsi logging update (the boring part) Hannes Reinecke
                   ` (16 preceding siblings ...)
  2014-10-06  9:01 ` [PATCH 17/26] scsi: repurpose the last argument from print_opcode_name() Hannes Reinecke
@ 2014-10-06  9:01 ` Hannes Reinecke
  2014-10-06  9:01 ` [PATCH 19/26] scsi: separate out scsi_(host|driver)byte_string() Hannes Reinecke
                   ` (7 subsequent siblings)
  25 siblings, 0 replies; 30+ messages in thread
From: Hannes Reinecke @ 2014-10-06  9:01 UTC (permalink / raw)
  To: James Bottomley
  Cc: Christoph Hellwig, linux-scsi, Robert Elliott, Hannes Reinecke

Calling scsi_print_command should not be necessary during abort;
if the information is required one should enable scsi logging.

Reviewed-by: Robert Elliott <elliott@hp.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 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(-)

diff --git a/drivers/scsi/53c700.c b/drivers/scsi/53c700.c
index 179a24e..474cc6d 100644
--- a/drivers/scsi/53c700.c
+++ b/drivers/scsi/53c700.c
@@ -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;
 
diff --git a/drivers/scsi/NCR5380.c b/drivers/scsi/NCR5380.c
index 45da3c8..50873bb 100644
--- a/drivers/scsi/NCR5380.c
+++ b/drivers/scsi/NCR5380.c
@@ -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);
 
diff --git a/drivers/scsi/arm/fas216.c b/drivers/scsi/arm/fas216.c
index d2581cb..e64c3af 100644
--- a/drivers/scsi/arm/fas216.c
+++ b/drivers/scsi/arm/fas216.c
@@ -2423,14 +2423,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, SCpnt->cmd_len);
+	scmd_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
@@ -2438,7 +2435,7 @@ int fas216_eh_abort(struct scsi_cmnd *SCpnt)
 	 * target, or the busylun bit is not set.
 	 */
 	case res_success:
-		printk("success\n");
+		scmd_printk(KERN_WARNING, SCpnt, "abort %p success\n", SCpnt);
 		result = SUCCESS;
 		break;
 
@@ -2448,14 +2445,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");
+		scmd_printk(KERN_WARNING, SCpnt, "abort %p failed\n", SCpnt);
 		break;
 	}
 
diff --git a/drivers/scsi/atari_NCR5380.c b/drivers/scsi/atari_NCR5380.c
index 79e6f04..229c61b 100644
--- a/drivers/scsi/atari_NCR5380.c
+++ b/drivers/scsi/atari_NCR5380.c
@@ -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);
+	scmd_printk(KERN_NOTICE, cmd, "aborting command\n");
 
 	NCR5380_print_status(instance);
 
diff --git a/drivers/scsi/ps3rom.c b/drivers/scsi/ps3rom.c
index ef23fab..b3b48b5 100644
--- a/drivers/scsi/ps3rom.c
+++ b/drivers/scsi/ps3rom.c
@@ -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;
 
diff --git a/drivers/scsi/stex.c b/drivers/scsi/stex.c
index 1aa4bef..713af13 100644
--- a/drivers/scsi/stex.c
+++ b/drivers/scsi/stex.c
@@ -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;
 }
diff --git a/drivers/scsi/sun3_NCR5380.c b/drivers/scsi/sun3_NCR5380.c
index 1a2367a..3abd796 100644
--- a/drivers/scsi/sun3_NCR5380.c
+++ b/drivers/scsi/sun3_NCR5380.c
@@ -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);
+    scmd_printk(KERN_NOTICE, cmd, "aborting command\n");
 
     NCR5380_print_status (instance);
 
-- 
1.8.5.2


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 19/26] scsi: separate out scsi_(host|driver)byte_string()
  2014-10-06  9:01 [PATCHv6 00/26] scsi logging update (the boring part) Hannes Reinecke
                   ` (17 preceding siblings ...)
  2014-10-06  9:01 ` [PATCH 18/26] scsi: Remove scsi_print_command when calling abort Hannes Reinecke
@ 2014-10-06  9:01 ` Hannes Reinecke
  2014-10-06  9:01 ` [PATCH 20/26] sd: Cleanup logging Hannes Reinecke
                   ` (6 subsequent siblings)
  25 siblings, 0 replies; 30+ messages in thread
From: Hannes Reinecke @ 2014-10-06  9:01 UTC (permalink / raw)
  To: James Bottomley
  Cc: Christoph Hellwig, linux-scsi, Robert Elliott, Hannes Reinecke

Export functions for later use.

Reviewed-by: Robert Elliott <elliott@hp.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/constants.c | 58 ++++++++++++++++++++++++++++++++++++------------
 include/scsi/scsi_dbg.h  |  2 ++
 2 files changed, 46 insertions(+), 14 deletions(-)

diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
index 84b2614..3244ec2 100644
--- a/drivers/scsi/constants.c
+++ b/drivers/scsi/constants.c
@@ -1406,38 +1406,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);
diff --git a/include/scsi/scsi_dbg.h b/include/scsi/scsi_dbg.h
index 81d0418..50f4d85 100644
--- a/include/scsi/scsi_dbg.h
+++ b/include/scsi/scsi_dbg.h
@@ -19,6 +19,8 @@ extern void __scsi_print_sense(const 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 **);
-- 
1.8.5.2


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 20/26] sd: Cleanup logging
  2014-10-06  9:01 [PATCHv6 00/26] scsi logging update (the boring part) Hannes Reinecke
                   ` (18 preceding siblings ...)
  2014-10-06  9:01 ` [PATCH 19/26] scsi: separate out scsi_(host|driver)byte_string() Hannes Reinecke
@ 2014-10-06  9:01 ` Hannes Reinecke
  2014-10-06  9:01 ` [PATCH 21/26] scsi: simplify scsi_log_(send|completion) Hannes Reinecke
                   ` (5 subsequent siblings)
  25 siblings, 0 replies; 30+ messages in thread
From: Hannes Reinecke @ 2014-10-06  9:01 UTC (permalink / raw)
  To: James Bottomley
  Cc: Christoph Hellwig, linux-scsi, Robert Elliott, Hannes Reinecke

Open-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: Robert Elliott <elliott@hp.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/constants.c | 16 ----------------
 drivers/scsi/sd.c        | 46 ++++++++++++++++++++++++----------------------
 include/scsi/scsi_dbg.h  |  1 -
 3 files changed, 24 insertions(+), 39 deletions(-)

diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
index 3244ec2..1ebdc5d 100644
--- a/drivers/scsi/constants.c
+++ b/drivers/scsi/constants.c
@@ -1439,22 +1439,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);
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 848b17d..20b0539 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -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);
@@ -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);
 
 		if (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,
-						   "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));
 	sdkp->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);
 			}
 			break;
 		}
-					
+
 		/*
 		 * The device does not want the automatic start to be issued.
 		 */
@@ -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);
 	if (driver_byte(the_result) & DRIVER_SENSE)
 		sd_print_sense_hdr(sdkp, sshdr);
 	else
@@ -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);
 		if (driver_byte(res) & DRIVER_SENSE)
 			sd_print_sense_hdr(sdkp, &sshdr);
 		if (scsi_sense_valid(&sshdr) &&
@@ -3328,9 +3319,20 @@ 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);
+	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 ? hb_string : "invalid",
+			  db_string ? db_string : "invalid");
+	else
+		sd_printk(KERN_INFO, sdkp,
+			  "%s: Result: hostbyte=0x%02x driverbyte=0x%02x\n",
+			  msg, host_byte(result), driver_byte(result));
 }
 
diff --git a/include/scsi/scsi_dbg.h b/include/scsi/scsi_dbg.h
index 50f4d85..f41a86b 100644
--- a/include/scsi/scsi_dbg.h
+++ b/include/scsi/scsi_dbg.h
@@ -17,7 +17,6 @@ extern void scsi_print_sense(const struct scsi_cmnd *);
 extern void __scsi_print_sense(const 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);
-- 
1.8.5.2


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 21/26] scsi: simplify scsi_log_(send|completion)
  2014-10-06  9:01 [PATCHv6 00/26] scsi logging update (the boring part) Hannes Reinecke
                   ` (19 preceding siblings ...)
  2014-10-06  9:01 ` [PATCH 20/26] sd: Cleanup logging Hannes Reinecke
@ 2014-10-06  9:01 ` Hannes Reinecke
  2014-10-06 19:53   ` Ewan Milne
  2014-10-06  9:01 ` [PATCH 22/26] scsi: fixup logging messages in scsi_error.c Hannes Reinecke
                   ` (4 subsequent siblings)
  25 siblings, 1 reply; 30+ messages in thread
From: Hannes Reinecke @ 2014-10-06  9:01 UTC (permalink / raw)
  To: James Bottomley
  Cc: Christoph Hellwig, linux-scsi, Robert Elliott, Hannes Reinecke

Simplify scsi_log_(send|completion) by externalizing
scsi_mlreturn_string() and always print the command address.

Reviewed-by: Robert Elliott <elliott@hp.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 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(-)

diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
index 1ebdc5d..59b7f9e 100644
--- a/drivers/scsi/constants.c
+++ b/drivers/scsi/constants.c
@@ -1439,19 +1439,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);
diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index 61aeaf1..3d81a07 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -527,9 +527,9 @@ void scsi_log_send(struct scsi_cmnd *cmd)
 	 *
 	 * 1: nothing (match completion)
 	 *
-	 * 2: log opcode + command of all commands
+	 * 2: log opcode + command of all commands + cmd address
 	 *
-	 * 3: same as 2 plus dump cmd address
+	 * 3: same as 2
 	 *
 	 * 4: same as 3 plus dump extra junk
 	 */
@@ -537,10 +537,8 @@ void scsi_log_send(struct scsi_cmnd *cmd)
 		level = SCSI_LOG_LEVEL(SCSI_LOG_MLQUEUE_SHIFT,
 				       SCSI_LOG_MLQUEUE_BITS);
 		if (level > 1) {
-			scmd_printk(KERN_INFO, cmd, "Send: ");
-			if (level > 2)
-				printk("0x%p ", cmd);
-			printk("\n");
+			scmd_printk(KERN_INFO, cmd,
+				    "Send: scmd 0x%p\n", cmd);
 			scsi_print_command(cmd);
 			if (level > 3) {
 				printk(KERN_INFO "buffer = 0x%p, bufflen = %d,"
@@ -565,7 +563,7 @@ void scsi_log_completion(struct scsi_cmnd *cmd, int disposition)
 	 *
 	 * 2: same as 1 but for all command completions.
 	 *
-	 * 3: same as 2 plus dump cmd address
+	 * 3: same as 2
 	 *
 	 * 4: same as 3 plus dump extra junk
 	 */
@@ -574,36 +572,7 @@ void scsi_log_completion(struct scsi_cmnd *cmd, int disposition)
 				       SCSI_LOG_MLCOMPLETE_BITS);
 		if (((level > 0) && (cmd->result || disposition != SUCCESS)) ||
 		    (level > 1)) {
-			scmd_printk(KERN_INFO, cmd, "Done: ");
-			if (level > 2)
-				printk("0x%p ", cmd);
-			/*
-			 * Dump truncated values, so we usually fit within
-			 * 80 chars.
-			 */
-			switch (disposition) {
-			case SUCCESS:
-				printk("SUCCESS\n");
-				break;
-			case NEEDS_RETRY:
-				printk("RETRY\n");
-				break;
-			case ADD_TO_MLQUEUE:
-				printk("MLQUEUE\n");
-				break;
-			case FAILED:
-				printk("FAILED\n");
-				break;
-			case TIMEOUT_ERROR:
-				/* 
-				 * If called via scsi_times_out.
-				 */
-				printk("TIMEOUT\n");
-				break;
-			default:
-				printk("UNKNOWN\n");
-			}
-			scsi_print_result(cmd);
+			scsi_print_result(cmd, "Done: ", disposition);
 			scsi_print_command(cmd);
 			if (status_byte(cmd->result) & CHECK_CONDITION)
 				scsi_print_sense(cmd);
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index b853659..2df485f 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -831,7 +831,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;
@@ -1037,8 +1037,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);
diff --git a/include/scsi/scsi_dbg.h b/include/scsi/scsi_dbg.h
index f41a86b..7982795 100644
--- a/include/scsi/scsi_dbg.h
+++ b/include/scsi/scsi_dbg.h
@@ -17,9 +17,10 @@ extern void scsi_print_sense(const struct scsi_cmnd *);
 extern void __scsi_print_sense(const 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 **);
-- 
1.8.5.2


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 22/26] scsi: fixup logging messages in scsi_error.c
  2014-10-06  9:01 [PATCHv6 00/26] scsi logging update (the boring part) Hannes Reinecke
                   ` (20 preceding siblings ...)
  2014-10-06  9:01 ` [PATCH 21/26] scsi: simplify scsi_log_(send|completion) Hannes Reinecke
@ 2014-10-06  9:01 ` Hannes Reinecke
  2014-10-06  9:01 ` [PATCH 23/26] scsi: use shost argument in scsi_eh_prt_fail_stats Hannes Reinecke
                   ` (3 subsequent siblings)
  25 siblings, 0 replies; 30+ messages in thread
From: Hannes Reinecke @ 2014-10-06  9:01 UTC (permalink / raw)
  To: James Bottomley
  Cc: Christoph Hellwig, linux-scsi, Robert Elliott, Hannes Reinecke

Use the matching scope for logging messages to allow for
better command tracing.

Suggested-by: Robert Elliott <elliott@hp.com>
Reviewed-by: Robert Elliott <elliott@hp.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/hosts.c      |  4 +--
 drivers/scsi/scsi_error.c | 77 +++++++++++++++++++++++------------------------
 2 files changed, 39 insertions(+), 42 deletions(-)

diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index 6de80e3..06030e1 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -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);
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index a113e99..087ca02 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -1156,9 +1156,9 @@ 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 +1265,9 @@ 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 +1318,20 @@ 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",
+					    current->comm));
 			return list_empty(work_q);
 		}
 		SCSI_LOG_ERROR_RECOVERY(3,
-			shost_printk(KERN_INFO, shost,
-				     "%s: aborting cmd: 0x%p\n",
-				     current->comm, scmd));
+			scmd_printk(KERN_INFO, scmd,
+				     "%s: aborting cmd\n", current->comm));
 		rtn = 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 +1389,9 @@ 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 +1406,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 +1422,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 +1455,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 +1471,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 +1488,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 +1525,8 @@ 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 +1588,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);
 		}
 
@@ -2189,9 +2186,9 @@ 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;
 		}
 
-- 
1.8.5.2


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 23/26] scsi: use shost argument in scsi_eh_prt_fail_stats
  2014-10-06  9:01 [PATCHv6 00/26] scsi logging update (the boring part) Hannes Reinecke
                   ` (21 preceding siblings ...)
  2014-10-06  9:01 ` [PATCH 22/26] scsi: fixup logging messages in scsi_error.c Hannes Reinecke
@ 2014-10-06  9:01 ` Hannes Reinecke
  2014-10-06  9:01 ` [PATCH 24/26] scsi_error: document scsi_try_to_abort_cmd Hannes Reinecke
                   ` (2 subsequent siblings)
  25 siblings, 0 replies; 30+ messages in thread
From: Hannes Reinecke @ 2014-10-06  9:01 UTC (permalink / raw)
  To: James Bottomley
  Cc: Christoph Hellwig, linux-scsi, Robert Elliott, Hannes Reinecke

The EH statistics are per host, so we should be using
shost_printk() here.

Suggested-by: Robert Elliott <elliott@hp.com>
Reviewed-by: Robert Elliott <elliott@hp.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/scsi_error.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index 087ca02..c485f28 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -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));
-- 
1.8.5.2


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 24/26] scsi_error: document scsi_try_to_abort_cmd
  2014-10-06  9:01 [PATCHv6 00/26] scsi logging update (the boring part) Hannes Reinecke
                   ` (22 preceding siblings ...)
  2014-10-06  9:01 ` [PATCH 23/26] scsi: use shost argument in scsi_eh_prt_fail_stats Hannes Reinecke
@ 2014-10-06  9:01 ` Hannes Reinecke
  2014-10-06  9:01 ` [PATCH 25/26] scsi: check for corrent return code in scsi_eh_abort_cmds() Hannes Reinecke
  2014-10-06  9:01 ` [PATCH 26/26] scsi: correct return values for .eh_abort_handler implementations Hannes Reinecke
  25 siblings, 0 replies; 30+ messages in thread
From: Hannes Reinecke @ 2014-10-06  9:01 UTC (permalink / raw)
  To: James Bottomley
  Cc: Christoph Hellwig, linux-scsi, Robert Elliott, Hannes Reinecke

scsi_try_to_abort_cmd() should only return
SUCCESS, FAILED, or FAST_IO_FAIL. So document
that in the function description and simplify the
logging message.

Suggested-by: Christoph Hellwig <hch@infradead.org>
Reviewed-by: Robert Elliott <elliott@hp.com>
Reviewed-by: Christoph Hellwig <hch@infradead.org>
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/scsi_error.c | 24 +++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index c485f28..9209407 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -157,8 +157,9 @@ 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 %s\n", scmd,
+					    (rtn == FAST_IO_FAIL) ?
+					    "not send" : "failed"));
 		}
 	}
 
@@ -869,7 +870,24 @@ 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, FAILED, or FAST_IO_FAIL
+ *
+ * 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. LLDDs should return FAST_IO_FAIL
+ *    if the device is temporarily unavailable (eg due to a
+ *    link down on FibreChannel)
+ */
+static int scsi_try_to_abort_cmd(struct scsi_host_template *hostt,
+				 struct scsi_cmnd *scmd)
 {
 	if (!hostt->eh_abort_handler)
 		return FAILED;
-- 
1.8.5.2


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 25/26] scsi: check for corrent return code in scsi_eh_abort_cmds()
  2014-10-06  9:01 [PATCHv6 00/26] scsi logging update (the boring part) Hannes Reinecke
                   ` (23 preceding siblings ...)
  2014-10-06  9:01 ` [PATCH 24/26] scsi_error: document scsi_try_to_abort_cmd Hannes Reinecke
@ 2014-10-06  9:01 ` Hannes Reinecke
  2014-10-06  9:01 ` [PATCH 26/26] scsi: correct return values for .eh_abort_handler implementations Hannes Reinecke
  25 siblings, 0 replies; 30+ messages in thread
From: Hannes Reinecke @ 2014-10-06  9:01 UTC (permalink / raw)
  To: James Bottomley
  Cc: Christoph Hellwig, linux-scsi, Robert Elliott, Hannes Reinecke

scsi_try_to_abort_cmd() might return SUCCESS, FAILED, or
FAST_IO_FAIL. So just checking for FAILED will treat
FAST_IO_FAIL as SUCCESS, which is wrong.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/scsi_error.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index 9209407..06cd624 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -1345,7 +1345,7 @@ static int scsi_eh_abort_cmds(struct list_head *work_q,
 			scmd_printk(KERN_INFO, scmd,
 				     "%s: aborting cmd\n", current->comm));
 		rtn = scsi_try_to_abort_cmd(shost->hostt, scmd);
-		if (rtn == FAILED) {
+		if (rtn != SUCCESS) {
 			SCSI_LOG_ERROR_RECOVERY(3,
 				scmd_printk(KERN_INFO, scmd,
 					    "%s: aborting cmd failed\n",
-- 
1.8.5.2


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 26/26] scsi: correct return values for .eh_abort_handler implementations
  2014-10-06  9:01 [PATCHv6 00/26] scsi logging update (the boring part) Hannes Reinecke
                   ` (24 preceding siblings ...)
  2014-10-06  9:01 ` [PATCH 25/26] scsi: check for corrent return code in scsi_eh_abort_cmds() Hannes Reinecke
@ 2014-10-06  9:01 ` Hannes Reinecke
  25 siblings, 0 replies; 30+ messages in thread
From: Hannes Reinecke @ 2014-10-06  9:01 UTC (permalink / raw)
  To: James Bottomley
  Cc: Christoph Hellwig, linux-scsi, Robert Elliott, Hannes Reinecke

The .eh_abort_handler needs to return SUCCESS, FAILED, or
FAST_IO_FAIL. So fixup all callers to adhere to this requirement.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/NCR5380.c            | 2 +-
 drivers/scsi/aha1740.c            | 2 +-
 drivers/scsi/esas2r/esas2r_main.c | 2 +-
 drivers/scsi/megaraid.c           | 8 ++++----
 4 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/NCR5380.c b/drivers/scsi/NCR5380.c
index 50873bb..e2c9e73 100644
--- a/drivers/scsi/NCR5380.c
+++ b/drivers/scsi/NCR5380.c
@@ -2703,7 +2703,7 @@ static int NCR5380_abort(Scsi_Cmnd * cmd) {
  * aborted flag and get back into our main loop.
  */
 
-		return 0;
+		return SUCCESS;
 	}
 #endif
 
diff --git a/drivers/scsi/aha1740.c b/drivers/scsi/aha1740.c
index 5f31017..31ace4b 100644
--- a/drivers/scsi/aha1740.c
+++ b/drivers/scsi/aha1740.c
@@ -531,7 +531,7 @@ static int aha1740_eh_abort_handler (Scsi_Cmnd *dummy)
  * quiet as possible...
  */
 
-	return 0;
+	return SUCCESS;
 }
 
 static struct scsi_host_template aha1740_template = {
diff --git a/drivers/scsi/esas2r/esas2r_main.c b/drivers/scsi/esas2r/esas2r_main.c
index 6504a19..45aa684 100644
--- a/drivers/scsi/esas2r/esas2r_main.c
+++ b/drivers/scsi/esas2r/esas2r_main.c
@@ -1057,7 +1057,7 @@ int esas2r_eh_abort(struct scsi_cmnd *cmd)
 
 		cmd->scsi_done(cmd);
 
-		return 0;
+		return SUCCESS;
 	}
 
 	spin_lock_irqsave(&a->queue_lock, flags);
diff --git a/drivers/scsi/megaraid.c b/drivers/scsi/megaraid.c
index ac5d94c..2485255 100644
--- a/drivers/scsi/megaraid.c
+++ b/drivers/scsi/megaraid.c
@@ -1945,7 +1945,7 @@ megaraid_abort_and_reset(adapter_t *adapter, Scsi_Cmnd *cmd, int aor)
 	     cmd->device->id, (u32)cmd->device->lun);
 
 	if(list_empty(&adapter->pending_list))
-		return FALSE;
+		return FAILED;
 
 	list_for_each_safe(pos, next, &adapter->pending_list) {
 
@@ -1968,7 +1968,7 @@ megaraid_abort_and_reset(adapter_t *adapter, Scsi_Cmnd *cmd, int aor)
 					(aor==SCB_ABORT) ? "ABORTING":"RESET",
 					scb->idx);
 
-				return FALSE;
+				return FAILED;
 			}
 			else {
 
@@ -1993,12 +1993,12 @@ megaraid_abort_and_reset(adapter_t *adapter, Scsi_Cmnd *cmd, int aor)
 				list_add_tail(SCSI_LIST(cmd),
 						&adapter->completed_list);
 
-				return TRUE;
+				return SUCCESS;
 			}
 		}
 	}
 
-	return FALSE;
+	return FAILED;
 }
 
 static inline int
-- 
1.8.5.2


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [PATCH 21/26] scsi: simplify scsi_log_(send|completion)
  2014-10-06  9:01 ` [PATCH 21/26] scsi: simplify scsi_log_(send|completion) Hannes Reinecke
@ 2014-10-06 19:53   ` Ewan Milne
  2014-10-07  8:45     ` Hannes Reinecke
  0 siblings, 1 reply; 30+ messages in thread
From: Ewan Milne @ 2014-10-06 19:53 UTC (permalink / raw)
  To: Hannes Reinecke
  Cc: James Bottomley, Christoph Hellwig, linux-scsi, Robert Elliott

Rats.  I found another case of "(null)" appearing in the log messages
where I think you really just wanted no text at all to appear.
Sorry I didn't find it last time along with the other one...

-Ewan

On Mon, 2014-10-06 at 11:01 +0200, Hannes Reinecke wrote:
> Simplify scsi_log_(send|completion) by externalizing
> scsi_mlreturn_string() and always print the command address.
> 
> Reviewed-by: Robert Elliott <elliott@hp.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>  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(-)
> 
> diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
> index 1ebdc5d..59b7f9e 100644
> --- a/drivers/scsi/constants.c
> +++ b/drivers/scsi/constants.c
> @@ -1439,19 +1439,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);
> diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
> index 61aeaf1..3d81a07 100644
> --- a/drivers/scsi/scsi.c
> +++ b/drivers/scsi/scsi.c
> @@ -527,9 +527,9 @@ void scsi_log_send(struct scsi_cmnd *cmd)
>  	 *
>  	 * 1: nothing (match completion)
>  	 *
> -	 * 2: log opcode + command of all commands
> +	 * 2: log opcode + command of all commands + cmd address
>  	 *
> -	 * 3: same as 2 plus dump cmd address
> +	 * 3: same as 2
>  	 *
>  	 * 4: same as 3 plus dump extra junk
>  	 */
> @@ -537,10 +537,8 @@ void scsi_log_send(struct scsi_cmnd *cmd)
>  		level = SCSI_LOG_LEVEL(SCSI_LOG_MLQUEUE_SHIFT,
>  				       SCSI_LOG_MLQUEUE_BITS);
>  		if (level > 1) {
> -			scmd_printk(KERN_INFO, cmd, "Send: ");
> -			if (level > 2)
> -				printk("0x%p ", cmd);
> -			printk("\n");
> +			scmd_printk(KERN_INFO, cmd,
> +				    "Send: scmd 0x%p\n", cmd);
>  			scsi_print_command(cmd);
>  			if (level > 3) {
>  				printk(KERN_INFO "buffer = 0x%p, bufflen = %d,"
> @@ -565,7 +563,7 @@ void scsi_log_completion(struct scsi_cmnd *cmd, int disposition)
>  	 *
>  	 * 2: same as 1 but for all command completions.
>  	 *
> -	 * 3: same as 2 plus dump cmd address
> +	 * 3: same as 2
>  	 *
>  	 * 4: same as 3 plus dump extra junk
>  	 */
> @@ -574,36 +572,7 @@ void scsi_log_completion(struct scsi_cmnd *cmd, int disposition)
>  				       SCSI_LOG_MLCOMPLETE_BITS);
>  		if (((level > 0) && (cmd->result || disposition != SUCCESS)) ||
>  		    (level > 1)) {
> -			scmd_printk(KERN_INFO, cmd, "Done: ");
> -			if (level > 2)
> -				printk("0x%p ", cmd);
> -			/*
> -			 * Dump truncated values, so we usually fit within
> -			 * 80 chars.
> -			 */
> -			switch (disposition) {
> -			case SUCCESS:
> -				printk("SUCCESS\n");
> -				break;
> -			case NEEDS_RETRY:
> -				printk("RETRY\n");
> -				break;
> -			case ADD_TO_MLQUEUE:
> -				printk("MLQUEUE\n");
> -				break;
> -			case FAILED:
> -				printk("FAILED\n");
> -				break;
> -			case TIMEOUT_ERROR:
> -				/* 
> -				 * If called via scsi_times_out.
> -				 */
> -				printk("TIMEOUT\n");
> -				break;
> -			default:
> -				printk("UNKNOWN\n");
> -			}
> -			scsi_print_result(cmd);
> +			scsi_print_result(cmd, "Done: ", disposition);
>  			scsi_print_command(cmd);
>  			if (status_byte(cmd->result) & CHECK_CONDITION)
>  				scsi_print_sense(cmd);
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index b853659..2df485f 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -831,7 +831,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;
> @@ -1037,8 +1037,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);

                                                 ^^^^--

I think this is what is causing other undesirable log messages:

Oct  6 15:30:51 rhel-storage-01 kernel: [ 1257.681129] sd 9:0:0:0: [sde] (null)FAILED Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE

                                                                         ^^^^^^--- here

because scsi_print_result() is using "%s%s" and the "msg" argument is NULL

                scmd_printk(KERN_INFO, cmd,
                            "%s%s Result: hostbyte=%s driverbyte=%s",
                            msg, mlret_string ? mlret_string : "UNKNOWN",
                            hb_string ? hb_string : "invalid",
                            db_string ? db_string : "invalid");


>  			if (driver_byte(result) & DRIVER_SENSE)
>  				scsi_print_sense(cmd);
>  			scsi_print_command(cmd);
> diff --git a/include/scsi/scsi_dbg.h b/include/scsi/scsi_dbg.h
> index f41a86b..7982795 100644
> --- a/include/scsi/scsi_dbg.h
> +++ b/include/scsi/scsi_dbg.h
> @@ -17,9 +17,10 @@ extern void scsi_print_sense(const struct scsi_cmnd *);
>  extern void __scsi_print_sense(const 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 **);



^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 21/26] scsi: simplify scsi_log_(send|completion)
  2014-10-06 19:53   ` Ewan Milne
@ 2014-10-07  8:45     ` Hannes Reinecke
  0 siblings, 0 replies; 30+ messages in thread
From: Hannes Reinecke @ 2014-10-07  8:45 UTC (permalink / raw)
  To: emilne; +Cc: James Bottomley, Christoph Hellwig, linux-scsi, Robert Elliott

On 10/06/2014 09:53 PM, Ewan Milne wrote:
> Rats.  I found another case of "(null)" appearing in the log messages
> where I think you really just wanted no text at all to appear.
> Sorry I didn't find it last time along with the other one...
> 
Nae bother. I'll be sending a new version of the patchset.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 10/26] scsi: stop decoding if scsi_normalize_sense() fails
  2014-10-07  9:02 [PATCHv7 00/26] scsi logging update (the boring part) Hannes Reinecke
@ 2014-10-07  9:03 ` Hannes Reinecke
  0 siblings, 0 replies; 30+ messages in thread
From: Hannes Reinecke @ 2014-10-07  9:03 UTC (permalink / raw)
  To: James Bottomley
  Cc: Christoph Hellwig, linux-scsi, Robert Elliott, Hannes Reinecke

If 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: Robert Elliott <elliott@hp.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/constants.c | 34 ++++++++++++++++------------------
 1 file changed, 16 insertions(+), 18 deletions(-)

diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
index f9e2317..0b74e94 100644
--- a/drivers/scsi/constants.c
+++ b/drivers/scsi/constants.c
@@ -1435,26 +1435,21 @@ scsi_print_sense_hdr(const 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(const 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);
-- 
1.8.5.2


^ permalink raw reply related	[flat|nested] 30+ messages in thread

end of thread, other threads:[~2014-10-07  9:03 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-06  9:01 [PATCHv6 00/26] scsi logging update (the boring part) Hannes Reinecke
2014-10-06  9:01 ` [PATCH 01/26] Remove scsi_cmd_print_sense_hdr() Hannes Reinecke
2014-10-06  9:01 ` [PATCH 02/26] sd: Remove scsi_print_sense() in sd_done() Hannes Reinecke
2014-10-06  9:01 ` [PATCH 03/26] aha152x: Debug output update and whitespace cleanup Hannes Reinecke
2014-10-06  9:01 ` [PATCH 04/26] scsi: introduce sdev_prefix_printk() Hannes Reinecke
2014-10-06  9:01 ` [PATCH 05/26] scsi: Use sdev as argument for sense code printing Hannes Reinecke
2014-10-06  9:01 ` [PATCH 06/26] acornscsi: use scsi_print_command() Hannes Reinecke
2014-10-06  9:01 ` [PATCH 07/26] fas216: Return DID_ERROR for incomplete data transfer Hannes Reinecke
2014-10-06  9:01 ` [PATCH 08/26] fas216: Update logging messages Hannes Reinecke
2014-10-06  9:01 ` [PATCH 09/26] 53c700: remove scsi_print_sense() usage Hannes Reinecke
2014-10-06  9:01 ` [PATCH 10/26] scsi: stop decoding if scsi_normalize_sense() fails Hannes Reinecke
2014-10-06  9:01 ` [PATCH 11/26] scsi: do not decode sense extras Hannes Reinecke
2014-10-06  9:01 ` [PATCH 12/26] scsi: use 'bool' as return value for scsi_normalize_sense() Hannes Reinecke
2014-10-06  9:01 ` [PATCH 13/26] scsi: remove scsi_print_status() Hannes Reinecke
2014-10-06  9:01 ` [PATCH 14/26] Implement scsi_opcode_sa_name Hannes Reinecke
2014-10-06  9:01 ` [PATCH 15/26] scsi: merge print_opcode_name() Hannes Reinecke
2014-10-06  9:01 ` [PATCH 16/26] scsi: consolidate opcode lookup in scsi_opcode_sa_name() Hannes Reinecke
2014-10-06  9:01 ` [PATCH 17/26] scsi: repurpose the last argument from print_opcode_name() Hannes Reinecke
2014-10-06  9:01 ` [PATCH 18/26] scsi: Remove scsi_print_command when calling abort Hannes Reinecke
2014-10-06  9:01 ` [PATCH 19/26] scsi: separate out scsi_(host|driver)byte_string() Hannes Reinecke
2014-10-06  9:01 ` [PATCH 20/26] sd: Cleanup logging Hannes Reinecke
2014-10-06  9:01 ` [PATCH 21/26] scsi: simplify scsi_log_(send|completion) Hannes Reinecke
2014-10-06 19:53   ` Ewan Milne
2014-10-07  8:45     ` Hannes Reinecke
2014-10-06  9:01 ` [PATCH 22/26] scsi: fixup logging messages in scsi_error.c Hannes Reinecke
2014-10-06  9:01 ` [PATCH 23/26] scsi: use shost argument in scsi_eh_prt_fail_stats Hannes Reinecke
2014-10-06  9:01 ` [PATCH 24/26] scsi_error: document scsi_try_to_abort_cmd Hannes Reinecke
2014-10-06  9:01 ` [PATCH 25/26] scsi: check for corrent return code in scsi_eh_abort_cmds() Hannes Reinecke
2014-10-06  9:01 ` [PATCH 26/26] scsi: correct return values for .eh_abort_handler implementations Hannes Reinecke
2014-10-07  9:02 [PATCHv7 00/26] scsi logging update (the boring part) Hannes Reinecke
2014-10-07  9:03 ` [PATCH 10/26] scsi: stop decoding if scsi_normalize_sense() fails Hannes Reinecke

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.