On 11/14/22 20:22, 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@xxxxxxx>
2 nits below. Otherwise, looks good to me.
Reviewed-by: Damien Le Moal <damien.lemoal@xxxxxxxxxxxxxxxxxx>
> ---
> drivers/scsi/sd_zbc.c | 5 +++
> include/trace/events/scsi.h | 64 +++++++++++++++++++++++++++++++++++++
> 2 files changed, 69 insertions(+)
>
> diff --git a/drivers/scsi/sd_zbc.c b/drivers/scsi/sd_zbc.c
> index bd15624c6322..956d1982c51b 100644
> --- a/drivers/scsi/sd_zbc.c
> +++ b/drivers/scsi/sd_zbc.c
> @@ -18,6 +18,8 @@
> #include <scsi/scsi.h>
> #include <scsi/scsi_cmnd.h>
>
> +#include <trace/events/scsi.h>
> +
> #include "sd.h"
>
> /**
> @@ -450,6 +452,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 +561,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/scsi.h b/include/trace/events/scsi.h
> index a2c7befd451a..50d36aa417cc 100644
> --- a/include/trace/events/scsi.h
> +++ b/include/trace/events/scsi.h
> @@ -327,6 +327,70 @@ TRACE_EVENT(scsi_eh_wakeup,
> TP_printk("host_no=%u", __entry->host_no)
> );
>
> +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 )
Why not align "lba" with the other fields ?
> + __field( unsigned int, wp_offset)
missing space before ")" (since that seems to be the style convention in
that file.
> + ),
> +
> + 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)
Same comment as above for rq_sector and missing spaces before ")".
> + ),
> +
> + 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 /* _TRACE_SCSI_H */
>
> /* This part must be outside protection */
--
Damien Le Moal
Western Digital Research
