Messages in this thread | | | Date | Wed, 27 Aug 2014 16:12:19 +0200 | From | Hannes Reinecke <> | Subject | Re: [RFC PATCH 07/10] scsi/trace: Use scsi_show_result trace point instead of printk |
| |
On 08/08/2014 01:50 PM, Yoshihiro YUNOMAE wrote: > Current SCSI trace has hostbyte table and driverbyte table, so we don't need to > have the same table in scsi/constants.c. > > - Result examples > > <Before> (printk) > sd 2:0:0:0: [sda] Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE > > <After> (ftrace) > scsi_show_result: host_no=2 channel=0 id=0 lun=0 [sda] result=(driver=DRIVER_SENSE host=DID_OK) > > 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: Hidehiro Kawai <hidehiro.kawai.ez@hitachi.com> > Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> > --- > drivers/scsi/constants.c | 52 ------------------------------------------- > drivers/scsi/scsi_trace.c | 16 +++++++++++++ > include/trace/events/scsi.h | 38 +++++++++++++++++++++++++++++++ > 3 files changed, 53 insertions(+), 53 deletions(-) > > diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c > index 6fad6b4..f7b7f32 100644 > --- a/drivers/scsi/constants.c > +++ b/drivers/scsi/constants.c > @@ -1488,55 +1488,3 @@ void scsi_print_sense(struct scsi_cmnd *cmd) > SCSI_SENSE_BUFFERSIZE); > } > EXPORT_SYMBOL(scsi_print_sense); > - > -#ifdef CONFIG_SCSI_CONSTANTS > - > -static const char * const hostbyte_table[]={ > -"DID_OK", "DID_NO_CONNECT", "DID_BUS_BUSY", "DID_TIME_OUT", "DID_BAD_TARGET", > -"DID_ABORT", "DID_PARITY", "DID_ERROR", "DID_RESET", "DID_BAD_INTR", > -"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(struct scsi_device *sdev, const char *name, int result) > -{ > - 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"; > - > - > - sdev_printk(KERN_INFO, sdev, > - "[%s] Result: hostbyte=%s driverbyte=%s\n", > - name, hb_string, db_string); > -} > - > -#else > - > -void scsi_show_result(struct scsi_device *sdev, const char *name, int result) > -{ > - sdev_printk(KERN_INFO, sdev, > - "[%s] Result: hostbyte=0x%02x driverbyte=0x%02x\n", > - name, host_byte(result), driver_byte(result)); > -} > - > -#endif > -EXPORT_SYMBOL(scsi_show_result); > - > -void scsi_print_result(struct scsi_cmnd *cmd) > -{ > - const char *devname = cmd->request->rq_disk ? > - cmd->request->rq_disk->disk_name : "scsi"; > - scsi_show_result(cmd->device, devname, cmd->result); > -} > -EXPORT_SYMBOL(scsi_print_result); > diff --git a/drivers/scsi/scsi_trace.c b/drivers/scsi/scsi_trace.c > index 2bea4f0..6ffbc40 100644 > --- a/drivers/scsi/scsi_trace.c > +++ b/drivers/scsi/scsi_trace.c > @@ -19,6 +19,8 @@ > #include <linux/trace_seq.h> > #include <trace/events/scsi.h> > > +#include <scsi/scsi_dbg.h> > + > #define SERVICE_ACTION16(cdb) (cdb[1] & 0x1f) > #define SERVICE_ACTION32(cdb) ((cdb[8] << 8) | cdb[9]) > > @@ -286,3 +288,17 @@ scsi_trace_parse_cdb(struct trace_seq *p, unsigned char *cdb, int len) > return scsi_trace_misc(p, cdb, len); > } > } > + > +void scsi_show_result(struct scsi_device *sdev, const char *name, int result) > +{ > + trace_scsi_show_result(sdev, name, result); > +} > +EXPORT_SYMBOL(scsi_show_result); > + > +void scsi_print_result(struct scsi_cmnd *cmd) > +{ > + const char *devname = cmd->request->rq_disk ? > + cmd->request->rq_disk->disk_name : "scsi"; > + scsi_show_result(cmd->device, devname, cmd->result); > +} > +EXPORT_SYMBOL(scsi_print_result); > diff --git a/include/trace/events/scsi.h b/include/trace/events/scsi.h > index 8aecdc2..0675195 100644 > --- a/include/trace/events/scsi.h > +++ b/include/trace/events/scsi.h > @@ -123,7 +123,11 @@ > scsi_hostbyte_name(DID_IMM_RETRY), \ > scsi_hostbyte_name(DID_REQUEUE), \ > scsi_hostbyte_name(DID_TRANSPORT_DISRUPTED), \ > - scsi_hostbyte_name(DID_TRANSPORT_FAILFAST)) > + scsi_hostbyte_name(DID_TRANSPORT_FAILFAST), \ > + scsi_hostbyte_name(DID_TARGET_FAILURE), \ > + scsi_hostbyte_name(DID_NEXUS_FAILURE), \ > + scsi_hostbyte_name(DID_ALLOC_FAILURE), \ > + scsi_hostbyte_name(DID_MEDIUM_ERROR)) > > #define scsi_driverbyte_name(result) { result, #result } > #define show_driverbyte_name(val) \ > @@ -359,6 +363,38 @@ TRACE_EVENT(scsi_eh_wakeup, > TP_printk("host_no=%u", __entry->host_no) > ); > > +TRACE_EVENT(scsi_show_result, > + > + TP_PROTO(struct scsi_device *sdev, const char *devname, int result), > + > + TP_ARGS(sdev, devname, result), > + > + TP_STRUCT__entry( > + __field( unsigned int, host_no ) > + __field( unsigned int, channel ) > + __field( unsigned int, id ) > + __field( unsigned int, lun ) > + __string(devname, devname ) > + __field( int, result ) > + ), > + > + TP_fast_assign( > + __entry->host_no = sdev->host->host_no; > + __entry->channel = sdev->channel; > + __entry->id = sdev->id; > + __entry->lun = sdev->lun; > + __assign_str(devname, devname); > + __entry->result = result; > + ), > + > + TP_printk("host_no=%u channel=%u id=%u lun=%u [%s] result=(driver=%s "\ > + "host=%s)", > + __entry->host_no, __entry->channel, __entry->id, __entry->lun, > + __get_str(devname), > + show_driverbyte_name(driver_byte(__entry->result)), > + show_hostbyte_name(host_byte(__entry->result))) > +); > + > #endif /* _TRACE_SCSI_H */ > > /* This part must be outside protection */ > > -- > 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 > Hmm. I'm not sure this is the correct way. Currently we have quite some code duplication in scsi_trace.c and constants.c, correct. So I definitely would like to see them both merged.
But constants.c is influenced by CONFIG_SCSI_CONSTANTS, whereas scsi_trace isn't, and the functions in constants.c are used throughout the scsi stack. So I'd rather see to have scsi_trace to be updated to use the functions from constants.c, and remove the duplicate code in scsi_trace.
At a later step we should be working on removing/replacing existing logging code in the SCSI stack with trace points, but that should be reserved for a separate 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-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |