Re: [PATCH 4/4] scsi: add more contexts in the ufs tracepoints
On 2020-10-20 19:57, Can Guo wrote: On 2020-10-20 19:02, Can Guo wrote: On 2020-10-20 18:51, Avri Altman wrote: On 2020-10-06 06:36, Jaegeuk Kim wrote: > From: Jaegeuk Kim > > This adds user-friendly tracepoints with group id. You have the entire cdb as part of the upiu trace, Can't you parse what you need from there? Thanks, Avri Yes, but assume we have a large trace log file, having a groud id allows us to filter the data by it easily, right? Thanks, Can Guo. I just dobule checked WRITE(10)'s CDB, byte 6 has group ID ONLY. So Avri is right, we don't even need to parse it, we can easily filter a ftrace log file by byte 6 to get the WRITE(10) cmds with specific group ID - we don't need this change. Thanks, Can Guo. Please ignore my previous mail, I misunderstood the change. :( You have my reivewed-by tag for this change. Regards, Can Guo.
Re: [PATCH 4/4] scsi: add more contexts in the ufs tracepoints
On 2020-10-20 19:02, Can Guo wrote: On 2020-10-20 18:51, Avri Altman wrote: On 2020-10-06 06:36, Jaegeuk Kim wrote: > From: Jaegeuk Kim > > This adds user-friendly tracepoints with group id. You have the entire cdb as part of the upiu trace, Can't you parse what you need from there? Thanks, Avri Yes, but assume we have a large trace log file, having a groud id allows us to filter the data by it easily, right? Thanks, Can Guo. I just dobule checked WRITE(10)'s CDB, byte 6 has group ID ONLY. So Avri is right, we don't even need to parse it, we can easily filter a ftrace log file by byte 6 to get the WRITE(10) cmds with specific group ID - we don't need this change. Thanks, Can Guo.
RE: [PATCH 4/4] scsi: add more contexts in the ufs tracepoints
> > On 2020-10-20 18:51, Avri Altman wrote: > >> > >> On 2020-10-06 06:36, Jaegeuk Kim wrote: > >> > From: Jaegeuk Kim > >> > > >> > This adds user-friendly tracepoints with group id. > > You have the entire cdb as part of the upiu trace, > > Can't you parse what you need from there? > > > > Thanks, > > Avri > > Yes, but assume we have a large trace log file, having a > groud id allows us to filter the data by it easily, right? > Ahha, ok. Thanks, Avri
Re: [PATCH 4/4] scsi: add more contexts in the ufs tracepoints
On 2020-10-20 18:51, Avri Altman wrote: On 2020-10-06 06:36, Jaegeuk Kim wrote: > From: Jaegeuk Kim > > This adds user-friendly tracepoints with group id. You have the entire cdb as part of the upiu trace, Can't you parse what you need from there? Thanks, Avri Yes, but assume we have a large trace log file, having a groud id allows us to filter the data by it easily, right? Thanks, Can Guo.
RE: [PATCH 4/4] scsi: add more contexts in the ufs tracepoints
> > On 2020-10-06 06:36, Jaegeuk Kim wrote: > > From: Jaegeuk Kim > > > > This adds user-friendly tracepoints with group id. You have the entire cdb as part of the upiu trace, Can't you parse what you need from there? Thanks, Avri
Re: [PATCH 4/4] scsi: add more contexts in the ufs tracepoints
On 2020-10-06 06:36, Jaegeuk Kim wrote:
From: Jaegeuk Kim
This adds user-friendly tracepoints with group id.
Cc: Alim Akhtar
Cc: Avri Altman
Cc: Can Guo
Signed-off-by: Jaegeuk Kim
Reviewed-by: Can Guo
---
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
+#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
)
);
