[ PATCH -logging 0/3] scsi/trace: Delete duplicated decoders

[ PATCH -logging 0/3] scsi/trace: Delete duplicated decoders

[Yoshihiro YUNOMAE]

Hi All,

This patchset deletes duplicated decoders in scsi_trace.c.
Almost decoders are implemented in constants.c, so we should use them.

This patchset deletes following decoders in scsi_trace.c:
 - hostbyte in patch [2/3]
 - driverbyte in patch [2/3]
 - SCSI command in patch [3/3]

This patchset is based on Hannes' logging branch:
http://git.kernel.org/cgit/linux/kernel/git/hare/scsi-devel.git/log/?h=logging

Thanks,
Yoshihiro YUNOMAE

---

Yoshihiro YUNOMAE (3):
      scsi/trace: Use macros for getting driver byte, host byte, msg byte, and status byte
      scsi/trace: Delete duplicated decoders of hostbyte and driverbyte
      scsi/trace: Delete a duplicated decoder of SCSI command


 drivers/scsi/constants.c    |   34 +++++---
 include/scsi/scsi.h         |    6 +
 include/trace/events/scsi.h |  179 ++++++++++---------------------------------
 3 files changed, 68 insertions(+), 151 deletions(-)

-- 
Yoshihiro YUNOMAE
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: yoshihiro.yunomae.ez@hitachi.com

[PATCH 1/3] scsi/trace: Use macros for getting driver byte, host byte, msg byte, and status byte

[Yoshihiro YUNOMAE]

For getting driver byte, host byte, msg byte, and status byte, macros are
implemented in scsi/scsi.h, so we use it.

Signed-off-by: Yoshihiro YUNOMAE <yoshihiro.yunomae.ez@hitachi.com>
Reviewed-by: Ewan D. Milne <emilne@redhat.com>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Cc: Hannes Reinecke <hare@suse.de>
Cc: Doug Gilbert <dgilbert@interlog.com>
Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
Cc: Robert Elliott <Elliott@hp.com>
Cc: Ewan D. Milne <emilne@redhat.com>
Cc: Hidehiro Kawai <hidehiro.kawai.ez@hitachi.com>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
 include/trace/events/scsi.h |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/trace/events/scsi.h b/include/trace/events/scsi.h
