Re: Re: Re: [RFC PATCH 10/10] scsi/trace: Use scsi_print_command trace point instead of printk
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 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,