[PATCH 1/2] nvme: add set feature tracing support
Hou Pu
houpu.main at gmail.com
Mon Jul 5 00:43:26 PDT 2021
On Mon, Jul 5, 2021 at 3:16 PM Max Gurtovoy <mgurtovoy at nvidia.com> wrote:
>
>
> On 7/5/2021 6:15 AM, Hou Pu wrote:
> > A nvme connect command produces following trace.
> >
> > Before:
> > /sys/kernel/debug/tracing# cat trace | grep feature
> > kworker/5:1H-98 [005] .... 3221.294844: nvme_setup_cmd: nvme0: qid=0, cmdid=25, nsid=0, flags=0x0, meta=0x0, cmd=(nvme_admin_set_features cdw10=07 00 00 00 07 00 07 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00)
> > kworker/4:1H-124 [004] .... 3222.009186: nvme_setup_cmd: nvme0: qid=0, cmdid=17, nsid=0, flags=0x0, meta=0x0, cmd=(nvme_admin_set_features cdw10=0b 00 00 00 00 09 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00)
> >
> > After:
> > /sys/kernel/debug/tracing# cat trace | grep feature
> > kworker/0:1H-253 [000] .... 196.060509: nvme_setup_cmd: nvme0: qid=0, cmdid=29, nsid=0, flags=0x0, meta=0x0, cmd=(nvme_admin_set_features fid=0x7, sv=0x0, cdw11=0x70007)
> > kworker/0:1H-253 [000] .... 196.763947: nvme_setup_cmd: nvme0: qid=0, cmdid=29, nsid=0, flags=0x0, meta=0x0, cmd=(nvme_admin_set_features fid=0xb, sv=0x0, cdw11=0x900)
> >
> > Using ',' to separate different field like others in
> > nvmet_trace_admin_get_features.
> >
> > Signed-off-by: Hou Pu <houpu.main at gmail.com>
> > ---
> > drivers/nvme/host/trace.c | 18 +++++++++++++++++-
> > 1 file changed, 17 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/nvme/host/trace.c b/drivers/nvme/host/trace.c
> > index 6543015b6121..2a89c5aa0790 100644
> > --- a/drivers/nvme/host/trace.c
> > +++ b/drivers/nvme/host/trace.c
> > @@ -72,6 +72,20 @@ static const char *nvme_trace_admin_identify(struct trace_seq *p, u8 *cdw10)
> > return ret;
> > }
> >
> > +static const char *nvme_trace_admin_set_features(struct trace_seq *p,
> > + u8 *cdw10)
> > +{
> > + const char *ret = trace_seq_buffer_ptr(p);
> > + u8 fid = cdw10[0];
> > + u8 sv = cdw10[3] & 0x8;
> > + u32 cdw11 = get_unaligned_le32(cdw10 + 4);
>
> By the spec cdw10, 11, 12, 13, 14, 15 are used by set features command.
>
> At least lets print cdw11-cdw15 as raw data and not only cdw11.
>
> We don't want to loose information.
Yes, it's better to keep these although currently only cdw11 is used.
Will change it in v2. Thanks.
>
> > +
> > + trace_seq_printf(p, "fid=0x%x, sv=0x%x, cdw11=0x%x", fid, sv, cdw11);
> > + trace_seq_putc(p, 0);
> > +
> > + return ret;
> > +}
> > +
> > static const char *nvme_trace_admin_get_features(struct trace_seq *p,
> > u8 *cdw10)
> > {
> > @@ -80,7 +94,7 @@ static const char *nvme_trace_admin_get_features(struct trace_seq *p,
> > u8 sel = cdw10[1] & 0x7;
> > u32 cdw11 = get_unaligned_le32(cdw10 + 4);
> >
> > - trace_seq_printf(p, "fid=0x%x sel=0x%x cdw11=0x%x", fid, sel, cdw11);
> > + trace_seq_printf(p, "fid=0x%x, sel=0x%x, cdw11=0x%x", fid, sel, cdw11);
>
> This can be done in separate commit.
>
> Also if you can fix the current status and print also cdw14 since it's
> used by get features by the spec (in separate commit).
OK. Thanks for the comment.
>
>
> > trace_seq_putc(p, 0);
> >
> > return ret;
> > @@ -201,6 +215,8 @@ const char *nvme_trace_parse_admin_cmd(struct trace_seq *p,
> > return nvme_trace_create_cq(p, cdw10);
> > case nvme_admin_identify:
> > return nvme_trace_admin_identify(p, cdw10);
> > + case nvme_admin_set_features:
> > + return nvme_trace_admin_set_features(p, cdw10);
> > case nvme_admin_get_features:
> > return nvme_trace_admin_get_features(p, cdw10);
> > case nvme_admin_get_lba_status:
More information about the Linux-nvme
mailing list