Re: Re: Re: [RFC PATCH 10/10] scsi/trace: Use scsi_print_command trace point instead of printk

2014-08-28 Thread Yoshihiro YUNOMAE

Hi Hannes,

I tried to remove duplicated decoder of SCSI command, but the
output format of it in constants.c is different from it in traceevents.
I have two questions for it.

(Ex1)
traceevents: TEST_UNIT_READY
constants: Test Unit Ready

= Which of XXX_YYY_ZZZ and Xxx Yyy Zzz should the kernel output
strings? This difference comes from difference of implementation.
The decoder in traceevents are using macro. So, it cannot define
separated words. On the other hand, the decoder in constants are using
constant string array table. So, it can define separated words.

(Ex2)
traceevents: (nothing)
constants: Set limits(12)

= Should we merge those decoder?

I understand we use the decoder of constants, but we need to solve
these problems. Would you give me your comments?

Thanks,
Yoshihiro YUNOMAE

(2014/08/28 10:40), Yoshihiro YUNOMAE wrote:

(2014/08/27 23:16), Hannes Reinecke wrote:

On 08/08/2014 01:50 PM, Yoshihiro YUNOMAE wrote:

Previous printk messages of SCSI command can be mixed into other printk
messages because multiple printk messages are output for it. To avoid
the
problem, patch 4e64bb8d6 in Hannes' branch(*1) introduced a local
buffer.
But using local buffers can induce stack overflow, so we want to solve
the
problem without local buffer if possible.

trace_seq_printf can add log messages without local buffer, so we use
it.

Note:
We don't need constans.c any more.

(*1)
http://git.kernel.org/cgit/linux/kernel/git/hare/scsi-devel.git/log/?h=logging



  - Result examples

Before (printk)
sd 2:0:0:0: [sda] CDB: Read(10)

After
scsi_print_command: host_no=2 channel=0 id=0 lun=0 [sda] CDB (Read(10))

Signed-off-by: Yoshihiro YUNOMAE yoshihiro.yunomae...@hitachi.com
Cc: Hannes Reinecke h...@suse.de
Cc: Doug Gilbert dgilb...@interlog.com
Cc: Martin K. Petersen martin.peter...@oracle.com
Cc: Christoph Hellwig h...@lst.de
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Hidehiro Kawai hidehiro.kawai...@hitachi.com
Cc: Masami Hiramatsu masami.hiramatsu...@hitachi.com
---
  drivers/scsi/Makefile   |2
  drivers/scsi/constants.c|  425
---
  drivers/scsi/scsi_trace.c   |  408
+
  include/scsi/scsi.h |8 +
  include/trace/events/scsi.h |   45 +
  5 files changed, 461 insertions(+), 427 deletions(-)
  delete mode 100644 drivers/scsi/constants.c

diff --git a/drivers/scsi/Makefile b/drivers/scsi/Makefile
index 5f0d299..c56f692 100644
--- a/drivers/scsi/Makefile
+++ b/drivers/scsi/Makefile
@@ -158,7 +158,7 @@ obj-$(CONFIG_SCSI_OSD_INITIATOR) += osd/
  # This goes last, so that real scsi devices probe earlier
  obj-$(CONFIG_SCSI_DEBUG)+= scsi_debug.o

-scsi_mod-y+= scsi.o hosts.o scsi_ioctl.o constants.o \
+scsi_mod-y+= scsi.o hosts.o scsi_ioctl.o \
 scsicam.o scsi_error.o scsi_lib.o
  scsi_mod-$(CONFIG_SCSI_DMA)+= scsi_lib_dma.o
  scsi_mod-y+= scsi_scan.o scsi_sysfs.o scsi_devinfo.o
diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
deleted file mode 100644
index ce9ceb8..000
--- a/drivers/scsi/constants.c
+++ /dev/null
@@ -1,425 +0,0 @@
-/*
- * ASCII values for a number of symbolic constants, printing functions,
- * etc.
- * Additions for SCSI 2 and Linux 2.2.x by D. Gilbert (990422)
- * Additions for SCSI 3+ (SPC-3 T10/1416-D Rev 07 3 May 2002)
- *   by D. Gilbert and aeb (20020609)
- * Updated to SPC-4 T10/1713-D Rev 36g, D. Gilbert 20130701
- */
-
-#include linux/blkdev.h
-#include linux/module.h
-#include linux/kernel.h
-#include asm/unaligned.h
-
-#include scsi/scsi.h
-#include scsi/scsi_cmnd.h
-
-/* Commands with service actions that change the command name */
-#define SERVICE_ACTION_IN_12 0xab
-#define SERVICE_ACTION_OUT_12 0xa9
-#define SERVICE_ACTION_BIDIRECTIONAL 0x9d
-#define SERVICE_ACTION_IN_16 0x9e
-#define SERVICE_ACTION_OUT_16 0x9f
-#define THIRD_PARTY_COPY_OUT 0x83
-#define THIRD_PARTY_COPY_IN 0x84
-
-
-
-#ifdef CONFIG_SCSI_CONSTANTS
-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,
-/* 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),
-Reserve(6),
-/* 17-1a */ Release(6), Copy, Erase, Mode Sense(6),
-/* 1b-1d */ Start/Stop Unit, Receive Diagnostic, Send Diagnostic,
-/* 1e-1f */ Prevent/Allow Medium Removal, NULL,
-/* 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,
-/* 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 

Re: Re: [RFC PATCH 10/10] scsi/trace: Use scsi_print_command trace point instead of printk

2014-08-27 Thread Yoshihiro YUNOMAE

(2014/08/27 23:16), Hannes Reinecke wrote:

On 08/08/2014 01:50 PM, Yoshihiro YUNOMAE wrote:

Previous printk messages of SCSI command can be mixed into other printk
messages because multiple printk messages are output for it. To avoid the
problem, patch 4e64bb8d6 in Hannes' branch(*1) introduced a local buffer.
But using local buffers can induce stack overflow, so we want to solve
the
problem without local buffer if possible.

trace_seq_printf can add log messages without local buffer, so we use it.

Note:
We don't need constans.c any more.

(*1)
http://git.kernel.org/cgit/linux/kernel/git/hare/scsi-devel.git/log/?h=logging


  - Result examples

Before (printk)
sd 2:0:0:0: [sda] CDB: Read(10)

After
scsi_print_command: host_no=2 channel=0 id=0 lun=0 [sda] CDB (Read(10))

Signed-off-by: Yoshihiro YUNOMAE yoshihiro.yunomae...@hitachi.com
Cc: Hannes Reinecke h...@suse.de
Cc: Doug Gilbert dgilb...@interlog.com
Cc: Martin K. Petersen martin.peter...@oracle.com
Cc: Christoph Hellwig h...@lst.de
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Hidehiro Kawai hidehiro.kawai...@hitachi.com
Cc: Masami Hiramatsu masami.hiramatsu...@hitachi.com
---
  drivers/scsi/Makefile   |2
  drivers/scsi/constants.c|  425
---
  drivers/scsi/scsi_trace.c   |  408
+
  include/scsi/scsi.h |8 +
  include/trace/events/scsi.h |   45 +
  5 files changed, 461 insertions(+), 427 deletions(-)
  delete mode 100644 drivers/scsi/constants.c

diff --git a/drivers/scsi/Makefile b/drivers/scsi/Makefile
index 5f0d299..c56f692 100644
--- a/drivers/scsi/Makefile
+++ b/drivers/scsi/Makefile
@@ -158,7 +158,7 @@ obj-$(CONFIG_SCSI_OSD_INITIATOR) += osd/
  # This goes last, so that real scsi devices probe earlier
  obj-$(CONFIG_SCSI_DEBUG)+= scsi_debug.o

-scsi_mod-y+= scsi.o hosts.o scsi_ioctl.o constants.o \
+scsi_mod-y+= scsi.o hosts.o scsi_ioctl.o \
 scsicam.o scsi_error.o scsi_lib.o
  scsi_mod-$(CONFIG_SCSI_DMA)+= scsi_lib_dma.o
  scsi_mod-y+= scsi_scan.o scsi_sysfs.o scsi_devinfo.o
diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
deleted file mode 100644
index ce9ceb8..000
--- a/drivers/scsi/constants.c
+++ /dev/null
@@ -1,425 +0,0 @@
-/*
- * ASCII values for a number of symbolic constants, printing functions,
- * etc.
- * Additions for SCSI 2 and Linux 2.2.x by D. Gilbert (990422)
- * Additions for SCSI 3+ (SPC-3 T10/1416-D Rev 07 3 May 2002)
- *   by D. Gilbert and aeb (20020609)
- * Updated to SPC-4 T10/1713-D Rev 36g, D. Gilbert 20130701
- */
-
-#include linux/blkdev.h
-#include linux/module.h
-#include linux/kernel.h
-#include asm/unaligned.h
-
-#include scsi/scsi.h
-#include scsi/scsi_cmnd.h
-
-/* Commands with service actions that change the command name */
-#define SERVICE_ACTION_IN_12 0xab
-#define SERVICE_ACTION_OUT_12 0xa9
-#define SERVICE_ACTION_BIDIRECTIONAL 0x9d
-#define SERVICE_ACTION_IN_16 0x9e
-#define SERVICE_ACTION_OUT_16 0x9f
-#define THIRD_PARTY_COPY_OUT 0x83
-#define THIRD_PARTY_COPY_IN 0x84
-
-
-
-#ifdef CONFIG_SCSI_CONSTANTS
-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,
-/* 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),
-Reserve(6),
-/* 17-1a */ Release(6), Copy, Erase, Mode Sense(6),
-/* 1b-1d */ Start/Stop Unit, Receive Diagnostic, Send Diagnostic,
-/* 1e-1f */ Prevent/Allow Medium Removal, NULL,
-/* 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,
-/* 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),
-Read Defect Data(10),
-/* 38-3c */ Medium Scan, Compare, Copy Verify, Write Buffer,
-Read Buffer,
-/* 3d-3f */ Update Block, Read Long(10),  Write Long(10),
-/* 40-41 */ Change Definition, Write Same(10),
-/* 42-48 */ Unmap/Read sub-channel, Read TOC/PMA/ATIP,
-Read density support, Play audio(10), Get configuration,
-Play audio msf, Sanitize/Play audio track/index,
-/* 49-4f */ Play track relative(10), Get event status notification,
-Pause/resume, Log Select, Log Sense, Stop play/scan,
-NULL,
-/* 50-55 */ Xdwrite, Xpwrite, Read disk info, Xdread, Read track
info,
-Reserve track, Send OPC info, Mode Select(10),
-/* 56-5b */ Reserve(10), Release(10), Repair track, Read
master cue,
-Mode Sense(10), Close track/session,
-/* 5c-5f */ Read buffer capacity,