2018-06-29 05:23:16

by Keith Busch

[permalink] [raw]
Subject: [PATCH 1/5] blk-mq: add API to get hctx index from request

Signed-off-by: Keith Busch <[email protected]>
---
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)
}


+unsigned int blk_mq_request_hctx_idx(struct request *rq);
int blk_mq_request_started(struct request *rq);
void blk_mq_start_request(struct request *rq);
void blk_mq_end_request(struct request *rq, blk_status_t error);
--
2.14.3



2018-06-28 21:10:39

by Keith Busch

[permalink] [raw]
Subject: [PATCH 5/5] nvme/trace: add disk name to tracepoints

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 <[email protected]>
---
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


2018-06-28 21:10:54

by Keith Busch

[permalink] [raw]
Subject: [PATCH 4/5] nvme: trace controller name

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 <[email protected]>
---
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


2018-06-28 21:16:28

by Bart Van Assche

[permalink] [raw]
Subject: Re: [PATCH 1/5] blk-mq: add API to get hctx index from request

On 06/28/18 14:03, Keith Busch wrote:
> Signed-off-by: Keith Busch <[email protected]>
> ---
> 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.

2018-06-28 21:21:47

by Keith Busch

[permalink] [raw]
Subject: Re: [PATCH 1/5] blk-mq: add API to get hctx index from request

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.

2018-06-29 05:21:48

by Keith Busch

[permalink] [raw]
Subject: [PATCH 3/5] nvme: cache struct nvme_ctrl reference to struct nvme_request

From: Sagi Grimberg <[email protected]>

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 <[email protected]>
---
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


2018-06-29 05:22:52

by Keith Busch

[permalink] [raw]
Subject: [PATCH 2/5] nvme: trace nvme queue identifiers

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 <[email protected]>
---
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