index db6c935..8aecdc2 100644
--- a/include/trace/events/scsi.h
+++ b/include/trace/events/scsi.h
@@ -328,10 +328,10 @@ DECLARE_EVENT_CLASS(scsi_cmd_done_timeout_template,
 		  show_opcode_name(__entry->opcode),
 		  __parse_cdb(__get_dynamic_array(cmnd), __entry->cmd_len),
 		  __print_hex(__get_dynamic_array(cmnd), __entry->cmd_len),
-		  show_driverbyte_name(((__entry->result) >> 24) & 0xff),
-		  show_hostbyte_name(((__entry->result) >> 16) & 0xff),
-		  show_msgbyte_name(((__entry->result) >> 8) & 0xff),
-		  show_statusbyte_name(__entry->result & 0xff))
+		  show_driverbyte_name(driver_byte(__entry->result)),
+		  show_hostbyte_name(host_byte(__entry->result)),
+		  show_msgbyte_name(msg_byte(__entry->result)),
+		  show_statusbyte_name(status_byte(__entry->result)))
 );
 
 DEFINE_EVENT(scsi_cmd_done_timeout_template, scsi_dispatch_cmd_done,

[PATCH 2/3] scsi/trace: Delete duplicated decoders of hostbyte and driverbyte

[Yoshihiro YUNOMAE]

There are decoders of hostbyte and driverbyte in constants.c, so this patch
deletes those in SCSI traveevents.

Note:
If CONFIG_SCSI_CONSTANTS is disabled, hostbyte, driverbyte, msgbyte, and
statusbyte are output as raw format from this patch.

Signed-off-by: Yoshihiro YUNOMAE <yoshihiro.yunomae.ez@hitachi.com>
Cc: Hannes Reinecke <hare@suse.de>
Cc: Doug Gilbert <dgilbert@interlog.com>
Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
Cc: Robert Elliott <Elliott@hp.com>
Cc: Ewan D. Milne <emilne@redhat.com>
Cc: Hidehiro Kawai <hidehiro.kawai.ez@hitachi.com>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
 drivers/scsi/constants.c    |   21 ++++++++++---------
 include/scsi/scsi.h         |    5 +++++
 include/trace/events/scsi.h |   47 +++++++++++++------------------------------
 3 files changed, 30 insertions(+), 43 deletions(-)

diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
index c6a7a4a..8c1f7ac 100644
--- a/drivers/scsi/constants.c
+++ b/drivers/scsi/constants.c
@@ -1505,21 +1505,22 @@ static const char * const driverbyte_table[]={
 "DRIVER_INVALID", "DRIVER_TIMEOUT", "DRIVER_HARD", "DRIVER_SENSE"};
 #define NUM_DRIVERBYTE_STRS ARRAY_SIZE(driverbyte_table)
 
-void scsi_show_result(struct scsi_device *sdev, const char *name, int result)
+inline const char *show_hostbyte_name(int hb)
 {
-	int hb = host_byte(result);
-	int db = driver_byte(result);
-	const char *hb_string;
-	const char *db_string;
-
-	hb_string = (hb < NUM_HOSTBYTE_STRS) ? hostbyte_table[hb] : "invalid";
-	db_string = (db < NUM_DRIVERBYTE_STRS) ?
-		driverbyte_table[db] : "invalid";
+	return (hb < NUM_HOSTBYTE_STRS) ? hostbyte_table[hb] : "invalid";
+}
 
+inline const char *show_driverbyte_name(int db)
+{
+	return (db < NUM_DRIVERBYTE_STRS) ? driverbyte_table[db] : "invalid";
+}
 
+void scsi_show_result(struct scsi_device *sdev, const char *name, int result)
+{
 	sdev_printk(KERN_INFO, sdev,
 		    "[%s] Result: hostbyte=%s driverbyte=%s\n",
-		    name, hb_string, db_string);
+		    name, show_hostbyte_name(host_byte(result)),
+		    show_driverbyte_name(driver_byte(result)));
 }
 
 #else
diff --git a/include/scsi/scsi.h b/include/scsi/scsi.h
index 54ebf54..6d6b3ef 100644
--- a/include/scsi/scsi.h
+++ b/include/scsi/scsi.h
@@ -586,4 +586,9 @@ static inline __u32 scsi_to_u32(__u8 *ptr)
 	return (ptr[0]<<24) + (ptr[1]<<16) + (ptr[2]<<8) + ptr[3];
 }
 
+#ifdef CONFIG_SCSI_CONSTANTS
+inline const char *show_hostbyte_name(int hb);
+inline const char *show_driverbyte_name(int db);
+#endif
+
 #endif /* _SCSI_SCSI_H */
diff --git a/include/trace/events/scsi.h b/include/trace/events/scsi.h
index 8aecdc2..67be592 100644
--- a/include/trace/events/scsi.h
+++ b/include/trace/events/scsi.h
@@ -105,39 +105,6 @@
 		scsi_opcode_name(ATA_16),			\
 		scsi_opcode_name(ATA_12))
 
