* nvme host complete request tracing
@ 2023-05-17 16:27 Engel, Amit
2023-05-18 9:18 ` Sagi Grimberg
0 siblings, 1 reply; 6+ messages in thread
From: Engel, Amit @ 2023-05-17 16:27 UTC (permalink / raw)
To: linux-nvme; +Cc: Engel, Amit
Hello,
nvme/host/core.c trace_nvme_complete_rq() is being called immediately when entering nvme_complete_rq()
As I understand it, the idea behind this trace is to trace every single nvme request that is being completed.
but this is actually not accurate since there are several options for a request as part of this function:
COMPLETE, RETRY, FAILOVER
In order to better understand the status per nvme request I think it’s better to have a separate trace per each case, something like:
trace_nvme_end_rq(req), trace_nvme_retry_rq(req), trace_nvme_failover_rq(req) etc.
Would love to hear your inputs.
Thanks,
Amit E
Internal Use - Confidential
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: nvme host complete request tracing
2023-05-17 16:27 nvme host complete request tracing Engel, Amit
@ 2023-05-18 9:18 ` Sagi Grimberg
2023-05-19 8:59 ` Chaitanya Kulkarni
0 siblings, 1 reply; 6+ messages in thread
From: Sagi Grimberg @ 2023-05-18 9:18 UTC (permalink / raw)
To: Engel, Amit, linux-nvme
> Hello,
>
> nvme/host/core.c trace_nvme_complete_rq() is being called immediately when entering nvme_complete_rq()
> As I understand it, the idea behind this trace is to trace every single nvme request that is being completed.
> but this is actually not accurate since there are several options for a request as part of this function:
> COMPLETE, RETRY, FAILOVER
Well, it is completing this particular request.
> In order to better understand the status per nvme request I think it’s better to have a separate trace per each case, something like:
> trace_nvme_end_rq(req), trace_nvme_retry_rq(req), trace_nvme_failover_rq(req) etc.
Maybe we can add the disposition to the trace?
Something like:
--
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index b374e6007553..eaf03ff61224 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -393,14 +393,17 @@ static inline void nvme_end_req(struct request *req)
void nvme_complete_rq(struct request *req)
{
struct nvme_ctrl *ctrl = nvme_req(req)->ctrl;
+ enum nvme_disposition disposition;
- trace_nvme_complete_rq(req);
nvme_cleanup_cmd(req);
if (ctrl->kas)
ctrl->comp_seen = true;
- switch (nvme_decide_disposition(req)) {
+ disposition = nvme_decide_disposition(req);
+ trace_nvme_complete_rq(req, disposition);
+
+ switch (disposition) {
case COMPLETE:
nvme_end_req(req);
return;
--
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: nvme host complete request tracing
2023-05-18 9:18 ` Sagi Grimberg
@ 2023-05-19 8:59 ` Chaitanya Kulkarni
2023-05-19 9:18 ` Engel, Amit
0 siblings, 1 reply; 6+ messages in thread
From: Chaitanya Kulkarni @ 2023-05-19 8:59 UTC (permalink / raw)
To: Sagi Grimberg; +Cc: linux-nvme, Engel, Amit
Sagi,
On 5/18/23 02:18, Sagi Grimberg wrote:
>
>> Hello,
>>
>> nvme/host/core.c trace_nvme_complete_rq() is being called immediately
>> when entering nvme_complete_rq()
>> As I understand it, the idea behind this trace is to trace every
>> single nvme request that is being completed.
>> but this is actually not accurate since there are several options for
>> a request as part of this function:
>> COMPLETE, RETRY, FAILOVER
>
> Well, it is completing this particular request.
>
>> In order to better understand the status per nvme request I think
>> it’s better to have a separate trace per each case, something like:
>> trace_nvme_end_rq(req), trace_nvme_retry_rq(req),
>> trace_nvme_failover_rq(req) etc.
>
> Maybe we can add the disposition to the trace?
>
> Something like:
> --
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index b374e6007553..eaf03ff61224 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -393,14 +393,17 @@ static inline void nvme_end_req(struct request
> *req)
> void nvme_complete_rq(struct request *req)
> {
> struct nvme_ctrl *ctrl = nvme_req(req)->ctrl;
> + enum nvme_disposition disposition;
>
> - trace_nvme_complete_rq(req);
> nvme_cleanup_cmd(req);
>
> if (ctrl->kas)
> ctrl->comp_seen = true;
>
> - switch (nvme_decide_disposition(req)) {
> + disposition = nvme_decide_disposition(req);
> + trace_nvme_complete_rq(req, disposition);
> +
> + switch (disposition) {
> case COMPLETE:
> nvme_end_req(req);
> return;
> --
>
Adding disposition to the nvme_complete_rq creates redundant
information since it will always be zero for status=0. Please have
a look at [1].
what do you think about optionally printing disposition value only
when disposition != COMPLETE in the trace_nvme_complete_rq() ?
see [2].
but if everyone is okay with redundant information I'll be happy to
send [1] :).
-ck
[1] duplicate request disposition information with unconditionally
printing disposition value :-
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index f586a4808e6e..9164fee13704 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -337,13 +337,6 @@ static void nvme_log_error(struct request *req)
nr->status & NVME_SC_DNR ? "DNR " : "");
}
-enum nvme_disposition {
- COMPLETE,
- RETRY,
- FAILOVER,
- AUTHENTICATE,
-};
-
static inline enum nvme_disposition nvme_decide_disposition(struct
request *req)
{
if (likely(nvme_req(req)->status == 0))
@@ -393,14 +386,16 @@ static inline void nvme_end_req(struct request *req)
void nvme_complete_rq(struct request *req)
{
struct nvme_ctrl *ctrl = nvme_req(req)->ctrl;
+ enum nvme_disposition disposition;
- trace_nvme_complete_rq(req);
nvme_cleanup_cmd(req);
if (ctrl->kas)
ctrl->comp_seen = true;
- switch (nvme_decide_disposition(req)) {
+ disposition = nvme_decide_disposition(req);
+ trace_nvme_complete_rq(req, disposition);
+ switch (disposition) {
case COMPLETE:
nvme_end_req(req);
return;
@@ -424,7 +419,7 @@ EXPORT_SYMBOL_GPL(nvme_complete_rq);
void nvme_complete_batch_req(struct request *req)
{
- trace_nvme_complete_rq(req);
+ trace_nvme_complete_rq(req, COMPLETE);
nvme_cleanup_cmd(req);
nvme_end_req_zoned(req);
}
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index f0a84e390a55..56e16bdd59ff 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -1119,4 +1119,12 @@ static inline const unsigned char
*nvme_opcode_str(int qid, u8 opcode, u8 fctype
return qid ? nvme_get_opcode_str(opcode) :
nvme_get_admin_opcode_str(opcode);
}
+
+enum nvme_disposition {
+ COMPLETE,
+ RETRY,
+ FAILOVER,
+ AUTHENTICATE,
+};
+
#endif /* _NVME_H */
diff --git a/drivers/nvme/host/trace.h b/drivers/nvme/host/trace.h
index 4fb5922ffdac..32290b058c0f 100644
--- a/drivers/nvme/host/trace.h
+++ b/drivers/nvme/host/trace.h
@@ -83,8 +83,8 @@ TRACE_EVENT(nvme_setup_cmd,
);
TRACE_EVENT(nvme_complete_rq,
- TP_PROTO(struct request *req),
- TP_ARGS(req),
+ TP_PROTO(struct request *req, u8 disposition),
+ TP_ARGS(req, disposition),
TP_STRUCT__entry(
__array(char, disk, DISK_NAME_LEN)
__field(int, ctrl_id)
@@ -93,6 +93,7 @@ TRACE_EVENT(nvme_complete_rq,
__field(u64, result)
__field(u8, retries)
__field(u8, flags)
+ __field(u8, disposition)
__field(u16, status)
),
TP_fast_assign(
@@ -102,13 +103,15 @@ TRACE_EVENT(nvme_complete_rq,
__entry->result = le64_to_cpu(nvme_req(req)->result.u64);
__entry->retries = nvme_req(req)->retries;
__entry->flags = nvme_req(req)->flags;
+ __entry->disposition = disposition;
__entry->status = nvme_req(req)->status;
__assign_disk_name(__entry->disk, req->q->disk);
),
- TP_printk("nvme%d: %sqid=%d, cmdid=%u, res=%#llx,
retries=%u, flags=0x%x, status=%#x",
+ TP_printk("nvme%d: %sqid=%d, cmdid=%u, res=%#llx,
retries=%u, flags=0x%x, status=%#x entry->disposition=%x",
__entry->ctrl_id, __print_disk_name(__entry->disk),
__entry->qid, __entry->cid, __entry->result,
- __entry->retries, __entry->flags, __entry->status)
+ __entry->retries, __entry->flags, __entry->status,
+ __entry->disposition)
);
modprobe-9923 [038] ..s1. 1585.696843: nvme_complete_rq:
nvme0: qid=0, cmdid=4113, res=0x0, retries=0, flags=0x0, status=0x0
entry->disposition=0
[2] optional req disposition print :-
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index f586a4808e6e..832715676fd1 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -337,13 +337,6 @@ static void nvme_log_error(struct request *req)
nr->status & NVME_SC_DNR ? "DNR " : "");
}
-enum nvme_disposition {
- COMPLETE,
- RETRY,
- FAILOVER,
- AUTHENTICATE,
-};
-
static inline enum nvme_disposition nvme_decide_disposition(struct
request *req)
{
if (likely(nvme_req(req)->status == 0))
@@ -393,14 +386,16 @@ static inline void nvme_end_req(struct request *req)
void nvme_complete_rq(struct request *req)
{
struct nvme_ctrl *ctrl = nvme_req(req)->ctrl;
+ enum nvme_disposition disposition;
- trace_nvme_complete_rq(req);
nvme_cleanup_cmd(req);
if (ctrl->kas)
ctrl->comp_seen = true;
- switch (nvme_decide_disposition(req)) {
+ disposition = nvme_decide_disposition(req);
+ trace_nvme_complete_rq(req, disposition);
+ switch (disposition) {
case COMPLETE:
nvme_end_req(req);
return;
@@ -424,7 +419,7 @@ EXPORT_SYMBOL_GPL(nvme_complete_rq);
void nvme_complete_batch_req(struct request *req)
{
- trace_nvme_complete_rq(req);
+ trace_nvme_complete_rq(req, COMPLETE);
nvme_cleanup_cmd(req);
nvme_end_req_zoned(req);
}
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index f0a84e390a55..56e16bdd59ff 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -1119,4 +1119,12 @@ static inline const unsigned char
*nvme_opcode_str(int qid, u8 opcode, u8 fctype
return qid ? nvme_get_opcode_str(opcode) :
nvme_get_admin_opcode_str(opcode);
}
+
+enum nvme_disposition {
+ COMPLETE,
+ RETRY,
+ FAILOVER,
+ AUTHENTICATE,
+};
+
#endif /* _NVME_H */
diff --git a/drivers/nvme/host/trace.c b/drivers/nvme/host/trace.c
index 1c36fcedea20..ec32faa5080e 100644
--- a/drivers/nvme/host/trace.c
+++ b/drivers/nvme/host/trace.c
@@ -343,6 +343,18 @@ const char *nvme_trace_parse_fabrics_cmd(struct
trace_seq *p,
}
}
+const char *nvme_get_disposition(enum nvme_disposition disp)
+{
+ const char *disp_str[] = {
+ "",
+ " dispostion=RETRY",
+ " disposition=FAILOVER",
+ " disposition=AUTHENTICATE",
+ };
+
+ return disp > AUTHENTICATE ? "unknown" : disp_str[disp];
+}
+
const char *nvme_trace_disk_name(struct trace_seq *p, char *name)
{
const char *ret = trace_seq_buffer_ptr(p);
diff --git a/drivers/nvme/host/trace.h b/drivers/nvme/host/trace.h
index 4fb5922ffdac..f8b33f59ff76 100644
--- a/drivers/nvme/host/trace.h
+++ b/drivers/nvme/host/trace.h
@@ -22,6 +22,7 @@ const char *nvme_trace_parse_nvm_cmd(struct trace_seq
*p, u8 opcode,
u8 *cdw10);
const char *nvme_trace_parse_fabrics_cmd(struct trace_seq *p, u8 fctype,
u8 *spc);
+const char *nvme_get_disposition(enum nvme_disposition disp);
#define parse_nvme_cmd(qid, opcode, fctype, cdw10) \
((opcode) == nvme_fabrics_command ? \
@@ -83,8 +84,8 @@ TRACE_EVENT(nvme_setup_cmd,
);
TRACE_EVENT(nvme_complete_rq,
- TP_PROTO(struct request *req),
- TP_ARGS(req),
+ TP_PROTO(struct request *req, u8 disposition),
+ TP_ARGS(req, disposition),
TP_STRUCT__entry(
__array(char, disk, DISK_NAME_LEN)
__field(int, ctrl_id)
@@ -93,6 +94,7 @@ TRACE_EVENT(nvme_complete_rq,
__field(u64, result)
__field(u8, retries)
__field(u8, flags)
+ __field(u8, disposition)
__field(u16, status)
),
TP_fast_assign(
@@ -102,13 +104,15 @@ TRACE_EVENT(nvme_complete_rq,
__entry->result = le64_to_cpu(nvme_req(req)->result.u64);
__entry->retries = nvme_req(req)->retries;
__entry->flags = nvme_req(req)->flags;
+ __entry->disposition = disposition;
__entry->status = nvme_req(req)->status;
__assign_disk_name(__entry->disk, req->q->disk);
),
- TP_printk("nvme%d: %sqid=%d, cmdid=%u, res=%#llx,
retries=%u, flags=0x%x, status=%#x",
+ TP_printk("nvme%d: %sqid=%d, cmdid=%u, res=%#llx,
retries=%u, flags=0x%x, status=%#x%s",
__entry->ctrl_id, __print_disk_name(__entry->disk),
__entry->qid, __entry->cid, __entry->result,
- __entry->retries, __entry->flags, __entry->status)
+ __entry->retries, __entry->flags, __entry->status,
+ nvme_get_disposition(__entry->disposition))
);
^ permalink raw reply related [flat|nested] 6+ messages in thread
* RE: nvme host complete request tracing
2023-05-19 8:59 ` Chaitanya Kulkarni
@ 2023-05-19 9:18 ` Engel, Amit
2023-05-22 5:52 ` Chaitanya Kulkarni
0 siblings, 1 reply; 6+ messages in thread
From: Engel, Amit @ 2023-05-19 9:18 UTC (permalink / raw)
To: Chaitanya Kulkarni, Sagi Grimberg; +Cc: linux-nvme
Hi,
I would go with [2],
optionally printing disposition value only when disposition != COMPLETE in the trace_nvme_complete_rq()
Thanks,
Amit
-----Original Message-----
From: Chaitanya Kulkarni <chaitanyak@nvidia.com>
Sent: Friday, 19 May 2023 12:00
To: Sagi Grimberg <sagi@grimberg.me>
Cc: linux-nvme@lists.infradead.org; Engel, Amit <Amit.Engel@Dell.com>
Subject: Re: nvme host complete request tracing
[EXTERNAL EMAIL]
Sagi,
On 5/18/23 02:18, Sagi Grimberg wrote:
>
>> Hello,
>>
>> nvme/host/core.c trace_nvme_complete_rq() is being called immediately
>> when entering nvme_complete_rq() As I understand it, the idea behind
>> this trace is to trace every single nvme request that is being
>> completed.
>> but this is actually not accurate since there are several options for
>> a request as part of this function:
>> COMPLETE, RETRY, FAILOVER
>
> Well, it is completing this particular request.
>
>> In order to better understand the status per nvme request I think
>> it’s better to have a separate trace per each case, something like:
>> trace_nvme_end_rq(req), trace_nvme_retry_rq(req),
>> trace_nvme_failover_rq(req) etc.
>
> Maybe we can add the disposition to the trace?
>
> Something like:
> --
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index
> b374e6007553..eaf03ff61224 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -393,14 +393,17 @@ static inline void nvme_end_req(struct request
> *req)
> void nvme_complete_rq(struct request *req)
> {
> struct nvme_ctrl *ctrl = nvme_req(req)->ctrl;
> + enum nvme_disposition disposition;
>
> - trace_nvme_complete_rq(req);
> nvme_cleanup_cmd(req);
>
> if (ctrl->kas)
> ctrl->comp_seen = true;
>
> - switch (nvme_decide_disposition(req)) {
> + disposition = nvme_decide_disposition(req);
> + trace_nvme_complete_rq(req, disposition);
> +
> + switch (disposition) {
> case COMPLETE:
> nvme_end_req(req);
> return;
> --
>
Adding disposition to the nvme_complete_rq creates redundant
information since it will always be zero for status=0. Please have
a look at [1].
what do you think about optionally printing disposition value only
when disposition != COMPLETE in the trace_nvme_complete_rq() ?
see [2].
but if everyone is okay with redundant information I'll be happy to
send [1] :).
-ck
[1] duplicate request disposition information with unconditionally
printing disposition value :-
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index f586a4808e6e..9164fee13704 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -337,13 +337,6 @@ static void nvme_log_error(struct request *req)
nr->status & NVME_SC_DNR ? "DNR " : "");
}
-enum nvme_disposition {
- COMPLETE,
- RETRY,
- FAILOVER,
- AUTHENTICATE,
-};
-
static inline enum nvme_disposition nvme_decide_disposition(struct
request *req)
{
if (likely(nvme_req(req)->status == 0))
@@ -393,14 +386,16 @@ static inline void nvme_end_req(struct request *req)
void nvme_complete_rq(struct request *req)
{
struct nvme_ctrl *ctrl = nvme_req(req)->ctrl;
+ enum nvme_disposition disposition;
- trace_nvme_complete_rq(req);
nvme_cleanup_cmd(req);
if (ctrl->kas)
ctrl->comp_seen = true;
- switch (nvme_decide_disposition(req)) {
+ disposition = nvme_decide_disposition(req);
+ trace_nvme_complete_rq(req, disposition);
+ switch (disposition) {
case COMPLETE:
nvme_end_req(req);
return;
@@ -424,7 +419,7 @@ EXPORT_SYMBOL_GPL(nvme_complete_rq);
void nvme_complete_batch_req(struct request *req)
{
- trace_nvme_complete_rq(req);
+ trace_nvme_complete_rq(req, COMPLETE);
nvme_cleanup_cmd(req);
nvme_end_req_zoned(req);
}
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index f0a84e390a55..56e16bdd59ff 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -1119,4 +1119,12 @@ static inline const unsigned char
*nvme_opcode_str(int qid, u8 opcode, u8 fctype
return qid ? nvme_get_opcode_str(opcode) :
nvme_get_admin_opcode_str(opcode);
}
+
+enum nvme_disposition {
+ COMPLETE,
+ RETRY,
+ FAILOVER,
+ AUTHENTICATE,
+};
+
#endif /* _NVME_H */
diff --git a/drivers/nvme/host/trace.h b/drivers/nvme/host/trace.h
index 4fb5922ffdac..32290b058c0f 100644
--- a/drivers/nvme/host/trace.h
+++ b/drivers/nvme/host/trace.h
@@ -83,8 +83,8 @@ TRACE_EVENT(nvme_setup_cmd,
);
TRACE_EVENT(nvme_complete_rq,
- TP_PROTO(struct request *req),
- TP_ARGS(req),
+ TP_PROTO(struct request *req, u8 disposition),
+ TP_ARGS(req, disposition),
TP_STRUCT__entry(
__array(char, disk, DISK_NAME_LEN)
__field(int, ctrl_id)
@@ -93,6 +93,7 @@ TRACE_EVENT(nvme_complete_rq,
__field(u64, result)
__field(u8, retries)
__field(u8, flags)
+ __field(u8, disposition)
__field(u16, status)
),
TP_fast_assign(
@@ -102,13 +103,15 @@ TRACE_EVENT(nvme_complete_rq,
__entry->result = le64_to_cpu(nvme_req(req)->result.u64);
__entry->retries = nvme_req(req)->retries;
__entry->flags = nvme_req(req)->flags;
+ __entry->disposition = disposition;
__entry->status = nvme_req(req)->status;
__assign_disk_name(__entry->disk, req->q->disk);
),
- TP_printk("nvme%d: %sqid=%d, cmdid=%u, res=%#llx,
retries=%u, flags=0x%x, status=%#x",
+ TP_printk("nvme%d: %sqid=%d, cmdid=%u, res=%#llx,
retries=%u, flags=0x%x, status=%#x entry->disposition=%x",
__entry->ctrl_id, __print_disk_name(__entry->disk),
__entry->qid, __entry->cid, __entry->result,
- __entry->retries, __entry->flags, __entry->status)
+ __entry->retries, __entry->flags, __entry->status,
+ __entry->disposition)
);
modprobe-9923 [038] ..s1. 1585.696843: nvme_complete_rq:
nvme0: qid=0, cmdid=4113, res=0x0, retries=0, flags=0x0, status=0x0
entry->disposition=0
[2] optional req disposition print :-
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index f586a4808e6e..832715676fd1 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -337,13 +337,6 @@ static void nvme_log_error(struct request *req)
nr->status & NVME_SC_DNR ? "DNR " : "");
}
-enum nvme_disposition {
- COMPLETE,
- RETRY,
- FAILOVER,
- AUTHENTICATE,
-};
-
static inline enum nvme_disposition nvme_decide_disposition(struct
request *req)
{
if (likely(nvme_req(req)->status == 0))
@@ -393,14 +386,16 @@ static inline void nvme_end_req(struct request *req)
void nvme_complete_rq(struct request *req)
{
struct nvme_ctrl *ctrl = nvme_req(req)->ctrl;
+ enum nvme_disposition disposition;
- trace_nvme_complete_rq(req);
nvme_cleanup_cmd(req);
if (ctrl->kas)
ctrl->comp_seen = true;
- switch (nvme_decide_disposition(req)) {
+ disposition = nvme_decide_disposition(req);
+ trace_nvme_complete_rq(req, disposition);
+ switch (disposition) {
case COMPLETE:
nvme_end_req(req);
return;
@@ -424,7 +419,7 @@ EXPORT_SYMBOL_GPL(nvme_complete_rq);
void nvme_complete_batch_req(struct request *req)
{
- trace_nvme_complete_rq(req);
+ trace_nvme_complete_rq(req, COMPLETE);
nvme_cleanup_cmd(req);
nvme_end_req_zoned(req);
}
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index f0a84e390a55..56e16bdd59ff 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -1119,4 +1119,12 @@ static inline const unsigned char
*nvme_opcode_str(int qid, u8 opcode, u8 fctype
return qid ? nvme_get_opcode_str(opcode) :
nvme_get_admin_opcode_str(opcode);
}
+
+enum nvme_disposition {
+ COMPLETE,
+ RETRY,
+ FAILOVER,
+ AUTHENTICATE,
+};
+
#endif /* _NVME_H */
diff --git a/drivers/nvme/host/trace.c b/drivers/nvme/host/trace.c
index 1c36fcedea20..ec32faa5080e 100644
--- a/drivers/nvme/host/trace.c
+++ b/drivers/nvme/host/trace.c
@@ -343,6 +343,18 @@ const char *nvme_trace_parse_fabrics_cmd(struct
trace_seq *p,
}
}
+const char *nvme_get_disposition(enum nvme_disposition disp)
+{
+ const char *disp_str[] = {
+ "",
+ " dispostion=RETRY",
+ " disposition=FAILOVER",
+ " disposition=AUTHENTICATE",
+ };
+
+ return disp > AUTHENTICATE ? "unknown" : disp_str[disp];
+}
+
const char *nvme_trace_disk_name(struct trace_seq *p, char *name)
{
const char *ret = trace_seq_buffer_ptr(p);
diff --git a/drivers/nvme/host/trace.h b/drivers/nvme/host/trace.h
index 4fb5922ffdac..f8b33f59ff76 100644
--- a/drivers/nvme/host/trace.h
+++ b/drivers/nvme/host/trace.h
@@ -22,6 +22,7 @@ const char *nvme_trace_parse_nvm_cmd(struct trace_seq
*p, u8 opcode,
u8 *cdw10);
const char *nvme_trace_parse_fabrics_cmd(struct trace_seq *p, u8 fctype,
u8 *spc);
+const char *nvme_get_disposition(enum nvme_disposition disp);
#define parse_nvme_cmd(qid, opcode, fctype, cdw10) \
((opcode) == nvme_fabrics_command ? \
@@ -83,8 +84,8 @@ TRACE_EVENT(nvme_setup_cmd,
);
TRACE_EVENT(nvme_complete_rq,
- TP_PROTO(struct request *req),
- TP_ARGS(req),
+ TP_PROTO(struct request *req, u8 disposition),
+ TP_ARGS(req, disposition),
TP_STRUCT__entry(
__array(char, disk, DISK_NAME_LEN)
__field(int, ctrl_id)
@@ -93,6 +94,7 @@ TRACE_EVENT(nvme_complete_rq,
__field(u64, result)
__field(u8, retries)
__field(u8, flags)
+ __field(u8, disposition)
__field(u16, status)
),
TP_fast_assign(
@@ -102,13 +104,15 @@ TRACE_EVENT(nvme_complete_rq,
__entry->result = le64_to_cpu(nvme_req(req)->result.u64);
__entry->retries = nvme_req(req)->retries;
__entry->flags = nvme_req(req)->flags;
+ __entry->disposition = disposition;
__entry->status = nvme_req(req)->status;
__assign_disk_name(__entry->disk, req->q->disk);
),
- TP_printk("nvme%d: %sqid=%d, cmdid=%u, res=%#llx,
retries=%u, flags=0x%x, status=%#x",
+ TP_printk("nvme%d: %sqid=%d, cmdid=%u, res=%#llx,
retries=%u, flags=0x%x, status=%#x%s",
__entry->ctrl_id, __print_disk_name(__entry->disk),
__entry->qid, __entry->cid, __entry->result,
- __entry->retries, __entry->flags, __entry->status)
+ __entry->retries, __entry->flags, __entry->status,
+ nvme_get_disposition(__entry->disposition))
);
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: nvme host complete request tracing
2023-05-19 9:18 ` Engel, Amit
@ 2023-05-22 5:52 ` Chaitanya Kulkarni
2023-05-22 6:28 ` Sagi Grimberg
0 siblings, 1 reply; 6+ messages in thread
From: Chaitanya Kulkarni @ 2023-05-22 5:52 UTC (permalink / raw)
To: Sagi Grimberg; +Cc: Engel, Amit, linux-nvme
Sagi,
On 5/19/2023 2:18 AM, Engel, Amit wrote:
> Hi,
>
> I would go with [2],
> optionally printing disposition value only when disposition != COMPLETE in the trace_nvme_complete_rq()
>
> Thanks,
> Amit
> -----Original Message-----
> From: Chaitanya Kulkarni <chaitanyak@nvidia.com>
> Sent: Friday, 19 May 2023 12:00
> To: Sagi Grimberg <sagi@grimberg.me>
> Cc: linux-nvme@lists.infradead.org; Engel, Amit <Amit.Engel@Dell.com>
> Subject: Re: nvme host complete request tracing
>
Are you okay with this ?
-ck
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: nvme host complete request tracing
2023-05-22 5:52 ` Chaitanya Kulkarni
@ 2023-05-22 6:28 ` Sagi Grimberg
0 siblings, 0 replies; 6+ messages in thread
From: Sagi Grimberg @ 2023-05-22 6:28 UTC (permalink / raw)
To: Chaitanya Kulkarni; +Cc: Engel, Amit, linux-nvme
> Sagi,
>
> On 5/19/2023 2:18 AM, Engel, Amit wrote:
>> Hi,
>>
>> I would go with [2],
>> optionally printing disposition value only when disposition != COMPLETE in the trace_nvme_complete_rq()
>>
>> Thanks,
>> Amit
>> -----Original Message-----
>> From: Chaitanya Kulkarni <chaitanyak@nvidia.com>
>> Sent: Friday, 19 May 2023 12:00
>> To: Sagi Grimberg <sagi@grimberg.me>
>> Cc: linux-nvme@lists.infradead.org; Engel, Amit <Amit.Engel@Dell.com>
>> Subject: Re: nvme host complete request tracing
>>
>
> Are you okay with this ?
Yes.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-05-22 6:29 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-17 16:27 nvme host complete request tracing Engel, Amit
2023-05-18 9:18 ` Sagi Grimberg
2023-05-19 8:59 ` Chaitanya Kulkarni
2023-05-19 9:18 ` Engel, Amit
2023-05-22 5:52 ` Chaitanya Kulkarni
2023-05-22 6:28 ` Sagi Grimberg
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).