* Re: [PATCH v3] scsi: ufs: core: Add trace event for MCQ
2023-02-22 7:39 ` [PATCH v3] scsi: ufs: core: Add trace event for MCQ Ziqi Chen
2023-02-22 19:27 ` Bean Huo
@ 2023-02-23 9:44 ` Can Guo
2023-02-23 22:27 ` Daejun Park
2 siblings, 0 replies; 4+ messages in thread
From: Can Guo @ 2023-02-23 9:44 UTC (permalink / raw)
To: Ziqi Chen, quic_asutoshd, bvanassche, mani, stanley.chu,
adrian.hunter, beanhuo, avri.altman, junwoo80.lee,
martin.petersen
Cc: linux-scsi, Alim Akhtar, James E.J. Bottomley, Steven Rostedt,
Masami Hiramatsu, open list, open list:TRACING
On 2/22/2023 3:39 PM, Ziqi Chen wrote:
> Added a new trace event to record MCQ relevant information
> for each request in MCQ mode, include hardware queue ID,
> SQ tail slot, CQ head slot and CQ tail slot.
>
> Signed-off-by: Ziqi Chen <quic_ziqichen@quicinc.com>
reviewed-by: Can Guo <quic_cang@quicinc.com>
>
> ---
> Changes to v2:
> - Shorten printing strings.
>
> Changes to v1:
> - Adjust the order of fileds to keep them aligned.
> ---
> drivers/ufs/core/ufshcd.c | 14 +++++++++++---
> include/trace/events/ufs.h | 48 ++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 59 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index 3b3cf78..67cb90d 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -426,6 +426,7 @@ static void ufshcd_add_command_trace(struct ufs_hba *hba, unsigned int tag,
> struct ufshcd_lrb *lrbp = &hba->lrb[tag];
> struct scsi_cmnd *cmd = lrbp->cmd;
> struct request *rq = scsi_cmd_to_rq(cmd);
> + struct ufs_hw_queue *hwq;
> int transfer_len = -1;
>
> if (!cmd)
> @@ -456,9 +457,16 @@ static void ufshcd_add_command_trace(struct ufs_hba *hba, unsigned int tag,
> }
>
> 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_t, tag,
> - doorbell, transfer_len, intr, lba, opcode, group_id);
> +
> + if (is_mcq_enabled(hba)) {
> + hwq = ufshcd_mcq_req_to_hwq(hba, rq);
> + trace_ufshcd_command_mcq(dev_name(hba->dev), str_t, tag,
> + hwq, transfer_len, intr, lba, opcode, group_id);
> + } else {
> + doorbell = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL);
> + trace_ufshcd_command(dev_name(hba->dev), str_t, tag,
> + 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 599739e..604b2cd 100644
> --- a/include/trace/events/ufs.h
> +++ b/include/trace/events/ufs.h
> @@ -10,6 +10,7 @@
> #define _TRACE_UFS_H
>
> #include <linux/tracepoint.h>
> +#include <ufs/ufshcd.h>
>
> #define str_opcode(opcode) \
> __print_symbolic(opcode, \
> @@ -307,6 +308,53 @@ TRACE_EVENT(ufshcd_command,
> )
> );
>
> +TRACE_EVENT(ufshcd_command_mcq,
> + TP_PROTO(const char *dev_name, enum ufs_trace_str_t str_t,
> + unsigned int tag, struct ufs_hw_queue *hwq, int transfer_len,
> + u32 intr, u64 lba, u8 opcode, u8 group_id),
> +
> + TP_ARGS(dev_name, str_t, tag, hwq, transfer_len, intr, lba, opcode, group_id),
> +
> + TP_STRUCT__entry(
> + __string(dev_name, dev_name)
> + __field(enum ufs_trace_str_t, str_t)
> + __field(unsigned int, tag)
> + __field(u32, hwq_id)
> + __field(u32, sq_tail)
> + __field(u32, cq_head)
> + __field(u32, cq_tail)
> + __field(int, transfer_len)
> + __field(u32, intr)
> + __field(u64, lba)
> + __field(u8, opcode)
> + __field(u8, group_id)
> + ),
> +
> + TP_fast_assign(
> + __assign_str(dev_name, dev_name);
> + __entry->str_t = str_t;
> + __entry->tag = tag;
> + __entry->hwq_id = hwq->id;
> + __entry->sq_tail = hwq->sq_tail_slot;
> + __entry->cq_head = hwq->cq_head_slot;
> + __entry->cq_tail = hwq->cq_tail_slot;
> + __entry->transfer_len = transfer_len;
> + __entry->intr = intr;
> + __entry->lba = lba;
> + __entry->opcode = opcode;
> + __entry->group_id = group_id;
> + ),
> +
> + TP_printk(
> + "%s: %s: tag: %u, hqid: %d, size: %d, IS: %u, LBA: %llu, opcode: 0x%x (%s), group_id: 0x%x, sqt: %d, cqh: %d, cqt: %d",
> + show_ufs_cmd_trace_str(__entry->str_t), __get_str(dev_name),
> + __entry->tag, __entry->hwq_id, __entry->transfer_len,
> + __entry->intr, __entry->lba, (u32)__entry->opcode,
> + str_opcode(__entry->opcode), (u32)__entry->group_id,
> + __entry->sq_tail, __entry->cq_head, __entry->cq_tail
> + )
> +);
> +
> TRACE_EVENT(ufshcd_uic_command,
> TP_PROTO(const char *dev_name, enum ufs_trace_str_t str_t, u32 cmd,
> u32 arg1, u32 arg2, u32 arg3),
^ permalink raw reply [flat|nested] 4+ messages in thread* RE: [PATCH v3] scsi: ufs: core: Add trace event for MCQ
2023-02-22 7:39 ` [PATCH v3] scsi: ufs: core: Add trace event for MCQ Ziqi Chen
2023-02-22 19:27 ` Bean Huo
2023-02-23 9:44 ` Can Guo
@ 2023-02-23 22:27 ` Daejun Park
2 siblings, 0 replies; 4+ messages in thread
From: Daejun Park @ 2023-02-23 22:27 UTC (permalink / raw)
To: Ziqi Chen, quic_asutoshd@quicinc.com, quic_cang@quicinc.com,
bvanassche@acm.org, mani@kernel.org, stanley.chu@mediatek.com,
adrian.hunter@intel.com, beanhuo@micron.com, avri.altman@wdc.com,
JunWoo Lee, martin.petersen@oracle.com
Cc: linux-scsi@vger.kernel.org, ALIM AKHTAR, James E.J. Bottomley,
Steven Rostedt, Masami Hiramatsu, open list, Daejun Park
Hi Ziqi Chen,
...
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index 3b3cf78..67cb90d 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -426,6 +426,7 @@ static void ufshcd_add_command_trace(struct ufs_hba *hba, unsigned int tag,
> struct ufshcd_lrb *lrbp = &hba->lrb[tag];
> struct scsi_cmnd *cmd = lrbp->cmd;
> struct request *rq = scsi_cmd_to_rq(cmd);
> + struct ufs_hw_queue *hwq;
> int transfer_len = -1;
>
> if (!cmd)
> @@ -456,9 +457,16 @@ static void ufshcd_add_command_trace(struct ufs_hba *hba, unsigned int tag,
> }
>
> 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_t, tag,
> - doorbell, transfer_len, intr, lba, opcode, group_id);
> +
> + if (is_mcq_enabled(hba)) {
> + hwq = ufshcd_mcq_req_to_hwq(hba, rq);
> + trace_ufshcd_command_mcq(dev_name(hba->dev), str_t, tag,
> + hwq, transfer_len, intr, lba, opcode, group_id);
How about passing member variables of hwq instead of passing hwq?
> + } else {
> + doorbell = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL);
> + trace_ufshcd_command(dev_name(hba->dev), str_t, tag,
> + 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 599739e..604b2cd 100644
> --- a/include/trace/events/ufs.h
> +++ b/include/trace/events/ufs.h
> @@ -10,6 +10,7 @@
> #define _TRACE_UFS_H
>
> #include <linux/tracepoint.h>
> +#include <ufs/ufshcd.h>
Then, it can be removed.
Thanks,
Daejun
^ permalink raw reply [flat|nested] 4+ messages in thread