Re: [PATCH 4/4] scsi: add more contexts in the ufs tracepoints

[Can Guo <cang@xxxxxxxxxxxxxx>]

On 2020-10-06 06:36, Jaegeuk Kim wrote:
> From: Jaegeuk Kim <jaegeuk@xxxxxxxxxx>
>
> This adds user-friendly tracepoints with group id.
>
> Cc: Alim Akhtar <alim.akhtar@xxxxxxxxxxx>
> Cc: Avri Altman <avri.altman@xxxxxxx>
> Cc: Can Guo <cang@xxxxxxxxxxxxxx>
> Signed-off-by: Jaegeuk Kim <jaegeuk@xxxxxxxxxx>

Reviewed-by: Can Guo <cang@xxxxxxxxxxxxxx>

> ---
>  drivers/scsi/ufs/ufshcd.c  |  6 ++++--
>  include/trace/events/ufs.h | 21 +++++++++++++++++----
>  2 files changed, 21 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 76e95963887be..a2db8182663da 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -336,7 +336,7 @@ static void ufshcd_add_command_trace(struct ufs_hba 
> *hba,
>  		unsigned int tag, const char *str)
>  {
>  	sector_t lba = -1;
> -	u8 opcode = 0;
> +	u8 opcode = 0, group_id = 0;
>  	u32 intr, doorbell;
>  	struct ufshcd_lrb *lrbp = &hba->lrb[tag];
>  	struct scsi_cmnd *cmd = lrbp->cmd;
> @@ -362,13 +362,15 @@ static void ufshcd_add_command_trace(struct 
> ufs_hba *hba,
>  				lba = cmd->request->bio->bi_iter.bi_sector;
>  			transfer_len = be32_to_cpu(
>  				lrbp->ucd_req_ptr->sc.exp_data_transfer_len);
> +			if (opcode == WRITE_10)
> +				group_id = lrbp->cmd->cmnd[6];
>  		}
>  	}
>
>  	intr = ufshcd_readl(hba, REG_INTERRUPT_STATUS);
>  	doorbell = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL);
>  	trace_ufshcd_command(dev_name(hba->dev), str, tag,
> -				doorbell, transfer_len, intr, lba, opcode);
> +			doorbell, transfer_len, intr, lba, opcode, group_id);
>  }
>
>  static void ufshcd_print_clk_freqs(struct ufs_hba *hba)
> diff --git a/include/trace/events/ufs.h b/include/trace/events/ufs.h
> index 84841b3a7ffd5..50654f3526392 100644
> --- a/include/trace/events/ufs.h
> +++ b/include/trace/events/ufs.h
> @@ -11,6 +11,15 @@
>
>  #include <linux/tracepoint.h>
>
> +#define str_opcode(opcode)						\
> +	__print_symbolic(opcode,					\
> +		{ WRITE_16,		"WRITE_16" },			\
> +		{ WRITE_10,		"WRITE_10" },			\
> +		{ READ_16,		"READ_16" },			\
> +		{ READ_10,		"READ_10" },			\
> +		{ SYNCHRONIZE_CACHE,	"SYNC" },			\
> +		{ UNMAP,		"UNMAP" })
> +
>  #define UFS_LINK_STATES			\
>  	EM(UIC_LINK_OFF_STATE)		\
>  	EM(UIC_LINK_ACTIVE_STATE)	\
> @@ -215,9 +224,10 @@ DEFINE_EVENT(ufshcd_template, ufshcd_init,
>  TRACE_EVENT(ufshcd_command,
>  	TP_PROTO(const char *dev_name, const char *str, unsigned int tag,
>  			u32 doorbell, int transfer_len, u32 intr, u64 lba,
> -			u8 opcode),
> +			u8 opcode, u8 group_id),
>
> -	TP_ARGS(dev_name, str, tag, doorbell, transfer_len, intr, lba, 
> opcode),
> +	TP_ARGS(dev_name, str, tag, doorbell, transfer_len,
> +				intr, lba, opcode, group_id),
>
>  	TP_STRUCT__entry(
>  		__string(dev_name, dev_name)
> @@ -228,6 +238,7 @@ TRACE_EVENT(ufshcd_command,
>  		__field(u32, intr)
>  		__field(u64, lba)
>  		__field(u8, opcode)
> +		__field(u8, group_id)
>  	),
>
>  	TP_fast_assign(
> @@ -239,13 +250,15 @@ TRACE_EVENT(ufshcd_command,
>  		__entry->intr = intr;
>  		__entry->lba = lba;
>  		__entry->opcode = opcode;
> +		__entry->group_id = group_id;
>  	),
>
>  	TP_printk(
> -		"%s: %s: tag: %u, DB: 0x%x, size: %d, IS: %u, LBA: %llu, opcode: 
> 0x%x",
> +		"%s: %s: tag: %u, DB: 0x%x, size: %d, IS: %u, LBA: %llu, opcode:
> 0x%x (%s), group_id: 0x%x",
>  		__get_str(str), __get_str(dev_name), __entry->tag,
>  		__entry->doorbell, __entry->transfer_len,
> -		__entry->intr, __entry->lba, (u32)__entry->opcode
> +		__entry->intr, __entry->lba, (u32)__entry->opcode,
> +		str_opcode(__entry->opcode), (u32)__entry->group_id
>  	)
>  );