-#define scsi_hostbyte_name(result)	{ result, #result }
-#define show_hostbyte_name(val)					\
-	__print_symbolic(val,					\
-		scsi_hostbyte_name(DID_OK),			\
-		scsi_hostbyte_name(DID_NO_CONNECT),		\
-		scsi_hostbyte_name(DID_BUS_BUSY),		\
-		scsi_hostbyte_name(DID_TIME_OUT),		\
-		scsi_hostbyte_name(DID_BAD_TARGET),		\
-		scsi_hostbyte_name(DID_ABORT),			\
-		scsi_hostbyte_name(DID_PARITY),			\
-		scsi_hostbyte_name(DID_ERROR),			\
-		scsi_hostbyte_name(DID_RESET),			\
-		scsi_hostbyte_name(DID_BAD_INTR),		\
-		scsi_hostbyte_name(DID_PASSTHROUGH),		\
-		scsi_hostbyte_name(DID_SOFT_ERROR),		\
-		scsi_hostbyte_name(DID_IMM_RETRY),		\
-		scsi_hostbyte_name(DID_REQUEUE),		\
-		scsi_hostbyte_name(DID_TRANSPORT_DISRUPTED),	\
-		scsi_hostbyte_name(DID_TRANSPORT_FAILFAST))
-
-#define scsi_driverbyte_name(result)	{ result, #result }
-#define show_driverbyte_name(val)				\
-	__print_symbolic(val,					\
-		scsi_driverbyte_name(DRIVER_OK),		\
-		scsi_driverbyte_name(DRIVER_BUSY),		\
-		scsi_driverbyte_name(DRIVER_SOFT),		\
-		scsi_driverbyte_name(DRIVER_MEDIA),		\
-		scsi_driverbyte_name(DRIVER_ERROR),		\
-		scsi_driverbyte_name(DRIVER_INVALID),		\
-		scsi_driverbyte_name(DRIVER_TIMEOUT),		\
-		scsi_driverbyte_name(DRIVER_HARD),		\
-		scsi_driverbyte_name(DRIVER_SENSE))
-
 #define scsi_msgbyte_name(result)	{ result, #result }
 #define show_msgbyte_name(val)					\
 	__print_symbolic(val,					\
@@ -319,6 +286,7 @@ DECLARE_EVENT_CLASS(scsi_cmd_done_timeout_template,
 		memcpy(__get_dynamic_array(cmnd), cmd->cmnd, cmd->cmd_len);
 	),
 
+#ifdef CONFIG_SCSI_CONSTANTS
 	TP_printk("host_no=%u channel=%u id=%u lun=%u data_sgl=%u " \
 		  "prot_sgl=%u prot_op=%s cmnd=(%s %s raw=%s) result=(driver=" \
 		  "%s host=%s message=%s status=%s)",
@@ -332,6 +300,19 @@ DECLARE_EVENT_CLASS(scsi_cmd_done_timeout_template,
 		  show_hostbyte_name(host_byte(__entry->result)),
 		  show_msgbyte_name(msg_byte(__entry->result)),
 		  show_statusbyte_name(status_byte(__entry->result)))
+#else
+	TP_printk("host_no=%u channel=%u id=%u lun=%u data_sgl=%u " \
+		  "prot_sgl=%u prot_op=%s cmnd=(%s %s raw=%s) result=(driver=" \
+		  "0x%02x host=0x%02x message=0x%02x status=0x%02x)",
+		  __entry->host_no, __entry->channel, __entry->id,
+		  __entry->lun, __entry->data_sglen, __entry->prot_sglen,
+		  show_prot_op_name(__entry->prot_op),
+		  show_opcode_name(__entry->opcode),
+		  __parse_cdb(__get_dynamic_array(cmnd), __entry->cmd_len),
+		  __print_hex(__get_dynamic_array(cmnd), __entry->cmd_len),
+		  driver_byte(__entry->result), host_byte(__entry->result),
+		  msg_byte(__entry->result), status_byte(__entry->result))
+#endif
 );
 
 DEFINE_EVENT(scsi_cmd_done_timeout_template, scsi_dispatch_cmd_done,

[PATCH 3/3] scsi/trace: Delete a duplicated decoder of SCSI command

[Yoshihiro YUNOMAE]

There is a decoder of SCSI command in constants.c, so this patch deletes it in
SCSI traveevents.

A decoder in traceevents uses macros, so the command name is output as
"XXX_YYY_ZZZ". On the other hand, a decoder in constants uses strings, so
the command name is output as "Xxx Yyy Zzz" including space. We use a decoder
in constants, so this patch adds double quotes for the name.

Note:
- If CONFIG_SCSI_CONSTANTS is disabled, command names are not shown.
- Add command names for 0x07 and 0x2b.

Signed-off-by: Yoshihiro YUNOMAE <yoshihiro.yunomae.ez@hitachi.com>
Cc: Hannes Reinecke <hare@suse.de>
Cc: Doug Gilbert <dgilbert@interlog.com>
Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
Cc: Robert Elliott <Elliott@hp.com>
Cc: Ewan D. Milne <emilne@redhat.com>
Cc: Hidehiro Kawai <hidehiro.kawai.ez@hitachi.com>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
 drivers/scsi/constants.c    |   13 +++-
 include/scsi/scsi.h         |    1 
 include/trace/events/scsi.h |  130 +++++++++----------------------------------
 3 files changed, 37 insertions(+), 107 deletions(-)

diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
index 8c1f7ac..50e08c8 100644
--- a/drivers/scsi/constants.c
+++ b/drivers/scsi/constants.c
@@ -36,7 +36,7 @@
 static const char * cdb_byte0_names[] = {
 /* 00-03 */ "Test Unit Ready", "Rezero Unit/Rewind", NULL, "Request Sense",
 /* 04-07 */ "Format Unit/Medium", "Read Block Limits", NULL,
-	    "Reassign Blocks",
+	    "Reassign Blocks/Initialize Element Status",
 /* 08-0d */ "Read(6)", NULL, "Write(6)", "Seek(6)", NULL, NULL,
 /* 0e-12 */ NULL, "Read Reverse", "Write Filemarks", "Space", "Inquiry",
 /* 13-16 */ "Verify(6)", "Recover Buffered Data", "Mode Select(6)",
@@ -47,8 +47,8 @@ static const char * cdb_byte0_names[] = {
 /* 20-22 */  NULL, NULL, NULL,
 /* 23-28 */ "Read Format Capacities", "Set Window",
 	    "Read Capacity(10)", NULL, NULL, "Read(10)",
-/* 29-2d */ "Read Generation", "Write(10)", "Seek(10)", "Erase(10)",
-            "Read updated block",
+/* 29-2d */ "Read Generation", "Write(10)", "Seek(10)/Position To Element",
+	    "Erase(10)", "Read updated block",
 /* 2e-31 */ "Write Verify(10)", "Verify(10)", "Search High", "Search Equal",
 /* 32-34 */ "Search Low", "Set Limits", "Prefetch/Read Position",
 /* 35-37 */ "Synchronize Cache(10)", "Lock/Unlock Cache(10)",
@@ -103,6 +103,11 @@ static const char * cdb_byte0_names[] = {
             "Volume set (out), Send DVD structure",
 };
 
+inline const char *show_opcode_name(int cdb0)
+{
+	return cdb_byte0_names[cdb0];
+}
+
 struct value_name_pair {
 	int value;
 	const char * name;
@@ -365,7 +370,7 @@ static void print_opcode_name(struct scsi_device *sdev, const char *prefix,
 	if (cdb0 < 0xc0) {
 		cdb_classifier = NULL;
 #ifdef CONFIG_SCSI_CONSTANTS
-		cdb_name = cdb_byte0_names[cdb0];
+		cdb_name = show_opcode_name(cdb0);
 		if (!cdb_name)
 			cdb_classifier = " (reserved)";
 #endif
diff --git a/include/scsi/scsi.h b/include/scsi/scsi.h
index 6d6b3ef..59fedff 100644
--- a/include/scsi/scsi.h
+++ b/include/scsi/scsi.h
@@ -587,6 +587,7 @@ static inline __u32 scsi_to_u32(__u8 *ptr)
 }
 
 #ifdef CONFIG_SCSI_CONSTANTS
+inline const char *show_opcode_name(int cdb0);
 inline const char *show_hostbyte_name(int hb);
 inline const char *show_driverbyte_name(int db);
 #endif
diff --git a/include/trace/events/scsi.h b/include/trace/events/scsi.h
index 67be592..5695ad7 100644
--- a/include/trace/events/scsi.h
+++ b/include/trace/events/scsi.h
@@ -9,102 +9,6 @@
 #include <linux/tracepoint.h>
 #include <linux/trace_seq.h>
 
-#define scsi_opcode_name(opcode)	{ opcode, #opcode }
-#define show_opcode_name(val)					\
-	__print_symbolic(val,					\
-		scsi_opcode_name(TEST_UNIT_READY),		\
-		scsi_opcode_name(REZERO_UNIT),			\
-		scsi_opcode_name(REQUEST_SENSE),		\
-		scsi_opcode_name(FORMAT_UNIT),			\
-		scsi_opcode_name(READ_BLOCK_LIMITS),		\
-		scsi_opcode_name(REASSIGN_BLOCKS),		\
-		scsi_opcode_name(INITIALIZE_ELEMENT_STATUS),	\
-		scsi_opcode_name(READ_6),			\
-		scsi_opcode_name(WRITE_6),			\
-		scsi_opcode_name(SEEK_6),			\
-		scsi_opcode_name(READ_REVERSE),			\
-		scsi_opcode_name(WRITE_FILEMARKS),		\
-		scsi_opcode_name(SPACE),			\
-		scsi_opcode_name(INQUIRY),			\
-		scsi_opcode_name(RECOVER_BUFFERED_DATA),	\
-		scsi_opcode_name(MODE_SELECT),			\
-		scsi_opcode_name(RESERVE),			\
-		scsi_opcode_name(RELEASE),			\
-		scsi_opcode_name(COPY),				\
-		scsi_opcode_name(ERASE),			\
-		scsi_opcode_name(MODE_SENSE),			\
-		scsi_opcode_name(START_STOP),			\
-		scsi_opcode_name(RECEIVE_DIAGNOSTIC),		\
-		scsi_opcode_name(SEND_DIAGNOSTIC),		\
-		scsi_opcode_name(ALLOW_MEDIUM_REMOVAL),		\
-		scsi_opcode_name(SET_WINDOW),			\
-		scsi_opcode_name(READ_CAPACITY),		\
-		scsi_opcode_name(READ_10),			\
-		scsi_opcode_name(WRITE_10),			\
-		scsi_opcode_name(SEEK_10),			\
-		scsi_opcode_name(POSITION_TO_ELEMENT),		\
-		scsi_opcode_name(WRITE_VERIFY),			\
-		scsi_opcode_name(VERIFY),			\
-		scsi_opcode_name(SEARCH_HIGH),			\
-		scsi_opcode_name(SEARCH_EQUAL),			\
-		scsi_opcode_name(SEARCH_LOW),			\
-		scsi_opcode_name(SET_LIMITS),			\
-		scsi_opcode_name(PRE_FETCH),			\
-		scsi_opcode_name(READ_POSITION),		\
-		scsi_opcode_name(SYNCHRONIZE_CACHE),		\
-		scsi_opcode_name(LOCK_UNLOCK_CACHE),		\
-		scsi_opcode_name(READ_DEFECT_DATA),		\
-		scsi_opcode_name(MEDIUM_SCAN),			\
-		scsi_opcode_name(COMPARE),			\
-		scsi_opcode_name(COPY_VERIFY),			\
-		scsi_opcode_name(WRITE_BUFFER),			\
-		scsi_opcode_name(READ_BUFFER),			\
-		scsi_opcode_name(UPDATE_BLOCK),			\
-		scsi_opcode_name(READ_LONG),			\
-		scsi_opcode_name(WRITE_LONG),			\
-		scsi_opcode_name(CHANGE_DEFINITION),		\
-		scsi_opcode_name(WRITE_SAME),			\
-		scsi_opcode_name(UNMAP),			\
-		scsi_opcode_name(READ_TOC),			\
-		scsi_opcode_name(LOG_SELECT),			\
-		scsi_opcode_name(LOG_SENSE),			\
-		scsi_opcode_name(XDWRITEREAD_10),		\
-		scsi_opcode_name(MODE_SELECT_10),		\
-		scsi_opcode_name(RESERVE_10),			\
-		scsi_opcode_name(RELEASE_10),			\
-		scsi_opcode_name(MODE_SENSE_10),		\
-		scsi_opcode_name(PERSISTENT_RESERVE_IN),	\
-		scsi_opcode_name(PERSISTENT_RESERVE_OUT),	\
-		scsi_opcode_name(VARIABLE_LENGTH_CMD),		\
-		scsi_opcode_name(REPORT_LUNS),			\
-		scsi_opcode_name(MAINTENANCE_IN),		\
-		scsi_opcode_name(MAINTENANCE_OUT),		\
-		scsi_opcode_name(MOVE_MEDIUM),			\
-		scsi_opcode_name(EXCHANGE_MEDIUM),		\
-		scsi_opcode_name(READ_12),			\
-		scsi_opcode_name(WRITE_12),			\
-		scsi_opcode_name(WRITE_VERIFY_12),		\
-		scsi_opcode_name(SEARCH_HIGH_12),		\
-		scsi_opcode_name(SEARCH_EQUAL_12),		\
-		scsi_opcode_name(SEARCH_LOW_12),		\
-		scsi_opcode_name(READ_ELEMENT_STATUS),		\
-		scsi_opcode_name(SEND_VOLUME_TAG),		\
-		scsi_opcode_name(WRITE_LONG_2),			\
-		scsi_opcode_name(READ_16),			\
-		scsi_opcode_name(WRITE_16),			\
-		scsi_opcode_name(VERIFY_16),			\
-		scsi_opcode_name(WRITE_SAME_16),		\
-		scsi_opcode_name(SERVICE_ACTION_IN),		\
-		scsi_opcode_name(SAI_READ_CAPACITY_16),		\
-		scsi_opcode_name(SAI_GET_LBA_STATUS),		\
-		scsi_opcode_name(MI_REPORT_TARGET_PGS),		\
-		scsi_opcode_name(MO_SET_TARGET_PGS),		\
-		scsi_opcode_name(READ_32),			\
-		scsi_opcode_name(WRITE_32),			\
-		scsi_opcode_name(WRITE_SAME_32),		\
-		scsi_opcode_name(ATA_16),			\
-		scsi_opcode_name(ATA_12))
-
 #define scsi_msgbyte_name(result)	{ result, #result }
 #define show_msgbyte_name(val)					\
 	__print_symbolic(val,					\
@@ -197,14 +101,24 @@ TRACE_EVENT(scsi_dispatch_cmd_start,
 		memcpy(__get_dynamic_array(cmnd), cmd->cmnd, cmd->cmd_len);
 	),
 
+#ifdef CONFIG_SCSI_CONSTANTS
 	TP_printk("host_no=%u channel=%u id=%u lun=%u data_sgl=%u prot_sgl=%u" \
-		  " prot_op=%s cmnd=(%s %s raw=%s)",
+		  " prot_op=%s cmnd=(\"%s\" %s raw=%s)",
 		  __entry->host_no, __entry->channel, __entry->id,
 		  __entry->lun, __entry->data_sglen, __entry->prot_sglen,
 		  show_prot_op_name(__entry->prot_op),
 		  show_opcode_name(__entry->opcode),
 		  __parse_cdb(__get_dynamic_array(cmnd), __entry->cmd_len),
 		  __print_hex(__get_dynamic_array(cmnd), __entry->cmd_len))
+#else
+	TP_printk("host_no=%u channel=%u id=%u lun=%u data_sgl=%u prot_sgl=%u" \
+		  " prot_op=%s cmnd=(%s raw=%s)",
+		  __entry->host_no, __entry->channel, __entry->id,
+		  __entry->lun, __entry->data_sglen, __entry->prot_sglen,
+		  show_prot_op_name(__entry->prot_op),
+		  __parse_cdb(__get_dynamic_array(cmnd), __entry->cmd_len),
+		  __print_hex(__get_dynamic_array(cmnd), __entry->cmd_len))
+#endif
 );
 
 TRACE_EVENT(scsi_dispatch_cmd_error,
@@ -241,8 +155,9 @@ TRACE_EVENT(scsi_dispatch_cmd_error,
 		memcpy(__get_dynamic_array(cmnd), cmd->cmnd, cmd->cmd_len);
 	),
 
+#ifdef CONFIG_SCSI_CONSTANTS
 	TP_printk("host_no=%u channel=%u id=%u lun=%u data_sgl=%u prot_sgl=%u" \
-		  " prot_op=%s cmnd=(%s %s raw=%s) rtn=%d",
+		  " prot_op=%s cmnd=(\"%s\" %s raw=%s) rtn=%d",
 		  __entry->host_no, __entry->channel, __entry->id,
 		  __entry->lun, __entry->data_sglen, __entry->prot_sglen,
 		  show_prot_op_name(__entry->prot_op),
@@ -250,6 +165,16 @@ TRACE_EVENT(scsi_dispatch_cmd_error,
 		  __parse_cdb(__get_dynamic_array(cmnd), __entry->cmd_len),
 		  __print_hex(__get_dynamic_array(cmnd), __entry->cmd_len),
 		  __entry->rtn)
+#else
+	TP_printk("host_no=%u channel=%u id=%u lun=%u data_sgl=%u prot_sgl=%u" \
+		  " prot_op=%s cmnd=(%s raw=%s) rtn=%d",
+		  __entry->host_no, __entry->channel, __entry->id,
+		  __entry->lun, __entry->data_sglen, __entry->prot_sglen,
+		  show_prot_op_name(__entry->prot_op),
+		  __parse_cdb(__get_dynamic_array(cmnd), __entry->cmd_len),
+		  __print_hex(__get_dynamic_array(cmnd), __entry->cmd_len),
+		  __entry->rtn)
+#endif
 );
 
 DECLARE_EVENT_CLASS(scsi_cmd_done_timeout_template,
@@ -288,8 +213,8 @@ DECLARE_EVENT_CLASS(scsi_cmd_done_timeout_template,
 
 #ifdef CONFIG_SCSI_CONSTANTS
 	TP_printk("host_no=%u channel=%u id=%u lun=%u data_sgl=%u " \
-		  "prot_sgl=%u prot_op=%s cmnd=(%s %s raw=%s) result=(driver=" \
-		  "%s host=%s message=%s status=%s)",
+		  "prot_sgl=%u prot_op=%s cmnd=(\"%s\" %s raw=%s) " \
+		  "result=(driver=%s host=%s message=%s status=%s)",
 		  __entry->host_no, __entry->channel, __entry->id,
 		  __entry->lun, __entry->data_sglen, __entry->prot_sglen,
 		  show_prot_op_name(__entry->prot_op),
@@ -302,12 +227,11 @@ DECLARE_EVENT_CLASS(scsi_cmd_done_timeout_template,
 		  show_statusbyte_name(status_byte(__entry->result)))
 #else
 	TP_printk("host_no=%u channel=%u id=%u lun=%u data_sgl=%u " \
-		  "prot_sgl=%u prot_op=%s cmnd=(%s %s raw=%s) result=(driver=" \
-		  "0x%02x host=0x%02x message=0x%02x status=0x%02x)",
+		  "prot_sgl=%u prot_op=%s cmnd=(%s raw=%s) result=" \
+		  "(driver=0x%02x host=0x%02x message=0x%02x status=0x%02x)",
 		  __entry->host_no, __entry->channel, __entry->id,
 		  __entry->lun, __entry->data_sglen, __entry->prot_sglen,
 		  show_prot_op_name(__entry->prot_op),
-		  show_opcode_name(__entry->opcode),
 		  __parse_cdb(__get_dynamic_array(cmnd), __entry->cmd_len),
 		  __print_hex(__get_dynamic_array(cmnd), __entry->cmd_len),
 		  driver_byte(__entry->result), host_byte(__entry->result),

Re: [PATCH 1/3] scsi/trace: Use macros for getting driver byte, host byte, msg byte, and status byte

[Christoph Hellwig]

On Mon, Sep 01, 2014 at 12:33:28PM +0000, Yoshihiro YUNOMAE wrote:
> For getting driver byte, host byte, msg byte, and status byte, macros are
> implemented in scsi/scsi.h, so we use it.

As mentioned about three times in various previous scsi logging discussions
this is entirely wrong and breaks decoding binary trace buffers.

NAK.

Re: Re: [PATCH 1/3] scsi/trace: Use macros for getting driver byte, host byte, msg byte, and status byte

[Yoshihiro YUNOMAE]

(2014/09/02 0:15), Christoph Hellwig wrote:
> On Mon, Sep 01, 2014 at 12:33:28PM +0000, Yoshihiro YUNOMAE wrote:
>> For getting driver byte, host byte, msg byte, and status byte, macros are
>> implemented in scsi/scsi.h, so we use it.
>
> As mentioned about three times in various previous scsi logging discussions
> this is entirely wrong and breaks decoding binary trace buffers.

No, this patch uses just macros, so this does not change decoders.
However, other patches change decoders in format files, so we need to
consider about these decoders more, as you say.
We'll discuss on https://lkml.org/lkml/2014/8/28/657.

Thanks,
Yoshihiro YUNOMAE

-- 
Yoshihiro YUNOMAE
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: yoshihiro.yunomae.ez@hitachi.com