* [PATCH 2/5] nvme: trace nvme queue identifiers
2018-06-28 20:55 [PATCH 1/5] blk-mq: add API to get hctx index from request Keith Busch
@ 2018-06-28 20:55 ` Keith Busch
2018-06-28 20:55 ` [PATCH 3/5] nvme: cache struct nvme_ctrl reference to struct nvme_request Keith Busch
` (3 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Keith Busch @ 2018-06-28 20:55 UTC (permalink / raw)
To: Johannes Thumshirn, Christoph Hellwig, Sagi Grimberg, linux-nvme,
linux-kernel, Jens Axboe
Cc: Keith Busch
We can not match up a command to its completion based on the command
id alone. We need to pair this up with the submitting queue identifier,
so this patch adds that to the trace buffer.
This patch is also collapsing the admin and io submission traces into
a single one since we don't need to duplicate this and creating
unnecessary code branches.
Signed-off-by: Keith Busch <keith.busch@intel.com>
---
drivers/nvme/host/core.c | 7 +++----
drivers/nvme/host/trace.h | 41 ++++++++++-------------------------------
2 files changed, 13 insertions(+), 35 deletions(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 46df030b2c3f..398a5fb26603 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -97,6 +97,8 @@ static dev_t nvme_chr_devt;
static struct class *nvme_class;
static struct class *nvme_subsys_class;
+static void nvme_remove_invalid_namespaces(struct nvme_ctrl *ctrl,
+ unsigned nsid);
static void nvme_ns_remove(struct nvme_ns *ns);
static int nvme_revalidate_disk(struct gendisk *disk);
static void nvme_put_subsystem(struct nvme_subsystem *subsys);
@@ -652,10 +654,7 @@ blk_status_t nvme_setup_cmd(struct nvme_ns *ns, struct request *req,
}
cmd->common.command_id = req->tag;
- if (ns)
- trace_nvme_setup_nvm_cmd(req->q->id, cmd);
- else
- trace_nvme_setup_admin_cmd(cmd);
+ trace_nvme_setup_nvm_cmd(req, cmd);
return ret;
}
EXPORT_SYMBOL_GPL(nvme_setup_cmd);
diff --git a/drivers/nvme/host/trace.h b/drivers/nvme/host/trace.h
index 01390f0e1671..bca3451caa37 100644
--- a/drivers/nvme/host/trace.h
+++ b/drivers/nvme/host/trace.h
@@ -75,34 +75,9 @@ const char *nvme_trace_parse_nvm_cmd(struct trace_seq *p, u8 opcode,
#define __parse_nvme_cmd(opcode, cdw10) \
nvme_trace_parse_nvm_cmd(p, opcode, cdw10)
-TRACE_EVENT(nvme_setup_admin_cmd,
- TP_PROTO(struct nvme_command *cmd),
- TP_ARGS(cmd),
- TP_STRUCT__entry(
- __field(u8, opcode)
- __field(u8, flags)
- __field(u16, cid)
- __field(u64, metadata)
- __array(u8, cdw10, 24)
- ),
- TP_fast_assign(
- __entry->opcode = cmd->common.opcode;
- __entry->flags = cmd->common.flags;
- __entry->cid = cmd->common.command_id;
- __entry->metadata = le64_to_cpu(cmd->common.metadata);
- memcpy(__entry->cdw10, cmd->common.cdw10,
- sizeof(__entry->cdw10));
- ),
- TP_printk(" cmdid=%u, flags=0x%x, meta=0x%llx, cmd=(%s %s)",
- __entry->cid, __entry->flags, __entry->metadata,
- show_admin_opcode_name(__entry->opcode),
- __parse_nvme_admin_cmd(__entry->opcode, __entry->cdw10))
-);
-
-
TRACE_EVENT(nvme_setup_nvm_cmd,
- TP_PROTO(int qid, struct nvme_command *cmd),
- TP_ARGS(qid, cmd),
+ TP_PROTO(struct request *req, struct nvme_command *cmd),
+ TP_ARGS(req, cmd),
TP_STRUCT__entry(
__field(int, qid)
__field(u8, opcode)
@@ -113,7 +88,7 @@ TRACE_EVENT(nvme_setup_nvm_cmd,
__array(u8, cdw10, 24)
),
TP_fast_assign(
- __entry->qid = qid;
+ __entry->qid = blk_mq_request_hctx_idx(req) + !!req->rq_disk;
__entry->opcode = cmd->common.opcode;
__entry->flags = cmd->common.flags;
__entry->cid = cmd->common.command_id;
@@ -125,8 +100,12 @@ TRACE_EVENT(nvme_setup_nvm_cmd,
TP_printk("qid=%d, nsid=%u, cmdid=%u, flags=0x%x, meta=0x%llx, cmd=(%s %s)",
__entry->qid, __entry->nsid, __entry->cid,
__entry->flags, __entry->metadata,
- show_opcode_name(__entry->opcode),
- __parse_nvme_cmd(__entry->opcode, __entry->cdw10))
+ __entry->qid ?
+ show_opcode_name(__entry->opcode) :
+ show_admin_opcode_name(__entry->opcode),
+ __entry->qid ?
+ __parse_nvme_cmd(__entry->opcode, __entry->cdw10) :
+ __parse_nvme_admin_cmd(__entry->opcode, __entry->cdw10))
);
TRACE_EVENT(nvme_complete_rq,
@@ -141,7 +120,7 @@ TRACE_EVENT(nvme_complete_rq,
__field(u16, status)
),
TP_fast_assign(
- __entry->qid = req->q->id;
+ __entry->qid = blk_mq_request_hctx_idx(req) + !!req->rq_disk;
__entry->cid = req->tag;
__entry->result = le64_to_cpu(nvme_req(req)->result.u64);
__entry->retries = nvme_req(req)->retries;
--
2.14.3
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 3/5] nvme: cache struct nvme_ctrl reference to struct nvme_request
2018-06-28 20:55 [PATCH 1/5] blk-mq: add API to get hctx index from request Keith Busch
2018-06-28 20:55 ` [PATCH 2/5] nvme: trace nvme queue identifiers Keith Busch
@ 2018-06-28 20:55 ` Keith Busch
2018-06-28 20:56 ` [PATCH 4/5] nvme: trace controller name Keith Busch
` (2 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Keith Busch @ 2018-06-28 20:55 UTC (permalink / raw)
To: Johannes Thumshirn, Christoph Hellwig, Sagi Grimberg, linux-nvme,
linux-kernel, Jens Axboe
From: Sagi Grimberg <sagi@grimberg.me>
We will need to reference the controller in the setup and completion
time for tracing and future traffic based keep alive support.
Signed-off-by: Sagi Grimberg <sagi@grimberg.me>
---
drivers/nvme/host/fc.c | 1 +
drivers/nvme/host/nvme.h | 1 +
drivers/nvme/host/pci.c | 2 ++
drivers/nvme/host/rdma.c | 1 +
drivers/nvme/target/loop.c | 1 +
5 files changed, 6 insertions(+)
diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
index 41d45a1b5c62..9cc33752539a 100644
--- a/drivers/nvme/host/fc.c
+++ b/drivers/nvme/host/fc.c
@@ -1737,6 +1737,7 @@ nvme_fc_init_request(struct blk_mq_tag_set *set, struct request *rq,
int queue_idx = (set == &ctrl->tag_set) ? hctx_idx + 1 : 0;
struct nvme_fc_queue *queue = &ctrl->queues[queue_idx];
+ nvme_req(rq)->ctrl = &ctrl->ctrl;
return __nvme_fc_init_request(ctrl, queue, op, rq, queue->rqcnt++);
}
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 0c4a33df3b2f..f2249387b60d 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -102,6 +102,7 @@ struct nvme_request {
u8 retries;
u8 flags;
u16 status;
+ struct nvme_ctrl *ctrl;
};
/*
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index ba943f211687..8dcae11bbf3a 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -418,6 +418,8 @@ static int nvme_init_request(struct blk_mq_tag_set *set, struct request *req,
BUG_ON(!nvmeq);
iod->nvmeq = nvmeq;
+
+ nvme_req(req)->ctrl = &dev->ctrl;
return 0;
}
diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
index 9544625c0b7d..ade55977d432 100644
--- a/drivers/nvme/host/rdma.c
+++ b/drivers/nvme/host/rdma.c
@@ -286,6 +286,7 @@ static int nvme_rdma_init_request(struct blk_mq_tag_set *set,
struct ib_device *ibdev = dev->dev;
int ret;
+ nvme_req(rq)->ctrl = &ctrl->ctrl;
ret = nvme_rdma_alloc_qe(ibdev, &req->sqe, sizeof(struct nvme_command),
DMA_TO_DEVICE);
if (ret)
diff --git a/drivers/nvme/target/loop.c b/drivers/nvme/target/loop.c
index d8d91f04bd7e..af7fbf4132b0 100644
--- a/drivers/nvme/target/loop.c
+++ b/drivers/nvme/target/loop.c
@@ -227,6 +227,7 @@ static int nvme_loop_init_request(struct blk_mq_tag_set *set,
{
struct nvme_loop_ctrl *ctrl = set->driver_data;
+ nvme_req(req)->ctrl = &ctrl->ctrl;
return nvme_loop_init_iod(ctrl, blk_mq_rq_to_pdu(req),
(set == &ctrl->tag_set) ? hctx_idx + 1 : 0);
}
--
2.14.3
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 4/5] nvme: trace controller name
2018-06-28 20:55 [PATCH 1/5] blk-mq: add API to get hctx index from request Keith Busch
2018-06-28 20:55 ` [PATCH 2/5] nvme: trace nvme queue identifiers Keith Busch
2018-06-28 20:55 ` [PATCH 3/5] nvme: cache struct nvme_ctrl reference to struct nvme_request Keith Busch
@ 2018-06-28 20:56 ` Keith Busch
2018-06-28 20:56 ` [PATCH 5/5] nvme/trace: add disk name to tracepoints Keith Busch
2018-06-28 21:12 ` [PATCH 1/5] blk-mq: add API to get hctx index from request Bart Van Assche
4 siblings, 0 replies; 7+ messages in thread
From: Keith Busch @ 2018-06-28 20:56 UTC (permalink / raw)
To: Johannes Thumshirn, Christoph Hellwig, Sagi Grimberg, linux-nvme,
linux-kernel, Jens Axboe
Cc: Keith Busch
This appends the controller instance to the trace buffer for nvme commands
to distinguish what controller is dispatching a command.
Signed-off-by: Keith Busch <keith.busch@intel.com>
---
drivers/nvme/host/trace.h | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/drivers/nvme/host/trace.h b/drivers/nvme/host/trace.h
index bca3451caa37..57344980af16 100644
--- a/drivers/nvme/host/trace.h
+++ b/drivers/nvme/host/trace.h
@@ -79,6 +79,7 @@ TRACE_EVENT(nvme_setup_nvm_cmd,
TP_PROTO(struct request *req, struct nvme_command *cmd),
TP_ARGS(req, cmd),
TP_STRUCT__entry(
+ __field(int, ctrl_id)
__field(int, qid)
__field(u8, opcode)
__field(u8, flags)
@@ -88,6 +89,7 @@ TRACE_EVENT(nvme_setup_nvm_cmd,
__array(u8, cdw10, 24)
),
TP_fast_assign(
+ __entry->ctrl_id = nvme_req(req)->ctrl->instance;
__entry->qid = blk_mq_request_hctx_idx(req) + !!req->rq_disk;
__entry->opcode = cmd->common.opcode;
__entry->flags = cmd->common.flags;
@@ -97,9 +99,9 @@ TRACE_EVENT(nvme_setup_nvm_cmd,
memcpy(__entry->cdw10, cmd->common.cdw10,
sizeof(__entry->cdw10));
),
- TP_printk("qid=%d, nsid=%u, cmdid=%u, flags=0x%x, meta=0x%llx, cmd=(%s %s)",
- __entry->qid, __entry->nsid, __entry->cid,
- __entry->flags, __entry->metadata,
+ TP_printk("nvme%d: qid=%d, cmdid=%u, nsid=%u, flags=0x%x, meta=0x%llx, cmd=(%s %s)",
+ __entry->ctrl_id, __entry->qid, __entry->nsid,
+ __entry->cid, __entry->flags, __entry->metadata,
__entry->qid ?
show_opcode_name(__entry->opcode) :
show_admin_opcode_name(__entry->opcode),
@@ -112,6 +114,7 @@ TRACE_EVENT(nvme_complete_rq,
TP_PROTO(struct request *req),
TP_ARGS(req),
TP_STRUCT__entry(
+ __field(int, ctrl_id)
__field(int, qid)
__field(int, cid)
__field(u64, result)
@@ -120,6 +123,7 @@ TRACE_EVENT(nvme_complete_rq,
__field(u16, status)
),
TP_fast_assign(
+ __entry->ctrl_id = nvme_req(req)->ctrl->instance;
__entry->qid = blk_mq_request_hctx_idx(req) + !!req->rq_disk;
__entry->cid = req->tag;
__entry->result = le64_to_cpu(nvme_req(req)->result.u64);
@@ -127,9 +131,10 @@ TRACE_EVENT(nvme_complete_rq,
__entry->flags = nvme_req(req)->flags;
__entry->status = nvme_req(req)->status;
),
- TP_printk("qid=%d, cmdid=%u, res=%llu, retries=%u, flags=0x%x, status=%u",
- __entry->qid, __entry->cid, __entry->result,
- __entry->retries, __entry->flags, __entry->status)
+ TP_printk("nvme%d: qid=%d, cmdid=%u, res=%llu, retries=%u, flags=0x%x, status=%u",
+ __entry->ctrl_id, __entry->qid, __entry->cid,
+ __entry->result, __entry->retries, __entry->flags,
+ __entry->status)
);
--
2.14.3
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 5/5] nvme/trace: add disk name to tracepoints
2018-06-28 20:55 [PATCH 1/5] blk-mq: add API to get hctx index from request Keith Busch
` (2 preceding siblings ...)
2018-06-28 20:56 ` [PATCH 4/5] nvme: trace controller name Keith Busch
@ 2018-06-28 20:56 ` Keith Busch
2018-06-28 21:12 ` [PATCH 1/5] blk-mq: add API to get hctx index from request Bart Van Assche
4 siblings, 0 replies; 7+ messages in thread
From: Keith Busch @ 2018-06-28 20:56 UTC (permalink / raw)
To: Johannes Thumshirn, Christoph Hellwig, Sagi Grimberg, linux-nvme,
linux-kernel, Jens Axboe
Cc: Keith Busch
This will print the disk name if the disk name for disk based requests
so a user can better distinguish traffic to different disks. This allows
disk based filters. For example, to see only nvme0n2 traffic:
echo "disk_name == nvme0n2" > /sys/kernel/debug/tracing/events/nvme/filter
Signed-off-by: Keith Busch <keith.busch@intel.com>
---
drivers/nvme/host/trace.c | 11 +++++++++++
drivers/nvme/host/trace.h | 27 ++++++++++++++++++++-------
2 files changed, 31 insertions(+), 7 deletions(-)
diff --git a/drivers/nvme/host/trace.c b/drivers/nvme/host/trace.c
index 41944bbef835..796a5bcf67b8 100644
--- a/drivers/nvme/host/trace.c
+++ b/drivers/nvme/host/trace.c
@@ -128,3 +128,14 @@ const char *nvme_trace_parse_nvm_cmd(struct trace_seq *p,
return nvme_trace_common(p, cdw10);
}
}
+
+const char *nvme_trace_disk_name(struct trace_seq *p, char *name)
+{
+ const char *ret = trace_seq_buffer_ptr(p);
+
+ if (name[0] != 0)
+ trace_seq_printf(p, "disk=%-.*s, ", DISK_NAME_LEN, name);
+ trace_seq_putc(p, 0);
+
+ return ret;
+}
diff --git a/drivers/nvme/host/trace.h b/drivers/nvme/host/trace.h
index 57344980af16..e9c886f4e6d9 100644
--- a/drivers/nvme/host/trace.h
+++ b/drivers/nvme/host/trace.h
@@ -75,10 +75,15 @@ const char *nvme_trace_parse_nvm_cmd(struct trace_seq *p, u8 opcode,
#define __parse_nvme_cmd(opcode, cdw10) \
nvme_trace_parse_nvm_cmd(p, opcode, cdw10)
+const char *nvme_trace_disk_name(struct trace_seq *p, char *name);
+#define __print_nvme_disk_name(name) \
+ nvme_trace_disk_name(p, name)
+
TRACE_EVENT(nvme_setup_nvm_cmd,
TP_PROTO(struct request *req, struct nvme_command *cmd),
TP_ARGS(req, cmd),
TP_STRUCT__entry(
+ __array(char, disk_name, DISK_NAME_LEN)
__field(int, ctrl_id)
__field(int, qid)
__field(u8, opcode)
@@ -98,10 +103,14 @@ TRACE_EVENT(nvme_setup_nvm_cmd,
__entry->metadata = le64_to_cpu(cmd->common.metadata);
memcpy(__entry->cdw10, cmd->common.cdw10,
sizeof(__entry->cdw10));
+ req->rq_disk ?
+ memcpy(__entry->disk_name, req->rq_disk->disk_name, DISK_NAME_LEN) :
+ memset(__entry->disk_name, 0, DISK_NAME_LEN);
),
- TP_printk("nvme%d: qid=%d, cmdid=%u, nsid=%u, flags=0x%x, meta=0x%llx, cmd=(%s %s)",
- __entry->ctrl_id, __entry->qid, __entry->nsid,
- __entry->cid, __entry->flags, __entry->metadata,
+ TP_printk("nvme%d: %sqid=%d, cmdid=%u, nsid=%u, flags=0x%x, meta=0x%llx, cmd=(%s %s)",
+ __entry->ctrl_id, __print_nvme_disk_name(__entry->disk_name),
+ __entry->qid, __entry->nsid, __entry->cid,
+ __entry->flags, __entry->metadata,
__entry->qid ?
show_opcode_name(__entry->opcode) :
show_admin_opcode_name(__entry->opcode),
@@ -114,6 +123,7 @@ TRACE_EVENT(nvme_complete_rq,
TP_PROTO(struct request *req),
TP_ARGS(req),
TP_STRUCT__entry(
+ __array(char, disk_name, DISK_NAME_LEN)
__field(int, ctrl_id)
__field(int, qid)
__field(int, cid)
@@ -130,11 +140,14 @@ TRACE_EVENT(nvme_complete_rq,
__entry->retries = nvme_req(req)->retries;
__entry->flags = nvme_req(req)->flags;
__entry->status = nvme_req(req)->status;
+ req->rq_disk ?
+ memcpy(__entry->disk_name, req->rq_disk->disk_name, DISK_NAME_LEN) :
+ memset(__entry->disk_name, 0, DISK_NAME_LEN);
),
- TP_printk("nvme%d: qid=%d, cmdid=%u, res=%llu, retries=%u, flags=0x%x, status=%u",
- __entry->ctrl_id, __entry->qid, __entry->cid,
- __entry->result, __entry->retries, __entry->flags,
- __entry->status)
+ TP_printk("nvme%d: %sqid=%d, cmdid=%u, res=%llu, retries=%u, flags=0x%x, status=%u",
+ __entry->ctrl_id, __print_nvme_disk_name(__entry->disk_name),
+ __entry->qid, __entry->cid, __entry->result,
+ __entry->retries, __entry->flags, __entry->status)
);
--
2.14.3
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 1/5] blk-mq: add API to get hctx index from request
2018-06-28 20:55 [PATCH 1/5] blk-mq: add API to get hctx index from request Keith Busch
` (3 preceding siblings ...)
2018-06-28 20:56 ` [PATCH 5/5] nvme/trace: add disk name to tracepoints Keith Busch
@ 2018-06-28 21:12 ` Bart Van Assche
2018-06-28 21:19 ` Keith Busch
4 siblings, 1 reply; 7+ messages in thread
From: Bart Van Assche @ 2018-06-28 21:12 UTC (permalink / raw)
To: Keith Busch, Johannes Thumshirn, Christoph Hellwig, Sagi Grimberg,
linux-nvme@lists.infradead.org, linux-kernel@vger.kernel.org,
Jens Axboe
On 06/28/18 14:03, Keith Busch wrote:
> Signed-off-by: Keith Busch <keith.busch@intel.com>
> ---
> block/blk-mq.c | 6 ++++++
> include/linux/blk-mq.h | 1 +
> 2 files changed, 7 insertions(+)
>
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index b429d515b568..c6478833464d 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -466,6 +466,12 @@ struct request *blk_mq_alloc_request_hctx(struct request_queue *q,
> }
> EXPORT_SYMBOL_GPL(blk_mq_alloc_request_hctx);
>
> +unsigned int blk_mq_request_hctx_idx(struct request *rq)
> +{
> + return blk_mq_map_queue(rq->q, rq->mq_ctx->cpu)->queue_num;
> +}
> +EXPORT_SYMBOL_GPL(blk_mq_request_hctx_idx);
> +
> static void __blk_mq_free_request(struct request *rq)
> {
> struct request_queue *q = rq->q;
> diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
> index e3147eb74222..af91b2d31a04 100644
> --- a/include/linux/blk-mq.h
> +++ b/include/linux/blk-mq.h
> @@ -248,6 +248,7 @@ static inline u16 blk_mq_unique_tag_to_tag(u32 unique_tag)
> }
When posting a patch series, please include a cover letter that explains
the purpose of the patch series.
Regarding the above patch: have you considered to use the existing
function blk_mq_unique_tag_to_hwq() instead of introducing this new
function?
Thanks,
Bart.
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 1/5] blk-mq: add API to get hctx index from request
2018-06-28 21:12 ` [PATCH 1/5] blk-mq: add API to get hctx index from request Bart Van Assche
@ 2018-06-28 21:19 ` Keith Busch
0 siblings, 0 replies; 7+ messages in thread
From: Keith Busch @ 2018-06-28 21:19 UTC (permalink / raw)
To: Bart Van Assche
Cc: Keith Busch, Johannes Thumshirn, Christoph Hellwig, Sagi Grimberg,
linux-nvme@lists.infradead.org, linux-kernel@vger.kernel.org,
Jens Axboe
On Thu, Jun 28, 2018 at 02:12:38PM -0700, Bart Van Assche wrote:
> Regarding the above patch: have you considered to use the existing function
> blk_mq_unique_tag_to_hwq() instead of introducing this new function?
Interesting. I think the usage I need using that is something like:
blk_mq_unique_tag_to_hwq(blk_mq_unique_tag(req))
Sort of a round-about way to get to the hwq from the struct request,
but I'd rather not introduce anything new if there are other ways,
so thank you for the pointer.
^ permalink raw reply [flat|nested] 7+ messages in thread