| Message ID | 39540065afb936255236311fc6c93f67a7805bf6.1669797508.git.johannes.thumshirn@wdc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v4] scsi: sd_zbc: trace zone append emulation | expand |
On 11/30/22 17:39, Johannes Thumshirn wrote: > Add tracepoints to the SCSI zone append emulation, in order to trace the > zone start to write-pointer aligned LBA translation and the corresponding > completion. > > Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com> [...] > diff --git a/include/trace/events/sd.h b/include/trace/events/sd.h > new file mode 100644 > index 000000000000..0b11fed8327b > --- /dev/null > +++ b/include/trace/events/sd.h Why not drivers/scsi/sd_trace.h ? That would work too, no ? Do we need that header as a common thing under include/trace/events/ ?
On Wed, Nov 30, 2022 at 06:59:43PM +0900, Damien Le Moal wrote:
> Why not drivers/scsi/sd_trace.h ? That would work too, no ?
Yes, that's also what I suggested previously.
On 30.11.22 10:59, Damien Le Moal wrote: > On 11/30/22 17:39, Johannes Thumshirn wrote: >> Add tracepoints to the SCSI zone append emulation, in order to trace the >> zone start to write-pointer aligned LBA translation and the corresponding >> completion. >> >> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com> > > [...] > >> diff --git a/include/trace/events/sd.h b/include/trace/events/sd.h >> new file mode 100644 >> index 000000000000..0b11fed8327b >> --- /dev/null >> +++ b/include/trace/events/sd.h > > Why not drivers/scsi/sd_trace.h ? That would work too, no ? Do we need > that header as a common thing under include/trace/events/ ? Well it's the common place for tracepoint headers.
On 30.11.22 11:01, Christoph Hellwig wrote: > On Wed, Nov 30, 2022 at 06:59:43PM +0900, Damien Le Moal wrote: >> Why not drivers/scsi/sd_trace.h ? That would work too, no ? > > Yes, that's also what I suggested previously. > But what's the reasoning behind it? All other trace events are in include/trace/events/.
On Wed, Nov 30, 2022 at 12:39:21AM -0800, Johannes Thumshirn wrote:
> +EXPORT_SYMBOL(scsi_trace_parse_cdb);
Unless I'm missing something you don't actually use this anywhere.
On 30.11.22 11:02, Christoph Hellwig wrote: > On Wed, Nov 30, 2022 at 12:39:21AM -0800, Johannes Thumshirn wrote: >> +EXPORT_SYMBOL(scsi_trace_parse_cdb); > > Unless I'm missing something you don't actually use this anywhere. > Gah correct, that's a leftover that isn't used.
On Wed, Nov 30, 2022 at 10:02:20AM +0000, Johannes Thumshirn wrote: > But what's the reasoning behind it? All other trace events > are in include/trace/events/. No, they aren't. Do a quick grep for TRACE_SYSTEM to show they mostly are with the actual drivers and subsystems.
On 30.11.22 11:03, hch@infradead.org wrote: > On Wed, Nov 30, 2022 at 10:02:20AM +0000, Johannes Thumshirn wrote: >> But what's the reasoning behind it? All other trace events >> are in include/trace/events/. > > No, they aren't. Do a quick grep for TRACE_SYSTEM to show they mostly > are with the actual drivers and subsystems. > For scsi it is in include/trace/events/scsi.h, but if you want I'll move it to drivers/scsi.
diff --git a/drivers/scsi/scsi_trace.c b/drivers/scsi/scsi_trace.c index 41a950075913..224b38c0fb0f 100644 --- a/drivers/scsi/scsi_trace.c +++ b/drivers/scsi/scsi_trace.c @@ -389,3 +389,4 @@ scsi_trace_parse_cdb(struct trace_seq *p, unsigned char *cdb, int len) return scsi_trace_misc(p, cdb, len); } } +EXPORT_SYMBOL(scsi_trace_parse_cdb); diff --git a/drivers/scsi/sd_zbc.c b/drivers/scsi/sd_zbc.c index bd15624c6322..8985681cffbd 100644 --- a/drivers/scsi/sd_zbc.c +++ b/drivers/scsi/sd_zbc.c @@ -18,6 +18,9 @@ #include <scsi/scsi.h> #include <scsi/scsi_cmnd.h> +#define CREATE_TRACE_POINTS +#include <trace/events/sd.h> + #include "sd.h" /** @@ -450,6 +453,7 @@ blk_status_t sd_zbc_prepare_zone_append(struct scsi_cmnd *cmd, sector_t *lba, break; } + trace_scsi_prepare_zone_append(cmd, *lba, wp_offset); *lba += wp_offset; } spin_unlock_irqrestore(&sdkp->zones_wp_offset_lock, flags); @@ -558,6 +562,8 @@ static unsigned int sd_zbc_zone_wp_update(struct scsi_cmnd *cmd, switch (op) { case REQ_OP_ZONE_APPEND: + trace_scsi_zone_wp_update(cmd, rq->__sector, + sdkp->zones_wp_offset[zno], good_bytes); rq->__sector += sdkp->zones_wp_offset[zno]; fallthrough; case REQ_OP_WRITE_ZEROES: diff --git a/include/trace/events/sd.h b/include/trace/events/sd.h new file mode 100644 index 000000000000..0b11fed8327b --- /dev/null +++ b/include/trace/events/sd.h @@ -0,0 +1,81 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright (C) 2022 Western Digital Corporation or its affiliates. + */ +#undef TRACE_SYSTEM +#define TRACE_SYSTEM sd + +#if !defined(_SD_TRACE_H) || defined(TRACE_HEADER_MULTI_READ) +#include <scsi/scsi_cmnd.h> +#include <scsi/scsi_host.h> +#include <linux/tracepoint.h> +#include <linux/trace_seq.h> + +TRACE_EVENT(scsi_prepare_zone_append, + + TP_PROTO(struct scsi_cmnd *cmnd, sector_t lba, + unsigned int wp_offset), + + TP_ARGS(cmnd, lba, wp_offset), + + TP_STRUCT__entry( + __field( unsigned int, host_no ) + __field( unsigned int, channel ) + __field( unsigned int, id ) + __field( unsigned int, lun ) + __field( sector_t, lba ) + __field( unsigned int, wp_offset ) + ), + + TP_fast_assign( + __entry->host_no = cmnd->device->host->host_no; + __entry->channel = cmnd->device->channel; + __entry->id = cmnd->device->id; + __entry->lun = cmnd->device->lun; + __entry->lba = lba; + __entry->wp_offset = wp_offset; + ), + + TP_printk("host_no=%u, channel=%u id=%u lun=%u lba=%llu wp_offset=%u", + __entry->host_no, __entry->channel, __entry->id, + __entry->lun, __entry->lba, __entry->wp_offset) +); + +TRACE_EVENT(scsi_zone_wp_update, + + TP_PROTO(struct scsi_cmnd *cmnd, sector_t rq_sector, + unsigned int wp_offset, unsigned int good_bytes), + + TP_ARGS(cmnd, rq_sector, wp_offset, good_bytes), + + TP_STRUCT__entry( + __field( unsigned int, host_no ) + __field( unsigned int, channel ) + __field( unsigned int, id ) + __field( unsigned int, lun ) + __field( sector_t, rq_sector ) + __field( unsigned int, wp_offset ) + __field( unsigned int, good_bytes ) + ), + + TP_fast_assign( + __entry->host_no = cmnd->device->host->host_no; + __entry->channel = cmnd->device->channel; + __entry->id = cmnd->device->id; + __entry->lun = cmnd->device->lun; + __entry->rq_sector = rq_sector; + __entry->wp_offset = wp_offset; + __entry->good_bytes = good_bytes; + ), + + TP_printk("host_no=%u, channel=%u id=%u lun=%u rq_sector=%llu" \ + " wp_offset=%u good_bytes=%u", + __entry->host_no, __entry->channel, __entry->id, + __entry->lun, __entry->rq_sector, __entry->wp_offset, + __entry->good_bytes) +); +#endif /* _SD_TRACE_H */ + +/* This part must be outside protection */ +#include <trace/define_trace.h> +
Add tracepoints to the SCSI zone append emulation, in order to trace the zone start to write-pointer aligned LBA translation and the corresponding completion. Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com> --- Changes to v3: - Move tracepoints into own sd trace events header Changes to v2: - Fix linking when SCSI is a module Changes to v1: - Fix style issues pointed out by Damien --- drivers/scsi/scsi_trace.c | 1 + drivers/scsi/sd_zbc.c | 6 +++ include/trace/events/sd.h | 81 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 88 insertions(+) create mode 100644 include/trace/events/sd.h