4:15 a.m.
Currently, crash doesn't support to display disk I/O statistics
for blk-mq devices. The patchset will implement it by parsing
the bitmap, it includes two patches:
[1] [PATCH 1/2] Support for "dev -d|-D" options by parsing bitmap in
blk-mq layer
[2] [PATCH 2/2] Fix for "dev -d|-D" options to correctly display disk I/O
statistics on linux v5.18-rc1
Related kernel commits:
[1] commit <9a14d6ce4135> ("block: remove debugfs blk_mq_ctx
dispatched/merged/completed attributes")
[2] commit <4e5cc99e1e48> ("blk-mq: manage hctx map via xarray")
The patchset is tested on v5.17.0-rc8 and the latest kernel 5.18.0-rc4.
With the patchset:
crash> dev -d
MAJOR GENDISK NAME REQUEST_QUEUE TOTAL ASYNC SYNC
8 ffff93d2ca593400 sda ffff93d2c917b960 18 15 3
Changes since v2:
[1] Remove the static_rqs[] and sched_tags to exactly follow up the
kernel path
[2] Add checking if the status of rq is inflight
[3] Use TRUE/FALSE for readability
[4] Replace "if MEMBER_EXISTS()" with "if VALID_MEMBER"
[5] Align it with the colons
Lianbo Jiang (2):
Support for "dev -d|-D" options by parsing bitmap in blk-mq layer
Fix for "dev -d|-D" options to correctly display disk I/O statistics
on linux v5.18-rc1
defs.h | 12 +++
dev.c | 285 ++++++++++++++++++++++++++++++++++++++++++++++--------
symbols.c | 24 +++++
3 files changed, 282 insertions(+), 39 deletions(-)
--
2.20.1
4:15 a.m.
New subject: [PATCH v3 1/2] Support for "dev -d|-D" options by parsing bitmap in blk-mq layer
Currently, crash doesn't support to display disk I/O statistics
for blk-mq devices. For more details, please refer to the following
commit: <98b417fc6346> ("Handle blk_mq_ctx member changes for kernels
5.16-rc1 and later").
Lets parse the bitmap in blk-mq layer to achieve it.
Signed-off-by: Lianbo Jiang <lijiang(a)redhat.com>
---
defs.h | 11 +++
dev.c | 261 ++++++++++++++++++++++++++++++++++++++++++++++--------
symbols.c | 22 +++++
3 files changed, 255 insertions(+), 39 deletions(-)
diff --git a/defs.h b/defs.h
index c8444b4e54eb..2681586a33dc 100644
--- a/defs.h
+++ b/defs.h
@@ -2170,6 +2170,16 @@ struct offset_table { /* stash of commonly-used
offsets */
long sbq_wait_state_wait;
long sbitmap_alloc_hint;
long sbitmap_round_robin;
+ long request_cmd_flags;
+ long request_q;
+ long request_state;
+ long request_queue_queue_hw_ctx;
+ long request_queue_nr_hw_queues;
+ long blk_mq_hw_ctx_tags;
+ long blk_mq_tags_bitmap_tags;
+ long blk_mq_tags_breserved_tags;
+ long blk_mq_tags_nr_reserved_tags;
+ long blk_mq_tags_rqs;
};
struct size_table { /* stash of commonly-used sizes */
@@ -2339,6 +2349,7 @@ struct size_table { /* stash of commonly-used sizes */
long sbitmap;
long sbitmap_queue;
long sbq_wait_state;
+ long blk_mq_tags;
};
struct array_table {
diff --git a/dev.c b/dev.c
index a493e51ac95c..c0240f7b889b 100644
--- a/dev.c
+++ b/dev.c
@@ -4238,19 +4238,193 @@ get_one_mctx_diskio(unsigned long mctx, struct diskio *io)
io->write = (dispatch[1] - comp[1]);
}
+typedef bool (busy_tag_iter_fn)(ulong rq, void *data);
+
+struct bt_iter_data {
+ ulong q;
+ struct diskio *dio;
+};
+
+struct bt_tags_iter_data {
+ ulong tags;
+ uint flags;
+ uint nr_reserved_tags;
+ busy_tag_iter_fn *fn;
+ void *data;
+};
+
+/*
+ * See the include/linux/blk_types.h and include/linux/blk-mq.h
+ */
+#define MQ_RQ_IN_FLIGHT 1
+#define REQ_OP_BITS 8
+#define REQ_OP_MASK ((1 << REQ_OP_BITS) - 1)
+
+static uint op_is_write(uint op)
+{
+ return (op & REQ_OP_MASK) & 1;
+}
+
+static bool mq_check_inflight(ulong rq, void *data)
+{
+ uint cmd_flags = 0, state = 0;
+ ulong addr = 0, queue = 0;
+ struct bt_iter_data *iter = data;
+
+ if (!IS_KVADDR(rq))
+ return TRUE;
+
+ addr = rq + OFFSET(request_q);
+ if (!readmem(addr, KVADDR, &queue, sizeof(ulong), "request.q",
RETURN_ON_ERROR))
+ return FALSE;
+
+ addr = rq + OFFSET(request_cmd_flags);
+ if (!readmem(addr, KVADDR, &cmd_flags, sizeof(uint), "request.cmd_flags",
RETURN_ON_ERROR))
+ return FALSE;
+
+ addr = rq + OFFSET(request_state);
+ if (!readmem(addr, KVADDR, &state, sizeof(uint), "request.state",
RETURN_ON_ERROR))
+ return FALSE;
+
+ if (queue == iter->q && state == MQ_RQ_IN_FLIGHT) {
+ if (op_is_write(cmd_flags))
+ iter->dio->write++;
+ else
+ iter->dio->read++;
+ }
+
+ return TRUE;
+}
+
+static bool bt_tags_iter(uint bitnr, void *data)
+{
+ ulong addr = 0, rqs_addr = 0, rq = 0;
+ struct bt_tags_iter_data *tag_iter = data;
+ ulong tag = tag_iter->tags;
+
+ if (!tag_iter->flags)
+ bitnr += tag_iter->nr_reserved_tags;
+
+ /* rqs */
+ addr = tag + OFFSET(blk_mq_tags_rqs);
+ if (!readmem(addr, KVADDR, &rqs_addr, sizeof(void *), "blk_mq_tags.rqs",
RETURN_ON_ERROR))
+ return FALSE;
+
+ addr = rqs_addr + bitnr * sizeof(ulong); /* rqs[bitnr] */
+ if (!readmem(addr, KVADDR, &rq, sizeof(ulong), "blk_mq_tags.rqs[]",
RETURN_ON_ERROR))
+ return FALSE;
+
+ return tag_iter->fn(rq, tag_iter->data);
+}
+
+static void bt_tags_for_each(ulong q, ulong tags, ulong sbq, uint flags, uint
nr_resvd_tags, struct diskio *dio)
+{
+ struct sbitmap_context sc = {0};
+ struct diskio tmp = {0};
+ struct bt_iter_data iter_data = {
+ .q = q,
+ .dio = &tmp,
+ };
+ struct bt_tags_iter_data tags_iter_data = {
+ .tags = tags,
+ .flags = flags,
+ .nr_reserved_tags = nr_resvd_tags,
+ .fn = mq_check_inflight,
+ .data = &iter_data,
+ };
+
+ sbitmap_context_load(sbq + OFFSET(sbitmap_queue_sb), &sc);
+ sbitmap_for_each_set(&sc, bt_tags_iter, &tags_iter_data);
+
+ dio->read = tmp.read;
+ dio->write = tmp.write;
+}
+
+static void queue_for_each_hw_ctx(ulong q, ulong *hctx, uint cnt, struct diskio *dio)
+{
+ uint i;
+ struct diskio tmp = {0};
+
+ if (!hctx || !dio)
+ error(FATAL, "%s: Invalid parameter.\n", __func__);
+
+ for (i = 0; i < cnt; i++) {
+ ulong addr = 0, tags = 0;
+ uint nr_reserved_tags = 0;
+
+ if (!IS_KVADDR(hctx[i]))
+ continue;
+
+ /* Tags owned by the block driver */
+ addr = hctx[i] + OFFSET(blk_mq_hw_ctx_tags);
+ if (!readmem(addr, KVADDR, &tags, sizeof(ulong),
+ "blk_mq_hw_ctx.tags", RETURN_ON_ERROR))
+ break;
+
+ addr = tags + OFFSET(blk_mq_tags_nr_reserved_tags);
+ if (!readmem(addr, KVADDR, &nr_reserved_tags, sizeof(uint),
+ "blk_mq_tags_nr_reserved_tags", RETURN_ON_ERROR))
+ break;
+
+ if (nr_reserved_tags) {
+ addr = tags + OFFSET(blk_mq_tags_breserved_tags);
+ bt_tags_for_each(q, tags, addr, 1, nr_reserved_tags, &tmp);
+ }
+ addr = tags + OFFSET(blk_mq_tags_bitmap_tags);
+ bt_tags_for_each(q, tags, addr, 0, nr_reserved_tags, &tmp);
+ }
+
+ dio->read = tmp.read;
+ dio->write = tmp.write;
+}
+
+static void get_mq_diskio_from_hw_queues(ulong q, struct diskio *dio)
+{
+ uint cnt = 0;
+ ulong addr = 0, hctx_addr = 0;
+ ulong *hctx_array = NULL;
+ struct diskio tmp = {0};
+
+ addr = q + OFFSET(request_queue_nr_hw_queues);
+ readmem(addr, KVADDR, &cnt, sizeof(uint),
+ "request_queue.nr_hw_queues", FAULT_ON_ERROR);
+
+ addr = q + OFFSET(request_queue_queue_hw_ctx);
+ readmem(addr, KVADDR, &hctx_addr, sizeof(void *),
+ "request_queue.queue_hw_ctx", FAULT_ON_ERROR);
+
+ hctx_array = (ulong *)GETBUF(sizeof(void *) * cnt);
+ if (!hctx_array)
+ error(FATAL, "fail to get memory for the hctx_array\n");
+
+ if (!readmem(hctx_addr, KVADDR, hctx_array, sizeof(void *) * cnt,
+ "request_queue.queue_hw_ctx[]", RETURN_ON_ERROR)) {
+ FREEBUF(hctx_array);
+ return;
+ }
+
+ queue_for_each_hw_ctx(q, hctx_array, cnt, &tmp);
+ dio->read = tmp.read;
+ dio->write = tmp.write;
+
+ FREEBUF(hctx_array);
+}
+
static void
get_mq_diskio(unsigned long q, unsigned long *mq_count)
{
int cpu;
unsigned long queue_ctx;
unsigned long mctx_addr;
- struct diskio tmp;
+ struct diskio tmp = {0};
if (INVALID_MEMBER(blk_mq_ctx_rq_dispatched) ||
- INVALID_MEMBER(blk_mq_ctx_rq_completed))
+ INVALID_MEMBER(blk_mq_ctx_rq_completed)) {
+ get_mq_diskio_from_hw_queues(q, &tmp);
+ mq_count[0] = tmp.read;
+ mq_count[1] = tmp.write;
return;
-
- memset(&tmp, 0x00, sizeof(struct diskio));
+ }
readmem(q + OFFSET(request_queue_queue_ctx), KVADDR, &queue_ctx,
sizeof(ulong), "request_queue.queue_ctx",
@@ -4479,41 +4653,24 @@ display_one_diskio(struct iter *i, unsigned long gendisk, ulong
flags)
&& (io.read + io.write == 0))
return;
- if (use_mq_interface(queue_addr) &&
- (INVALID_MEMBER(blk_mq_ctx_rq_dispatched) ||
- INVALID_MEMBER(blk_mq_ctx_rq_completed)))
- fprintf(fp, "%s%s%s %s%s%s%s %s%s%s",
- mkstring(buf0, 5, RJUST|INT_DEC, (char *)(unsigned long)major),
- space(MINSPACE),
- mkstring(buf1, VADDR_PRLEN, LJUST|LONG_HEX, (char *)gendisk),
- space(MINSPACE),
- mkstring(buf2, 10, LJUST, disk_name),
- space(MINSPACE),
- mkstring(buf3, VADDR_PRLEN <= 11 ? 11 : VADDR_PRLEN,
- LJUST|LONG_HEX, (char *)queue_addr),
- space(MINSPACE),
- mkstring(buf4, 17, RJUST, "(not supported)"),
- space(MINSPACE));
-
- else
- fprintf(fp, "%s%s%s %s%s%s%s %s%5d%s%s%s%s%s",
- mkstring(buf0, 5, RJUST|INT_DEC, (char *)(unsigned long)major),
- space(MINSPACE),
- mkstring(buf1, VADDR_PRLEN, LJUST|LONG_HEX, (char *)gendisk),
- space(MINSPACE),
- mkstring(buf2, 10, LJUST, disk_name),
- space(MINSPACE),
- mkstring(buf3, VADDR_PRLEN <= 11 ? 11 : VADDR_PRLEN,
- LJUST|LONG_HEX, (char *)queue_addr),
- space(MINSPACE),
- io.read + io.write,
- space(MINSPACE),
- mkstring(buf4, 5, RJUST|INT_DEC,
- (char *)(unsigned long)io.read),
- space(MINSPACE),
- mkstring(buf5, 5, RJUST|INT_DEC,
- (char *)(unsigned long)io.write),
- space(MINSPACE));
+ fprintf(fp, "%s%s%s %s%s%s%s %s%5d%s%s%s%s%s",
+ mkstring(buf0, 5, RJUST|INT_DEC, (char *)(unsigned long)major),
+ space(MINSPACE),
+ mkstring(buf1, VADDR_PRLEN, LJUST|LONG_HEX, (char *)gendisk),
+ space(MINSPACE),
+ mkstring(buf2, 10, LJUST, disk_name),
+ space(MINSPACE),
+ mkstring(buf3, VADDR_PRLEN <= 11 ? 11 : VADDR_PRLEN,
+ LJUST|LONG_HEX, (char *)queue_addr),
+ space(MINSPACE),
+ io.read + io.write,
+ space(MINSPACE),
+ mkstring(buf4, 5, RJUST|INT_DEC,
+ (char *)(unsigned long)io.read),
+ space(MINSPACE),
+ mkstring(buf5, 5, RJUST|INT_DEC,
+ (char *)(unsigned long)io.write),
+ space(MINSPACE));
if (VALID_MEMBER(request_queue_in_flight)) {
if (!use_mq_interface(queue_addr)) {
@@ -4597,6 +4754,9 @@ void diskio_init(void)
MEMBER_OFFSET_INIT(kobject_entry, "kobject", "entry");
MEMBER_OFFSET_INIT(kset_list, "kset", "list");
MEMBER_OFFSET_INIT(request_list_count, "request_list", "count");
+ MEMBER_OFFSET_INIT(request_cmd_flags, "request", "cmd_flags");
+ MEMBER_OFFSET_INIT(request_q, "request", "q");
+ MEMBER_OFFSET_INIT(request_state, "request", "state");
MEMBER_OFFSET_INIT(request_queue_in_flight, "request_queue",
"in_flight");
if (MEMBER_EXISTS("request_queue", "rq"))
@@ -4608,10 +4768,33 @@ void diskio_init(void)
"mq_ops");
ANON_MEMBER_OFFSET_INIT(request_queue_queue_ctx,
"request_queue", "queue_ctx");
+ MEMBER_OFFSET_INIT(request_queue_queue_hw_ctx,
+ "request_queue", "queue_hw_ctx");
+ MEMBER_OFFSET_INIT(request_queue_nr_hw_queues,
+ "request_queue", "nr_hw_queues");
MEMBER_OFFSET_INIT(blk_mq_ctx_rq_dispatched, "blk_mq_ctx",
"rq_dispatched");
MEMBER_OFFSET_INIT(blk_mq_ctx_rq_completed, "blk_mq_ctx",
"rq_completed");
+ MEMBER_OFFSET_INIT(blk_mq_hw_ctx_tags, "blk_mq_hw_ctx", "tags");
+ MEMBER_OFFSET_INIT(blk_mq_tags_bitmap_tags, "blk_mq_tags",
+ "bitmap_tags");
+ MEMBER_OFFSET_INIT(blk_mq_tags_breserved_tags, "blk_mq_tags",
+ "breserved_tags");
+ MEMBER_OFFSET_INIT(blk_mq_tags_nr_reserved_tags, "blk_mq_tags",
+ "nr_reserved_tags");
+ MEMBER_OFFSET_INIT(blk_mq_tags_rqs, "blk_mq_tags", "rqs");
+ STRUCT_SIZE_INIT(blk_mq_tags, "blk_mq_tags");
+ STRUCT_SIZE_INIT(sbitmap, "sbitmap");
+ STRUCT_SIZE_INIT(sbitmap_word, "sbitmap_word");
+ MEMBER_OFFSET_INIT(sbitmap_word_word, "sbitmap_word", "word");
+ MEMBER_OFFSET_INIT(sbitmap_word_cleared, "sbitmap_word",
"cleared");
+ MEMBER_OFFSET_INIT(sbitmap_depth, "sbitmap", "depth");
+ MEMBER_OFFSET_INIT(sbitmap_shift, "sbitmap", "shift");
+ MEMBER_OFFSET_INIT(sbitmap_map_nr, "sbitmap", "map_nr");
+ MEMBER_OFFSET_INIT(sbitmap_map, "sbitmap", "map");
+ MEMBER_OFFSET_INIT(sbitmap_queue_sb, "sbitmap_queue", "sb");
+
}
MEMBER_OFFSET_INIT(subsys_private_klist_devices, "subsys_private",
"klist_devices");
diff --git a/symbols.c b/symbols.c
index 5d12a021c769..c1f09556d710 100644
--- a/symbols.c
+++ b/symbols.c
@@ -10385,6 +10385,12 @@ dump_offset_table(char *spec, ulong makestruct)
OFFSET(kset_list));
fprintf(fp, " request_list_count: %ld\n",
OFFSET(request_list_count));
+ fprintf(fp, " request_cmd_flags: %ld\n",
+ OFFSET(request_cmd_flags));
+ fprintf(fp, " request_q: %ld\n",
+ OFFSET(request_q));
+ fprintf(fp, " request_state: %ld\n",
+ OFFSET(request_state));
fprintf(fp, " request_queue_in_flight: %ld\n",
OFFSET(request_queue_in_flight));
fprintf(fp, " request_queue_rq: %ld\n",
@@ -10393,10 +10399,25 @@ dump_offset_table(char *spec, ulong makestruct)
OFFSET(request_queue_mq_ops));
fprintf(fp, " request_queue_queue_ctx: %ld\n",
OFFSET(request_queue_queue_ctx));
+ fprintf(fp, " request_queue_queue_hw_ctx: %ld\n",
+ OFFSET(request_queue_queue_hw_ctx));
+ fprintf(fp, " request_queue_nr_hw_queues: %ld\n",
+ OFFSET(request_queue_nr_hw_queues));
fprintf(fp, " blk_mq_ctx_rq_dispatched: %ld\n",
OFFSET(blk_mq_ctx_rq_dispatched));
fprintf(fp, " blk_mq_ctx_rq_completed: %ld\n",
OFFSET(blk_mq_ctx_rq_completed));
+ fprintf(fp, " blk_mq_hw_ctx_tags: %ld\n",
+ OFFSET(blk_mq_hw_ctx_tags));
+ fprintf(fp, " blk_mq_tags_bitmap_tags: %ld\n",
+ OFFSET(blk_mq_tags_bitmap_tags));
+ fprintf(fp, " blk_mq_tags_breserved_tags: %ld\n",
+ OFFSET(blk_mq_tags_breserved_tags));
+ fprintf(fp, " blk_mq_tags_nr_reserved_tags: %ld\n",
+ OFFSET(blk_mq_tags_nr_reserved_tags));
+ fprintf(fp, " blk_mq_tags_rqs: %ld\n",
+ OFFSET(blk_mq_tags_rqs));
+
fprintf(fp, " subsys_private_klist_devices: %ld\n",
OFFSET(subsys_private_klist_devices));
fprintf(fp, " subsystem_kset: %ld\n",
@@ -11003,6 +11024,7 @@ dump_offset_table(char *spec, ulong makestruct)
fprintf(fp, " sbitmap: %ld\n", SIZE(sbitmap));
fprintf(fp, " sbitmap_queue: %ld\n", SIZE(sbitmap_queue));
fprintf(fp, " sbq_wait_state: %ld\n", SIZE(sbq_wait_state));
+ fprintf(fp, " blk_mq_tags: %ld\n", SIZE(blk_mq_tags));
fprintf(fp, "\n array_table:\n");
/*
--
2.20.1
3:51 a.m.
New subject: [PATCH v3 1/2] Support for "dev -d|-D" options by parsing bitmap in blk-mq layer
Hi Lianbo,
Thank you for the update, much better structure :)
Let's go into detail...
On 2022/05/30 18:15, Lianbo Jiang wrote:
Currently, crash doesn't support to display disk I/O statistics
for blk-mq devices. For more details, please refer to the following
commit: <98b417fc6346> ("Handle blk_mq_ctx member changes for kernels
5.16-rc1 and later").
Lets parse the bitmap in blk-mq layer to achieve it.
Signed-off-by: Lianbo Jiang <lijiang(a)redhat.com>
---
defs.h | 11 +++
dev.c | 261 ++++++++++++++++++++++++++++++++++++++++++++++--------
symbols.c | 22 +++++
3 files changed, 255 insertions(+), 39 deletions(-)
diff --git a/defs.h b/defs.h
index c8444b4e54eb..2681586a33dc 100644
--- a/defs.h
+++ b/defs.h
@@ -2170,6 +2170,16 @@ struct offset_table { /* stash of commonly-used
offsets */
long sbq_wait_state_wait;
long sbitmap_alloc_hint;
long sbitmap_round_robin;
+ long request_cmd_flags;
+ long request_q;
+ long request_state;
+ long request_queue_queue_hw_ctx;
+ long request_queue_nr_hw_queues;
+ long blk_mq_hw_ctx_tags;
+ long blk_mq_tags_bitmap_tags;
+ long blk_mq_tags_breserved_tags;
+ long blk_mq_tags_nr_reserved_tags;
+ long blk_mq_tags_rqs;
};
struct size_table { /* stash of commonly-used sizes */
@@ -2339,6 +2349,7 @@ struct size_table { /* stash of commonly-used sizes */
long sbitmap;
long sbitmap_queue;
long sbq_wait_state;
+ long blk_mq_tags;
};
struct array_table {
diff --git a/dev.c b/dev.c
index a493e51ac95c..c0240f7b889b 100644
--- a/dev.c
+++ b/dev.c
@@ -4238,19 +4238,193 @@ get_one_mctx_diskio(unsigned long mctx, struct diskio *io)
io->write = (dispatch[1] - comp[1]);
}
+typedef bool (busy_tag_iter_fn)(ulong rq, void *data);
+
+struct bt_iter_data {
I think that some names are confusing and not synced with the kernel.
How about renaming to "mq_inflight"?
+ ulong q;
+ struct diskio *dio;
+};
+
+struct bt_tags_iter_data {
And please rename this to "bt_iter_data".
+ ulong tags;
+ uint flags;
rename to "reserved".
+ uint nr_reserved_tags;
+ busy_tag_iter_fn *fn;
+ void *data;
+};
+
+/*
+ * See the include/linux/blk_types.h and include/linux/blk-mq.h
+ */
+#define MQ_RQ_IN_FLIGHT 1
+#define REQ_OP_BITS 8
+#define REQ_OP_MASK ((1 << REQ_OP_BITS) - 1)
+
+static uint op_is_write(uint op)
+{
+ return (op & REQ_OP_MASK) & 1;
+}
+
+static bool mq_check_inflight(ulong rq, void *data)
+{
+ uint cmd_flags = 0, state = 0;
+ ulong addr = 0, queue = 0;
+ struct bt_iter_data *iter = data;
+
+ if (!IS_KVADDR(rq))
+ return TRUE;
Why return TRUE?
Does this mean that rq can be invalid with bit on?
+
+ addr = rq + OFFSET(request_q);
+ if (!readmem(addr, KVADDR, &queue, sizeof(ulong), "request.q",
RETURN_ON_ERROR))
+ return FALSE;
+
+ addr = rq + OFFSET(request_cmd_flags);
+ if (!readmem(addr, KVADDR, &cmd_flags, sizeof(uint), "request.cmd_flags",
RETURN_ON_ERROR))
+ return FALSE;
+
+ addr = rq + OFFSET(request_state);
+ if (!readmem(addr, KVADDR, &state, sizeof(uint), "request.state",
RETURN_ON_ERROR))
+ return FALSE;
Rethinking these, RETURN_ON_ERROR leads to stop counting (i.e. wrong results)
with error messages. I think it would be better to use FAULT_ON_ERROR unless
we can ignore or handle an error correctly. What do you think?
(And for the RETURN_ON_ERRORs below, same as above.)
+
+ if (queue == iter->q && state == MQ_RQ_IN_FLIGHT) {
+ if (op_is_write(cmd_flags))
+ iter->dio->write++;
+ else
+ iter->dio->read++;
+ }
+
+ return TRUE;
+}
+
+static bool bt_tags_iter(uint bitnr, void *data)
+{
+ ulong addr = 0, rqs_addr = 0, rq = 0;
+ struct bt_tags_iter_data *tag_iter = data;
+ ulong tag = tag_iter->tags;
+
+ if (!tag_iter->flags)
+ bitnr += tag_iter->nr_reserved_tags;
+
+ /* rqs */
+ addr = tag + OFFSET(blk_mq_tags_rqs);
+ if (!readmem(addr, KVADDR, &rqs_addr, sizeof(void *), "blk_mq_tags.rqs",
RETURN_ON_ERROR))
+ return FALSE;
+
+ addr = rqs_addr + bitnr * sizeof(ulong); /* rqs[bitnr] */
+ if (!readmem(addr, KVADDR, &rq, sizeof(ulong), "blk_mq_tags.rqs[]",
RETURN_ON_ERROR))
+ return FALSE;
+
+ return tag_iter->fn(rq, tag_iter->data);
+}
+
+static void bt_tags_for_each(ulong q, ulong tags, ulong sbq, uint flags, uint
nr_resvd_tags, struct diskio *dio)
Please rename this to bt_for_each.
+{
+ struct sbitmap_context sc = {0};
+ struct diskio tmp = {0};
This tmp can be removed?
(The dio->{read,write} are incremented in mq_check_inflight(), so
we don't need the local tmps for each function, I think)
+ struct bt_iter_data iter_data = {
+ .q = q,
+ .dio = &tmp,
+ };
+ struct bt_tags_iter_data tags_iter_data = {
+ .tags = tags,
+ .flags = flags,
+ .nr_reserved_tags = nr_resvd_tags,
+ .fn = mq_check_inflight,
+ .data = &iter_data,
+ };
+
+ sbitmap_context_load(sbq + OFFSET(sbitmap_queue_sb), &sc);
+ sbitmap_for_each_set(&sc, bt_tags_iter, &tags_iter_data);
+
+ dio->read = tmp.read;
+ dio->write = tmp.write;
+}
+
+static void queue_for_each_hw_ctx(ulong q, ulong *hctx, uint cnt, struct diskio *dio)
+{
+ uint i;
+ struct diskio tmp = {0};
This also can be removed.
+
+ if (!hctx || !dio)
+ error(FATAL, "%s: Invalid parameter.\n", __func__);
This can be used?
+
+ for (i = 0; i < cnt; i++) {
+ ulong addr = 0, tags = 0;
+ uint nr_reserved_tags = 0;
+
+ if (!IS_KVADDR(hctx[i]))
+ continue;
Is this the remain of the v1 patch?
+
+ /* Tags owned by the block driver */
+ addr = hctx[i] + OFFSET(blk_mq_hw_ctx_tags);
+ if (!readmem(addr, KVADDR, &tags, sizeof(ulong),
+ "blk_mq_hw_ctx.tags", RETURN_ON_ERROR))
+ break;
+
+ addr = tags + OFFSET(blk_mq_tags_nr_reserved_tags);
+ if (!readmem(addr, KVADDR, &nr_reserved_tags, sizeof(uint),
+ "blk_mq_tags_nr_reserved_tags", RETURN_ON_ERROR))
+ break;
+
+ if (nr_reserved_tags) {
+ addr = tags + OFFSET(blk_mq_tags_breserved_tags);
+ bt_tags_for_each(q, tags, addr, 1, nr_reserved_tags, &tmp);
+ }
+ addr = tags + OFFSET(blk_mq_tags_bitmap_tags);
+ bt_tags_for_each(q, tags, addr, 0, nr_reserved_tags, &tmp);
(the local tmp set by the nr_reserved_tags case is overwritten here.)
+ }
+
+ dio->read = tmp.read;
+ dio->write = tmp.write;
+}
+
+static void get_mq_diskio_from_hw_queues(ulong q, struct diskio *dio)
+{
+ uint cnt = 0;
+ ulong addr = 0, hctx_addr = 0;
+ ulong *hctx_array = NULL;
+ struct diskio tmp = {0};
This also can be removed.
> +
> + addr = q + OFFSET(request_queue_nr_hw_queues);
> + readmem(addr, KVADDR, &cnt, sizeof(uint),
> + "request_queue.nr_hw_queues", FAULT_ON_ERROR);
> +
> + addr = q + OFFSET(request_queue_queue_hw_ctx);
> + readmem(addr, KVADDR, &hctx_addr, sizeof(void *),
> + "request_queue.queue_hw_ctx", FAULT_ON_ERROR);
> +
> + hctx_array = (ulong *)GETBUF(sizeof(void *) * cnt);
> + if (!hctx_array)
> + error(FATAL, "fail to get memory for the hctx_array\n");
> +
> + if (!readmem(hctx_addr, KVADDR, hctx_array, sizeof(void *) * cnt,
> + "request_queue.queue_hw_ctx[]", RETURN_ON_ERROR)) {
> + FREEBUF(hctx_array);
> + return;
> + }
> +
> + queue_for_each_hw_ctx(q, hctx_array, cnt, &tmp);
> + dio->read = tmp.read;
> + dio->write = tmp.write;
> +
> + FREEBUF(hctx_array);
> +}
> +
> static void
> get_mq_diskio(unsigned long q, unsigned long *mq_count)
> {
> int cpu;
> unsigned long queue_ctx;
> unsigned long mctx_addr;
> - struct diskio tmp;
+ struct diskio tmp = {0};
>
> if (INVALID_MEMBER(blk_mq_ctx_rq_dispatched) ||
> - INVALID_MEMBER(blk_mq_ctx_rq_completed))
> + INVALID_MEMBER(blk_mq_ctx_rq_completed)) {
> + get_mq_diskio_from_hw_queues(q, &tmp);
> + mq_count[0] = tmp.read;
> + mq_count[1] = tmp.write;
> return;
> -
> - memset(&tmp, 0x00, sizeof(struct diskio));
> + }
>
> readmem(q + OFFSET(request_queue_queue_ctx), KVADDR, &queue_ctx,
> sizeof(ulong), "request_queue.queue_ctx",
> @@ -4479,41 +4653,24 @@ display_one_diskio(struct iter *i, unsigned long gendisk,
ulong flags)
> && (io.read + io.write == 0))
> return;
>
> - if (use_mq_interface(queue_addr) &&
> - (INVALID_MEMBER(blk_mq_ctx_rq_dispatched) ||
> - INVALID_MEMBER(blk_mq_ctx_rq_completed)))
> - fprintf(fp, "%s%s%s %s%s%s%s %s%s%s",
> - mkstring(buf0, 5, RJUST|INT_DEC, (char *)(unsigned long)major),
> - space(MINSPACE),
> - mkstring(buf1, VADDR_PRLEN, LJUST|LONG_HEX, (char *)gendisk),
> - space(MINSPACE),
> - mkstring(buf2, 10, LJUST, disk_name),
> - space(MINSPACE),
> - mkstring(buf3, VADDR_PRLEN <= 11 ? 11 : VADDR_PRLEN,
> - LJUST|LONG_HEX, (char *)queue_addr),
> - space(MINSPACE),
> - mkstring(buf4, 17, RJUST, "(not supported)"),
> - space(MINSPACE));
> -
> - else
> - fprintf(fp, "%s%s%s %s%s%s%s %s%5d%s%s%s%s%s",
> - mkstring(buf0, 5, RJUST|INT_DEC, (char *)(unsigned long)major),
> - space(MINSPACE),
> - mkstring(buf1, VADDR_PRLEN, LJUST|LONG_HEX, (char *)gendisk),
> - space(MINSPACE),
> - mkstring(buf2, 10, LJUST, disk_name),
> - space(MINSPACE),
> - mkstring(buf3, VADDR_PRLEN <= 11 ? 11 : VADDR_PRLEN,
> - LJUST|LONG_HEX, (char *)queue_addr),
> - space(MINSPACE),
> - io.read + io.write,
> - space(MINSPACE),
> - mkstring(buf4, 5, RJUST|INT_DEC,
> - (char *)(unsigned long)io.read),
> - space(MINSPACE),
> - mkstring(buf5, 5, RJUST|INT_DEC,
> - (char *)(unsigned long)io.write),
> - space(MINSPACE));
> + fprintf(fp, "%s%s%s %s%s%s%s %s%5d%s%s%s%s%s",
> + mkstring(buf0, 5, RJUST|INT_DEC, (char *)(unsigned long)major),
> + space(MINSPACE),
> + mkstring(buf1, VADDR_PRLEN, LJUST|LONG_HEX, (char *)gendisk),
> + space(MINSPACE),
> + mkstring(buf2, 10, LJUST, disk_name),
> + space(MINSPACE),
> + mkstring(buf3, VADDR_PRLEN <= 11 ? 11 : VADDR_PRLEN,
> + LJUST|LONG_HEX, (char *)queue_addr),
> + space(MINSPACE),
> + io.read + io.write,
> + space(MINSPACE),
> + mkstring(buf4, 5, RJUST|INT_DEC,
> + (char *)(unsigned long)io.read),
> + space(MINSPACE),
> + mkstring(buf5, 5, RJUST|INT_DEC,
> + (char *)(unsigned long)io.write),
> + space(MINSPACE));
>
> if (VALID_MEMBER(request_queue_in_flight)) {
> if (!use_mq_interface(queue_addr)) {
> @@ -4597,6 +4754,9 @@ void diskio_init(void)
> MEMBER_OFFSET_INIT(kobject_entry, "kobject", "entry");
> MEMBER_OFFSET_INIT(kset_list, "kset", "list");
> MEMBER_OFFSET_INIT(request_list_count, "request_list",
"count");
> + MEMBER_OFFSET_INIT(request_cmd_flags, "request",
"cmd_flags");
> + MEMBER_OFFSET_INIT(request_q, "request", "q");
> + MEMBER_OFFSET_INIT(request_state, "request", "state");
> MEMBER_OFFSET_INIT(request_queue_in_flight, "request_queue",
> "in_flight");
> if (MEMBER_EXISTS("request_queue", "rq"))
> @@ -4608,10 +4768,33 @@ void diskio_init(void)
> "mq_ops");
> ANON_MEMBER_OFFSET_INIT(request_queue_queue_ctx,
> "request_queue", "queue_ctx");
> + MEMBER_OFFSET_INIT(request_queue_queue_hw_ctx,
> + "request_queue", "queue_hw_ctx");
> + MEMBER_OFFSET_INIT(request_queue_nr_hw_queues,
> + "request_queue", "nr_hw_queues");
> MEMBER_OFFSET_INIT(blk_mq_ctx_rq_dispatched, "blk_mq_ctx",
> "rq_dispatched");
> MEMBER_OFFSET_INIT(blk_mq_ctx_rq_completed, "blk_mq_ctx",
> "rq_completed");
> + MEMBER_OFFSET_INIT(blk_mq_hw_ctx_tags, "blk_mq_hw_ctx",
"tags");
> + MEMBER_OFFSET_INIT(blk_mq_tags_bitmap_tags, "blk_mq_tags",
> + "bitmap_tags");
> + MEMBER_OFFSET_INIT(blk_mq_tags_breserved_tags, "blk_mq_tags",
> + "breserved_tags");
> + MEMBER_OFFSET_INIT(blk_mq_tags_nr_reserved_tags, "blk_mq_tags",
> + "nr_reserved_tags");
> + MEMBER_OFFSET_INIT(blk_mq_tags_rqs, "blk_mq_tags", "rqs");
> + STRUCT_SIZE_INIT(blk_mq_tags, "blk_mq_tags");
> + STRUCT_SIZE_INIT(sbitmap, "sbitmap");
> + STRUCT_SIZE_INIT(sbitmap_word, "sbitmap_word");
> + MEMBER_OFFSET_INIT(sbitmap_word_word, "sbitmap_word",
"word");
> + MEMBER_OFFSET_INIT(sbitmap_word_cleared, "sbitmap_word",
"cleared");
> + MEMBER_OFFSET_INIT(sbitmap_depth, "sbitmap", "depth");
> + MEMBER_OFFSET_INIT(sbitmap_shift, "sbitmap", "shift");
> + MEMBER_OFFSET_INIT(sbitmap_map_nr, "sbitmap", "map_nr");
> + MEMBER_OFFSET_INIT(sbitmap_map, "sbitmap", "map");
> + MEMBER_OFFSET_INIT(sbitmap_queue_sb, "sbitmap_queue", "sb");
> +
> }
> MEMBER_OFFSET_INIT(subsys_private_klist_devices, "subsys_private",
> "klist_devices");
> diff --git a/symbols.c b/symbols.c
> index 5d12a021c769..c1f09556d710 100644
> --- a/symbols.c
> +++ b/symbols.c
> @@ -10385,6 +10385,12 @@ dump_offset_table(char *spec, ulong makestruct)
> OFFSET(kset_list));
> fprintf(fp, " request_list_count: %ld\n",
> OFFSET(request_list_count));
> + fprintf(fp, " request_cmd_flags: %ld\n",
> + OFFSET(request_cmd_flags));
> + fprintf(fp, " request_q: %ld\n",
> + OFFSET(request_q));
> + fprintf(fp, " request_state: %ld\n",
> + OFFSET(request_state));
> fprintf(fp, " request_queue_in_flight: %ld\n",
> OFFSET(request_queue_in_flight));
> fprintf(fp, " request_queue_rq: %ld\n",
> @@ -10393,10 +10399,25 @@ dump_offset_table(char *spec, ulong makestruct)
> OFFSET(request_queue_mq_ops));
> fprintf(fp, " request_queue_queue_ctx: %ld\n",
> OFFSET(request_queue_queue_ctx));
> + fprintf(fp, " request_queue_queue_hw_ctx: %ld\n",
> + OFFSET(request_queue_queue_hw_ctx));
> + fprintf(fp, " request_queue_nr_hw_queues: %ld\n",
> + OFFSET(request_queue_nr_hw_queues));
> fprintf(fp, " blk_mq_ctx_rq_dispatched: %ld\n",
> OFFSET(blk_mq_ctx_rq_dispatched));
> fprintf(fp, " blk_mq_ctx_rq_completed: %ld\n",
> OFFSET(blk_mq_ctx_rq_completed));
> + fprintf(fp, " blk_mq_hw_ctx_tags: %ld\n",
> + OFFSET(blk_mq_hw_ctx_tags));
> + fprintf(fp, " blk_mq_tags_bitmap_tags: %ld\n",
> + OFFSET(blk_mq_tags_bitmap_tags));
> + fprintf(fp, " blk_mq_tags_breserved_tags: %ld\n",
> + OFFSET(blk_mq_tags_breserved_tags));
> + fprintf(fp, " blk_mq_tags_nr_reserved_tags: %ld\n",
> + OFFSET(blk_mq_tags_nr_reserved_tags));
> + fprintf(fp, " blk_mq_tags_rqs: %ld\n",
> + OFFSET(blk_mq_tags_rqs));
Good, thanks for adjusting these indents.
Thanks,
Kazu
> +
> fprintf(fp, " subsys_private_klist_devices: %ld\n",
> OFFSET(subsys_private_klist_devices));
> fprintf(fp, " subsystem_kset: %ld\n",
> @@ -11003,6 +11024,7 @@ dump_offset_table(char *spec, ulong makestruct)
> fprintf(fp, " sbitmap: %ld\n", SIZE(sbitmap));
> fprintf(fp, " sbitmap_queue: %ld\n",
SIZE(sbitmap_queue));
> fprintf(fp, " sbq_wait_state: %ld\n",
SIZE(sbq_wait_state));
> + fprintf(fp, " blk_mq_tags: %ld\n",
SIZE(blk_mq_tags));
>
> fprintf(fp, "\n array_table:\n");
> /*
10:35 p.m.
New subject: [PATCH v3 1/2] Support for "dev -d|-D" options by parsing bitmap in blk-mq layer
On Wed, Jun 1, 2022 at 4:51 PM HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab(a)nec.com>
wrote:
Hi Lianbo,
Thank you for the update, much better structure :)
Let's go into detail...
Thank you for helping the review, it seems that we are getting closer to
this goal.
On 2022/05/30 18:15, Lianbo Jiang wrote:
> Currently, crash doesn't support to display disk I/O
statistics
> for blk-mq devices. For more details, please refer to the following
> commit: <98b417fc6346> ("Handle blk_mq_ctx member changes for kernels
> 5.16-rc1 and later").
>
> Lets parse the bitmap in blk-mq layer to achieve it.
>
> Signed-off-by: Lianbo Jiang <lijiang(a)redhat.com>
> ---
> defs.h | 11 +++
> dev.c | 261 ++++++++++++++++++++++++++++++++++++++++++++++--------
> symbols.c | 22 +++++
> 3 files changed, 255 insertions(+), 39 deletions(-)
>
> diff --git a/defs.h b/defs.h
> index c8444b4e54eb..2681586a33dc 100644
> --- a/defs.h
> +++ b/defs.h
> @@ -2170,6 +2170,16 @@ struct offset_table { /* stash
of commonly-used offsets */
> long sbq_wait_state_wait;
> long sbitmap_alloc_hint;
> long sbitmap_round_robin;
> + long request_cmd_flags;
> + long request_q;
> + long request_state;
> + long request_queue_queue_hw_ctx;
> + long request_queue_nr_hw_queues;
> + long blk_mq_hw_ctx_tags;
> + long blk_mq_tags_bitmap_tags;
> + long blk_mq_tags_breserved_tags;
> + long blk_mq_tags_nr_reserved_tags;
> + long blk_mq_tags_rqs;
> };
>
> struct size_table { /* stash of commonly-used sizes */
> @@ -2339,6 +2349,7 @@ struct size_table { /* stash of
commonly-used sizes */
> long sbitmap;
> long sbitmap_queue;
> long sbq_wait_state;
> + long blk_mq_tags;
> };
>
> struct array_table {
> diff --git a/dev.c b/dev.c
> index a493e51ac95c..c0240f7b889b 100644
> --- a/dev.c
> +++ b/dev.c
> @@ -4238,19 +4238,193 @@ get_one_mctx_diskio(unsigned long mctx, struct
diskio *io)
> io->write = (dispatch[1] - comp[1]);
> }
>
> +typedef bool (busy_tag_iter_fn)(ulong rq, void *data);
> +
> +struct bt_iter_data {
I think that some names are confusing and not synced with the kernel.
How about renaming to "mq_inflight"?
> + ulong q;
> + struct diskio *dio;
> +};
> +
> +struct bt_tags_iter_data {
And please rename this to "bt_iter_data".
> + ulong tags;
> + uint flags;
rename to "reserved".
OK, I will rename the above three variables.
+ uint nr_reserved_tags;
> + busy_tag_iter_fn *fn;
> + void *data;
> +};
> +
> +/*
> + * See the include/linux/blk_types.h and include/linux/blk-mq.h
> + */
> +#define MQ_RQ_IN_FLIGHT 1
> +#define REQ_OP_BITS 8
> +#define REQ_OP_MASK ((1 << REQ_OP_BITS) - 1)
> +
> +static uint op_is_write(uint op)
> +{
> + return (op & REQ_OP_MASK) & 1;
> +}
> +
> +static bool mq_check_inflight(ulong rq, void *data)
> +{
> + uint cmd_flags = 0, state = 0;
> + ulong addr = 0, queue = 0;
> + struct bt_iter_data *iter = data;
> +
> + if (!IS_KVADDR(rq))
> + return TRUE;
Why return TRUE?
Does this mean that rq can be invalid with bit on?
The intent is to filter out the zero address of rq, and crash cannot
completely believe that the address
of rq is always valid. To be on the safe side, it should be good to do this
check.
In addition, the kernel has the similar logic of code, for example:
static bool bt_tags_iter(...)
{
...
if (!rq)
return true;
^^^^
...
}
+
> + addr = rq + OFFSET(request_q);
> + if (!readmem(addr, KVADDR, &queue, sizeof(ulong), "request.q",
RETURN_ON_ERROR))
> + return FALSE;
> +
> + addr = rq + OFFSET(request_cmd_flags);
> + if (!readmem(addr, KVADDR, &cmd_flags, sizeof(uint),
"request.cmd_flags", RETURN_ON_ERROR))
> + return FALSE;
> +
> + addr = rq + OFFSET(request_state);
> + if (!readmem(addr, KVADDR, &state, sizeof(uint),
"request.state",
RETURN_ON_ERROR))
> + return FALSE;
Rethinking these, RETURN_ON_ERROR leads to stop counting (i.e. wrong
results)
with error messages. I think it would be better to use FAULT_ON_ERROR
unless
we can ignore or handle an error correctly. What do you think?
(And for the RETURN_ON_ERRORs below, same as above.)
At first, I had the same opinion as yours. But eventually, called the
readmem()
with the "RETURN_ON_ERROR",
the reason is that crash can help to output the statistics as much as
possible, which is helpful to user, and once the
readmem() failed, it will print an appropriate error, and it can explicitly
tell users that the statistics may be unreliable.
If this is really confusing to the user, let's replace it with the
"FAULT_ON_ERROR".
+
> + if (queue == iter->q && state == MQ_RQ_IN_FLIGHT) {
> + if (op_is_write(cmd_flags))
> + iter->dio->write++;
> + else
> + iter->dio->read++;
> + }
> +
> + return TRUE;
> +}
> +
> +static bool bt_tags_iter(uint bitnr, void *data)
> +{
> + ulong addr = 0, rqs_addr = 0, rq = 0;
> + struct bt_tags_iter_data *tag_iter = data;
> + ulong tag = tag_iter->tags;
> +
> + if (!tag_iter->flags)
> + bitnr += tag_iter->nr_reserved_tags;
> +
> + /* rqs */
> + addr = tag + OFFSET(blk_mq_tags_rqs);
> + if (!readmem(addr, KVADDR, &rqs_addr, sizeof(void *),
"blk_mq_tags.rqs", RETURN_ON_ERROR))
> + return FALSE;
> +
> + addr = rqs_addr + bitnr * sizeof(ulong); /* rqs[bitnr] */
> + if (!readmem(addr, KVADDR, &rq, sizeof(ulong),
"blk_mq_tags.rqs[]", RETURN_ON_ERROR))
> + return FALSE;
> +
> + return tag_iter->fn(rq, tag_iter->data);
> +}
> +
> +static void bt_tags_for_each(ulong q, ulong tags, ulong sbq, uint
flags, uint nr_resvd_tags, struct diskio *dio)
Please rename this to bt_for_each.
No problem.
+{
> + struct sbitmap_context sc = {0};
> + struct diskio tmp = {0};
This tmp can be removed?
(The dio->{read,write} are incremented in mq_check_inflight(), so
we don't need the local tmps for each function, I think)
Let me double check.
+ struct bt_iter_data iter_data = {
> + .q = q,
> + .dio = &tmp,
> + };
> + struct bt_tags_iter_data tags_iter_data = {
> + .tags = tags,
> + .flags = flags,
> + .nr_reserved_tags = nr_resvd_tags,
> + .fn = mq_check_inflight,
> + .data = &iter_data,
> + };
> +
> + sbitmap_context_load(sbq + OFFSET(sbitmap_queue_sb), &sc);
> + sbitmap_for_each_set(&sc, bt_tags_iter, &tags_iter_data);
> +
> + dio->read = tmp.read;
> + dio->write = tmp.write;
> +}
> +
> +static void queue_for_each_hw_ctx(ulong q, ulong *hctx, uint cnt,
struct diskio *dio)
> +{
> + uint i;
> + struct diskio tmp = {0};
This also can be removed.
> +
> + if (!hctx || !dio)
> + error(FATAL, "%s: Invalid parameter.\n", __func__);
This can be used?
Will remove it.
+
> + for (i = 0; i < cnt; i++) {
> + ulong addr = 0, tags = 0;
> + uint nr_reserved_tags = 0;
> +
> + if (!IS_KVADDR(hctx[i]))
> + continue;
Is this the remain of the v1 patch?
Yes, this can be removed from the 'for loop'.
+
> + /* Tags owned by the block driver */
> + addr = hctx[i] + OFFSET(blk_mq_hw_ctx_tags);
> + if (!readmem(addr, KVADDR, &tags, sizeof(ulong),
> + "blk_mq_hw_ctx.tags", RETURN_ON_ERROR))
> + break;
> +
> + addr = tags + OFFSET(blk_mq_tags_nr_reserved_tags);
> + if (!readmem(addr, KVADDR, &nr_reserved_tags, sizeof(uint),
> + "blk_mq_tags_nr_reserved_tags",
RETURN_ON_ERROR))
> + break;
> +
> + if (nr_reserved_tags) {
> + addr = tags + OFFSET(blk_mq_tags_breserved_tags);
> + bt_tags_for_each(q, tags, addr, 1,
nr_reserved_tags, &tmp);
> + }
> + addr = tags + OFFSET(blk_mq_tags_bitmap_tags);
> + bt_tags_for_each(q, tags, addr, 0, nr_reserved_tags, &tmp);
(the local tmp set by the nr_reserved_tags case is overwritten here.)
Thanks for pointing out this issue. It should be accumulated in the
bt_tags_for_each() as below:
+ dio->read += tmp.read;
+ dio->write += tmp.write;
But anyway, I will rethink the variable 'tmp' and see what is the good way.
Thanks.
Lianbo
> + }
> +
> + dio->read = tmp.read;
> + dio->write = tmp.write;
> +}
> +
> +static void get_mq_diskio_from_hw_queues(ulong q, struct diskio *dio)
> +{
> + uint cnt = 0;
> + ulong addr = 0, hctx_addr = 0;
> + ulong *hctx_array = NULL;
> + struct diskio tmp = {0};
This also can be removed.
Ditto.
+
> + addr = q + OFFSET(request_queue_nr_hw_queues);
> + readmem(addr, KVADDR, &cnt, sizeof(uint),
> + "request_queue.nr_hw_queues", FAULT_ON_ERROR);
> +
> + addr = q + OFFSET(request_queue_queue_hw_ctx);
> + readmem(addr, KVADDR, &hctx_addr, sizeof(void *),
> + "request_queue.queue_hw_ctx", FAULT_ON_ERROR);
> +
> + hctx_array = (ulong *)GETBUF(sizeof(void *) * cnt);
> + if (!hctx_array)
> + error(FATAL, "fail to get memory for the hctx_array\n");
> +
> + if (!readmem(hctx_addr, KVADDR, hctx_array, sizeof(void *) * cnt,
> + "request_queue.queue_hw_ctx[]", RETURN_ON_ERROR)) {
> + FREEBUF(hctx_array);
> + return;
> + }
> +
> + queue_for_each_hw_ctx(q, hctx_array, cnt, &tmp);
> + dio->read = tmp.read;
> + dio->write = tmp.write;
> +
> + FREEBUF(hctx_array);
> +}
> +
> static void
> get_mq_diskio(unsigned long q, unsigned long *mq_count)
> {
> int cpu;
> unsigned long queue_ctx;
> unsigned long mctx_addr;
> - struct diskio tmp;
> + struct diskio tmp = {0};
>
> if (INVALID_MEMBER(blk_mq_ctx_rq_dispatched) ||
> - INVALID_MEMBER(blk_mq_ctx_rq_completed))
> + INVALID_MEMBER(blk_mq_ctx_rq_completed)) {
> + get_mq_diskio_from_hw_queues(q, &tmp);
> + mq_count[0] = tmp.read;
> + mq_count[1] = tmp.write;
> return;
> -
> - memset(&tmp, 0x00, sizeof(struct diskio));
> + }
>
> readmem(q + OFFSET(request_queue_queue_ctx), KVADDR, &queue_ctx,
> sizeof(ulong), "request_queue.queue_ctx",
> @@ -4479,41 +4653,24 @@ display_one_diskio(struct iter *i, unsigned long
gendisk, ulong flags)
> && (io.read + io.write == 0))
> return;
>
> - if (use_mq_interface(queue_addr) &&
> - (INVALID_MEMBER(blk_mq_ctx_rq_dispatched) ||
> - INVALID_MEMBER(blk_mq_ctx_rq_completed)))
> - fprintf(fp, "%s%s%s %s%s%s%s %s%s%s",
> - mkstring(buf0, 5, RJUST|INT_DEC, (char *)(unsigned
long)major),
> - space(MINSPACE),
> - mkstring(buf1, VADDR_PRLEN, LJUST|LONG_HEX, (char
*)gendisk),
> - space(MINSPACE),
> - mkstring(buf2, 10, LJUST, disk_name),
> - space(MINSPACE),
> - mkstring(buf3, VADDR_PRLEN <= 11 ? 11 :
VADDR_PRLEN,
> - LJUST|LONG_HEX, (char *)queue_addr),
> - space(MINSPACE),
> - mkstring(buf4, 17, RJUST, "(not supported)"),
> - space(MINSPACE));
> -
> - else
> - fprintf(fp, "%s%s%s %s%s%s%s %s%5d%s%s%s%s%s",
> - mkstring(buf0, 5, RJUST|INT_DEC, (char *)(unsigned
long)major),
> - space(MINSPACE),
> - mkstring(buf1, VADDR_PRLEN, LJUST|LONG_HEX, (char
*)gendisk),
> - space(MINSPACE),
> - mkstring(buf2, 10, LJUST, disk_name),
> - space(MINSPACE),
> - mkstring(buf3, VADDR_PRLEN <= 11 ? 11 :
VADDR_PRLEN,
> - LJUST|LONG_HEX, (char *)queue_addr),
> - space(MINSPACE),
> - io.read + io.write,
> - space(MINSPACE),
> - mkstring(buf4, 5, RJUST|INT_DEC,
> - (char *)(unsigned long)io.read),
> - space(MINSPACE),
> - mkstring(buf5, 5, RJUST|INT_DEC,
> - (char *)(unsigned long)io.write),
> - space(MINSPACE));
> + fprintf(fp, "%s%s%s %s%s%s%s %s%5d%s%s%s%s%s",
> + mkstring(buf0, 5, RJUST|INT_DEC, (char *)(unsigned
long)major),
> + space(MINSPACE),
> + mkstring(buf1, VADDR_PRLEN, LJUST|LONG_HEX, (char
*)gendisk),
> + space(MINSPACE),
> + mkstring(buf2, 10, LJUST, disk_name),
> + space(MINSPACE),
> + mkstring(buf3, VADDR_PRLEN <= 11 ? 11 : VADDR_PRLEN,
> + LJUST|LONG_HEX, (char *)queue_addr),
> + space(MINSPACE),
> + io.read + io.write,
> + space(MINSPACE),
> + mkstring(buf4, 5, RJUST|INT_DEC,
> + (char *)(unsigned long)io.read),
> + space(MINSPACE),
> + mkstring(buf5, 5, RJUST|INT_DEC,
> + (char *)(unsigned long)io.write),
> + space(MINSPACE));
>
> if (VALID_MEMBER(request_queue_in_flight)) {
> if (!use_mq_interface(queue_addr)) {
> @@ -4597,6 +4754,9 @@ void diskio_init(void)
> MEMBER_OFFSET_INIT(kobject_entry, "kobject", "entry");
> MEMBER_OFFSET_INIT(kset_list, "kset", "list");
> MEMBER_OFFSET_INIT(request_list_count, "request_list",
"count");
> + MEMBER_OFFSET_INIT(request_cmd_flags, "request",
"cmd_flags");
> + MEMBER_OFFSET_INIT(request_q, "request", "q");
> + MEMBER_OFFSET_INIT(request_state, "request", "state");
> MEMBER_OFFSET_INIT(request_queue_in_flight, "request_queue",
> "in_flight");
> if (MEMBER_EXISTS("request_queue", "rq"))
> @@ -4608,10 +4768,33 @@ void diskio_init(void)
> "mq_ops");
> ANON_MEMBER_OFFSET_INIT(request_queue_queue_ctx,
> "request_queue", "queue_ctx");
> + MEMBER_OFFSET_INIT(request_queue_queue_hw_ctx,
> + "request_queue", "queue_hw_ctx");
> + MEMBER_OFFSET_INIT(request_queue_nr_hw_queues,
> + "request_queue", "nr_hw_queues");
> MEMBER_OFFSET_INIT(blk_mq_ctx_rq_dispatched, "blk_mq_ctx",
> "rq_dispatched");
> MEMBER_OFFSET_INIT(blk_mq_ctx_rq_completed, "blk_mq_ctx",
> "rq_completed");
> + MEMBER_OFFSET_INIT(blk_mq_hw_ctx_tags, "blk_mq_hw_ctx",
"tags");
> + MEMBER_OFFSET_INIT(blk_mq_tags_bitmap_tags, "blk_mq_tags",
> + "bitmap_tags");
> + MEMBER_OFFSET_INIT(blk_mq_tags_breserved_tags,
"blk_mq_tags",
> + "breserved_tags");
> + MEMBER_OFFSET_INIT(blk_mq_tags_nr_reserved_tags,
"blk_mq_tags",
> + "nr_reserved_tags");
> + MEMBER_OFFSET_INIT(blk_mq_tags_rqs, "blk_mq_tags",
"rqs");
> + STRUCT_SIZE_INIT(blk_mq_tags, "blk_mq_tags");
> + STRUCT_SIZE_INIT(sbitmap, "sbitmap");
> + STRUCT_SIZE_INIT(sbitmap_word, "sbitmap_word");
> + MEMBER_OFFSET_INIT(sbitmap_word_word, "sbitmap_word",
"word");
> + MEMBER_OFFSET_INIT(sbitmap_word_cleared, "sbitmap_word",
"cleared");
> + MEMBER_OFFSET_INIT(sbitmap_depth, "sbitmap",
"depth");
> + MEMBER_OFFSET_INIT(sbitmap_shift, "sbitmap",
"shift");
> + MEMBER_OFFSET_INIT(sbitmap_map_nr, "sbitmap",
"map_nr");
> + MEMBER_OFFSET_INIT(sbitmap_map, "sbitmap",
"map");
> + MEMBER_OFFSET_INIT(sbitmap_queue_sb, "sbitmap_queue",
"sb");
> +
> }
> MEMBER_OFFSET_INIT(subsys_private_klist_devices, "subsys_private",
> "klist_devices");
> diff --git a/symbols.c b/symbols.c
> index 5d12a021c769..c1f09556d710 100644
> --- a/symbols.c
> +++ b/symbols.c
> @@ -10385,6 +10385,12 @@ dump_offset_table(char *spec, ulong makestruct)
> OFFSET(kset_list));
> fprintf(fp, " request_list_count: %ld\n",
> OFFSET(request_list_count));
> + fprintf(fp, " request_cmd_flags: %ld\n",
> + OFFSET(request_cmd_flags));
> + fprintf(fp, " request_q: %ld\n",
> + OFFSET(request_q));
> + fprintf(fp, " request_state: %ld\n",
> + OFFSET(request_state));
> fprintf(fp, " request_queue_in_flight: %ld\n",
> OFFSET(request_queue_in_flight));
> fprintf(fp, " request_queue_rq: %ld\n",
> @@ -10393,10 +10399,25 @@ dump_offset_table(char *spec, ulong makestruct)
> OFFSET(request_queue_mq_ops));
> fprintf(fp, " request_queue_queue_ctx: %ld\n",
> OFFSET(request_queue_queue_ctx));
> + fprintf(fp, " request_queue_queue_hw_ctx: %ld\n",
> + OFFSET(request_queue_queue_hw_ctx));
> + fprintf(fp, " request_queue_nr_hw_queues: %ld\n",
> + OFFSET(request_queue_nr_hw_queues));
> fprintf(fp, " blk_mq_ctx_rq_dispatched: %ld\n",
> OFFSET(blk_mq_ctx_rq_dispatched));
> fprintf(fp, " blk_mq_ctx_rq_completed: %ld\n",
> OFFSET(blk_mq_ctx_rq_completed));
> + fprintf(fp, " blk_mq_hw_ctx_tags: %ld\n",
> + OFFSET(blk_mq_hw_ctx_tags));
> + fprintf(fp, " blk_mq_tags_bitmap_tags: %ld\n",
> + OFFSET(blk_mq_tags_bitmap_tags));
> + fprintf(fp, " blk_mq_tags_breserved_tags: %ld\n",
> + OFFSET(blk_mq_tags_breserved_tags));
> + fprintf(fp, " blk_mq_tags_nr_reserved_tags: %ld\n",
> + OFFSET(blk_mq_tags_nr_reserved_tags));
> + fprintf(fp, " blk_mq_tags_rqs: %ld\n",
> + OFFSET(blk_mq_tags_rqs));
Good, thanks for adjusting these indents.
Thanks,
Kazu
> +
> fprintf(fp, " subsys_private_klist_devices: %ld\n",
> OFFSET(subsys_private_klist_devices));
> fprintf(fp, " subsystem_kset: %ld\n",
> @@ -11003,6 +11024,7 @@ dump_offset_table(char *spec, ulong makestruct)
> fprintf(fp, " sbitmap: %ld\n",
SIZE(sbitmap));
> fprintf(fp, " sbitmap_queue: %ld\n",
SIZE(sbitmap_queue));
> fprintf(fp, " sbq_wait_state: %ld\n",
SIZE(sbq_wait_state));
> + fprintf(fp, " blk_mq_tags: %ld\n",
SIZE(blk_mq_tags));
>
> fprintf(fp, "\n array_table:\n");
> /*
11:32 p.m.
New subject: [PATCH v3 1/2] Support for "dev -d|-D" options by parsing bitmap in blk-mq layer
On 2022/06/02 12:35, lijiang wrote:
On Wed, Jun 1, 2022 at 4:51 PM HAGIO KAZUHITO(萩尾 一仁)
<k-hagio-ab(a)nec.com <mailto:k-hagio-ab@nec.com>> wrote:
Hi Lianbo,
Thank you for the update, much better structure :)
Let's go into detail...
Thank you for helping the review, it seems that we are getting closer to this goal.
yes, thanks to your hard work :)
On 2022/05/30 18:15, Lianbo Jiang wrote:
> Currently, crash doesn't support to display disk I/O statistics
> for blk-mq devices. For more details, please refer to the following
> commit: <98b417fc6346> ("Handle blk_mq_ctx member changes for
kernels
> 5.16-rc1 and later").
>
> Lets parse the bitmap in blk-mq layer to achieve it.
>
> Signed-off-by: Lianbo Jiang <lijiang(a)redhat.com
<mailto:lijiang@redhat.com>>
> ---
> defs.h | 11 +++
> dev.c | 261 ++++++++++++++++++++++++++++++++++++++++++++++--------
> symbols.c | 22 +++++
> 3 files changed, 255 insertions(+), 39 deletions(-)
>
> diff --git a/defs.h b/defs.h
> index c8444b4e54eb..2681586a33dc 100644
> --- a/defs.h
> +++ b/defs.h
> @@ -2170,6 +2170,16 @@ struct offset_table { /* stash of
commonly-used offsets */
> long sbq_wait_state_wait;
> long sbitmap_alloc_hint;
> long sbitmap_round_robin;
> + long request_cmd_flags;
> + long request_q;
> + long request_state;
> + long request_queue_queue_hw_ctx;
> + long request_queue_nr_hw_queues;
> + long blk_mq_hw_ctx_tags;
> + long blk_mq_tags_bitmap_tags;
> + long blk_mq_tags_breserved_tags;
> + long blk_mq_tags_nr_reserved_tags;
> + long blk_mq_tags_rqs;
> };
>
> struct size_table { /* stash of commonly-used sizes */
> @@ -2339,6 +2349,7 @@ struct size_table { /* stash of commonly-used
sizes */
> long sbitmap;
> long sbitmap_queue;
> long sbq_wait_state;
> + long blk_mq_tags;
> };
>
> struct array_table {
> diff --git a/dev.c b/dev.c
> index a493e51ac95c..c0240f7b889b 100644
> --- a/dev.c
> +++ b/dev.c
> @@ -4238,19 +4238,193 @@ get_one_mctx_diskio(unsigned long mctx, struct diskio
*io)
> io->write = (dispatch[1] - comp[1]);
> }
>
> +typedef bool (busy_tag_iter_fn)(ulong rq, void *data);
> +
> +struct bt_iter_data {
I think that some names are confusing and not synced with the kernel.
How about renaming to "mq_inflight"?
> + ulong q;
> + struct diskio *dio;
> +};
> +
> +struct bt_tags_iter_data {
And please rename this to "bt_iter_data".
> + ulong tags;
> + uint flags;
rename to "reserved".
OK, I will rename the above three variables.
> + uint nr_reserved_tags;
> + busy_tag_iter_fn *fn;
> + void *data;
> +};
> +
> +/*
> + * See the include/linux/blk_types.h and include/linux/blk-mq.h
> + */
> +#define MQ_RQ_IN_FLIGHT 1
> +#define REQ_OP_BITS 8
> +#define REQ_OP_MASK ((1 << REQ_OP_BITS) - 1)
> +
> +static uint op_is_write(uint op)
> +{
> + return (op & REQ_OP_MASK) & 1;
> +}
> +
> +static bool mq_check_inflight(ulong rq, void *data)
> +{
> + uint cmd_flags = 0, state = 0;
> + ulong addr = 0, queue = 0;
> + struct bt_iter_data *iter = data;
> +
> + if (!IS_KVADDR(rq))
> + return TRUE;
Why return TRUE?
Does this mean that rq can be invalid with bit on?
The intent is to filter out the zero address of rq, and crash cannot completely believe
that the address
of rq is always valid. To be on the safe side, it should be good to do this check.
In addition, the kernel has the similar logic of code, for example:
static bool bt_tags_iter(...)
{
...
if (!rq)
return true;
^^^^
...
}
Got it, thanks. I missed this comment:
/*
* We can hit rq == NULL here, because the tagging functions
* test and set the bit before assigning ->rqs[].
*/
> +
> + addr = rq + OFFSET(request_q);
> + if (!readmem(addr, KVADDR, &queue, sizeof(ulong),
"request.q", RETURN_ON_ERROR))
> + return FALSE;
> +
> + addr = rq + OFFSET(request_cmd_flags);
> + if (!readmem(addr, KVADDR, &cmd_flags, sizeof(uint),
"request.cmd_flags", RETURN_ON_ERROR))
> + return FALSE;
> +
> + addr = rq + OFFSET(request_state);
> + if (!readmem(addr, KVADDR, &state, sizeof(uint),
"request.state", RETURN_ON_ERROR))
> + return FALSE;
Rethinking these, RETURN_ON_ERROR leads to stop counting (i.e. wrong results)
with error messages. I think it would be better to use FAULT_ON_ERROR unless
we can ignore or handle an error correctly. What do you think?
(And for the RETURN_ON_ERRORs below, same as above.)
At first, I had the same opinion as yours. But eventually, called the readmem() with the
"RETURN_ON_ERROR",
the reason is that crash can help to output the statistics as much as possible, which is
helpful to user, and once the
readmem() failed, it will print an appropriate error, and it can explicitly tell users
that the statistics may be unreliable.
Agree. I tested and got this:
crash> dev -d
MAJOR GENDISK NAME REQUEST_QUEUE TOTAL ASYNC SYNC
dev: invalid kernel virtual address: 0 type: "request.q"
252 ffff88ee0188c800 vda ffff88ee11588ff0 0 0 0
253 ffff88ee109fa200 dm-0 ffff88ee01a7aa80 0 0 0
253 ffff88ee109fb000 dm-1 ffff88ee01a7da50 12 12 0
It would be better to print the remains as much as possible, let's go with
RETURN_ON_ERROR.
If this is really confusing to the user, let's replace it with the
"FAULT_ON_ERROR".
> +
> + if (queue == iter->q && state == MQ_RQ_IN_FLIGHT) {
> + if (op_is_write(cmd_flags))
> + iter->dio->write++;
> + else
> + iter->dio->read++;
> + }
> +
> + return TRUE;
> +}
> +
> +static bool bt_tags_iter(uint bitnr, void *data)
I missed this, please rename also this to bt_iter, according to the kernel
path that we follow.
Thanks,
Kazu
> > +{
> > + ulong addr = 0, rqs_addr = 0, rq = 0;
> > + struct bt_tags_iter_data *tag_iter = data;
> > + ulong tag = tag_iter->tags;
> > +
> > + if (!tag_iter->flags)
> > + bitnr += tag_iter->nr_reserved_tags;
> > +
> > + /* rqs */
> > + addr = tag + OFFSET(blk_mq_tags_rqs);
> > + if (!readmem(addr, KVADDR, &rqs_addr, sizeof(void *),
"blk_mq_tags.rqs", RETURN_ON_ERROR))
> > + return FALSE;
> > +
> > + addr = rqs_addr + bitnr * sizeof(ulong); /* rqs[bitnr] */
> > + if (!readmem(addr, KVADDR, &rq, sizeof(ulong),
"blk_mq_tags.rqs[]", RETURN_ON_ERROR))
> > + return FALSE;
> > +
> > + return tag_iter->fn(rq, tag_iter->data);
> > +}
> > +
> > +static void bt_tags_for_each(ulong q, ulong tags, ulong sbq, uint flags,
uint nr_resvd_tags, struct diskio *dio)
>
> Please rename this to bt_for_each.
>
> No problem.
>
> > +{
> > + struct sbitmap_context sc = {0};
>
> > + struct diskio tmp = {0};
>
> This tmp can be removed?
>
> (The dio->{read,write} are incremented in mq_check_inflight(), so
> we don't need the local tmps for each function, I think)
>
> Let me double check.
>
> > + struct bt_iter_data iter_data = {
> > + .q = q,
> > + .dio = &tmp,
> > + };
> > + struct bt_tags_iter_data tags_iter_data = {
> > + .tags = tags,
> > + .flags = flags,
> > + .nr_reserved_tags = nr_resvd_tags,
> > + .fn = mq_check_inflight,
> > + .data = &iter_data,
> > + };
> > +
> > + sbitmap_context_load(sbq + OFFSET(sbitmap_queue_sb), &sc);
> > + sbitmap_for_each_set(&sc, bt_tags_iter, &tags_iter_data);
> > +
> > + dio->read = tmp.read;
> > + dio->write = tmp.write;
> > +}
> > +
> > +static void queue_for_each_hw_ctx(ulong q, ulong *hctx, uint cnt, struct
diskio *dio)
> > +{
> > + uint i;
>
> > + struct diskio tmp = {0};
>
> This also can be removed.
>
> > +
> > + if (!hctx || !dio)
> > + error(FATAL, "%s: Invalid parameter.\n",
__func__);
>
> This can be used?
>
> Will remove it.
>
> > +
> > + for (i = 0; i < cnt; i++) {
> > + ulong addr = 0, tags = 0;
> > + uint nr_reserved_tags = 0;
> > +
>
> > + if (!IS_KVADDR(hctx[i]))
> > + continue;
>
> Is this the remain of the v1 patch?
>
>
> Yes, this can be removed from the 'for loop'.
>
> > +
> > + /* Tags owned by the block driver */
> > + addr = hctx[i] + OFFSET(blk_mq_hw_ctx_tags);
> > + if (!readmem(addr, KVADDR, &tags, sizeof(ulong),
> > + "blk_mq_hw_ctx.tags",
RETURN_ON_ERROR))
> > + break;
> > +
> > + addr = tags + OFFSET(blk_mq_tags_nr_reserved_tags);
> > + if (!readmem(addr, KVADDR, &nr_reserved_tags,
sizeof(uint),
> > + "blk_mq_tags_nr_reserved_tags",
RETURN_ON_ERROR))
> > + break;
> > +
> > + if (nr_reserved_tags) {
> > + addr = tags + OFFSET(blk_mq_tags_breserved_tags);
> > + bt_tags_for_each(q, tags, addr, 1, nr_reserved_tags,
&tmp);
> > + }
> > + addr = tags + OFFSET(blk_mq_tags_bitmap_tags);
> > + bt_tags_for_each(q, tags, addr, 0, nr_reserved_tags,
&tmp);
>
> (the local tmp set by the nr_reserved_tags case is overwritten here.)
>
> Thanks for pointing out this issue. It should be accumulated in the
bt_tags_for_each() as below:
> + dio->read += tmp.read;
> + dio->write += tmp.write;
>
> But anyway, I will rethink the variable 'tmp' and see what is the good way.
>
> Thanks.
> Lianbo
>
> > + }
> > +
> > + dio->read = tmp.read;
> > + dio->write = tmp.write;
> > +}
> > +
> > +static void get_mq_diskio_from_hw_queues(ulong q, struct diskio *dio)
> > +{
> > + uint cnt = 0;
> > + ulong addr = 0, hctx_addr = 0;
> > + ulong *hctx_array = NULL;
>
> > + struct diskio tmp = {0};
>
> This also can be removed.
>
> Ditto.
>
> > +
> > + addr = q + OFFSET(request_queue_nr_hw_queues);
> > + readmem(addr, KVADDR, &cnt, sizeof(uint),
> > + "request_queue.nr_hw_queues", FAULT_ON_ERROR);
> > +
> > + addr = q + OFFSET(request_queue_queue_hw_ctx);
> > + readmem(addr, KVADDR, &hctx_addr, sizeof(void *),
> > + "request_queue.queue_hw_ctx", FAULT_ON_ERROR);
> > +
> > + hctx_array = (ulong *)GETBUF(sizeof(void *) * cnt);
> > + if (!hctx_array)
> > + error(FATAL, "fail to get memory for the
hctx_array\n");
> > +
> > + if (!readmem(hctx_addr, KVADDR, hctx_array, sizeof(void *) * cnt,
> > + "request_queue.queue_hw_ctx[]",
RETURN_ON_ERROR)) {
> > + FREEBUF(hctx_array);
> > + return;
> > + }
> > +
> > + queue_for_each_hw_ctx(q, hctx_array, cnt, &tmp);
> > + dio->read = tmp.read;
> > + dio->write = tmp.write;
> > +
> > + FREEBUF(hctx_array);
> > +}
> > +
> > static void
> > get_mq_diskio(unsigned long q, unsigned long *mq_count)
> > {
> > int cpu;
> > unsigned long queue_ctx;
> > unsigned long mctx_addr;
> > - struct diskio tmp;
> > + struct diskio tmp = {0};
> >
> > if (INVALID_MEMBER(blk_mq_ctx_rq_dispatched) ||
> > - INVALID_MEMBER(blk_mq_ctx_rq_completed))
> > + INVALID_MEMBER(blk_mq_ctx_rq_completed)) {
> > + get_mq_diskio_from_hw_queues(q, &tmp);
> > + mq_count[0] = tmp.read;
> > + mq_count[1] = tmp.write;
> > return;
> > -
> > - memset(&tmp, 0x00, sizeof(struct diskio));
> > + }
> >
> > readmem(q + OFFSET(request_queue_queue_ctx), KVADDR,
&queue_ctx,
> > sizeof(ulong), "request_queue.queue_ctx",
> > @@ -4479,41 +4653,24 @@ display_one_diskio(struct iter *i, unsigned long
gendisk, ulong flags)
> > && (io.read + io.write == 0))
> > return;
> >
> > - if (use_mq_interface(queue_addr) &&
> > - (INVALID_MEMBER(blk_mq_ctx_rq_dispatched) ||
> > - INVALID_MEMBER(blk_mq_ctx_rq_completed)))
> > - fprintf(fp, "%s%s%s %s%s%s%s %s%s%s",
> > - mkstring(buf0, 5, RJUST|INT_DEC, (char *)(unsigned
long)major),
> > - space(MINSPACE),
> > - mkstring(buf1, VADDR_PRLEN, LJUST|LONG_HEX, (char
*)gendisk),
> > - space(MINSPACE),
> > - mkstring(buf2, 10, LJUST, disk_name),
> > - space(MINSPACE),
> > - mkstring(buf3, VADDR_PRLEN <= 11 ? 11 :
VADDR_PRLEN,
> > - LJUST|LONG_HEX, (char *)queue_addr),
> > - space(MINSPACE),
> > - mkstring(buf4, 17, RJUST, "(not
supported)"),
> > - space(MINSPACE));
> > -
> > - else
> > - fprintf(fp, "%s%s%s %s%s%s%s %s%5d%s%s%s%s%s",
> > - mkstring(buf0, 5, RJUST|INT_DEC, (char *)(unsigned
long)major),
> > - space(MINSPACE),
> > - mkstring(buf1, VADDR_PRLEN, LJUST|LONG_HEX, (char
*)gendisk),
> > - space(MINSPACE),
> > - mkstring(buf2, 10, LJUST, disk_name),
> > - space(MINSPACE),
> > - mkstring(buf3, VADDR_PRLEN <= 11 ? 11 :
VADDR_PRLEN,
> > - LJUST|LONG_HEX, (char *)queue_addr),
> > - space(MINSPACE),
> > - io.read + io.write,
> > - space(MINSPACE),
> > - mkstring(buf4, 5, RJUST|INT_DEC,
> > - (char *)(unsigned long)io.read),
> > - space(MINSPACE),
> > - mkstring(buf5, 5, RJUST|INT_DEC,
> > - (char *)(unsigned long)io.write),
> > - space(MINSPACE));
> > + fprintf(fp, "%s%s%s %s%s%s%s %s%5d%s%s%s%s%s",
> > + mkstring(buf0, 5, RJUST|INT_DEC, (char *)(unsigned
long)major),
> > + space(MINSPACE),
> > + mkstring(buf1, VADDR_PRLEN, LJUST|LONG_HEX, (char
*)gendisk),
> > + space(MINSPACE),
> > + mkstring(buf2, 10, LJUST, disk_name),
> > + space(MINSPACE),
> > + mkstring(buf3, VADDR_PRLEN <= 11 ? 11 : VADDR_PRLEN,
> > + LJUST|LONG_HEX, (char *)queue_addr),
> > + space(MINSPACE),
> > + io.read + io.write,
> > + space(MINSPACE),
> > + mkstring(buf4, 5, RJUST|INT_DEC,
> > + (char *)(unsigned long)io.read),
> > + space(MINSPACE),
> > + mkstring(buf5, 5, RJUST|INT_DEC,
> > + (char *)(unsigned long)io.write),
> > + space(MINSPACE));
> >
> > if (VALID_MEMBER(request_queue_in_flight)) {
> > if (!use_mq_interface(queue_addr)) {
> > @@ -4597,6 +4754,9 @@ void diskio_init(void)
> > MEMBER_OFFSET_INIT(kobject_entry, "kobject",
"entry");
> > MEMBER_OFFSET_INIT(kset_list, "kset", "list");
> > MEMBER_OFFSET_INIT(request_list_count, "request_list",
"count");
> > + MEMBER_OFFSET_INIT(request_cmd_flags, "request",
"cmd_flags");
> > + MEMBER_OFFSET_INIT(request_q, "request", "q");
> > + MEMBER_OFFSET_INIT(request_state, "request",
"state");
> > MEMBER_OFFSET_INIT(request_queue_in_flight,
"request_queue",
> > "in_flight");
> > if (MEMBER_EXISTS("request_queue", "rq"))
> > @@ -4608,10 +4768,33 @@ void diskio_init(void)
> > "mq_ops");
> > ANON_MEMBER_OFFSET_INIT(request_queue_queue_ctx,
> > "request_queue", "queue_ctx");
> > + MEMBER_OFFSET_INIT(request_queue_queue_hw_ctx,
> > + "request_queue",
"queue_hw_ctx");
> > + MEMBER_OFFSET_INIT(request_queue_nr_hw_queues,
> > + "request_queue",
"nr_hw_queues");
> > MEMBER_OFFSET_INIT(blk_mq_ctx_rq_dispatched,
"blk_mq_ctx",
> > "rq_dispatched");
> > MEMBER_OFFSET_INIT(blk_mq_ctx_rq_completed,
"blk_mq_ctx",
> > "rq_completed");
> > + MEMBER_OFFSET_INIT(blk_mq_hw_ctx_tags,
"blk_mq_hw_ctx", "tags");
> > + MEMBER_OFFSET_INIT(blk_mq_tags_bitmap_tags,
"blk_mq_tags",
> > + "bitmap_tags");
> > + MEMBER_OFFSET_INIT(blk_mq_tags_breserved_tags,
"blk_mq_tags",
> > + "breserved_tags");
> > + MEMBER_OFFSET_INIT(blk_mq_tags_nr_reserved_tags,
"blk_mq_tags",
> > + "nr_reserved_tags");
> > + MEMBER_OFFSET_INIT(blk_mq_tags_rqs, "blk_mq_tags",
"rqs");
> > + STRUCT_SIZE_INIT(blk_mq_tags, "blk_mq_tags");
> > + STRUCT_SIZE_INIT(sbitmap, "sbitmap");
> > + STRUCT_SIZE_INIT(sbitmap_word, "sbitmap_word");
> > + MEMBER_OFFSET_INIT(sbitmap_word_word,
"sbitmap_word", "word");
> > + MEMBER_OFFSET_INIT(sbitmap_word_cleared,
"sbitmap_word", "cleared");
> > + MEMBER_OFFSET_INIT(sbitmap_depth, "sbitmap",
"depth");
> > + MEMBER_OFFSET_INIT(sbitmap_shift, "sbitmap",
"shift");
> > + MEMBER_OFFSET_INIT(sbitmap_map_nr, "sbitmap",
"map_nr");
> > + MEMBER_OFFSET_INIT(sbitmap_map, "sbitmap",
"map");
> > + MEMBER_OFFSET_INIT(sbitmap_queue_sb,
"sbitmap_queue", "sb");
> > +
> > }
> > MEMBER_OFFSET_INIT(subsys_private_klist_devices,
"subsys_private",
> > "klist_devices");
> > diff --git a/symbols.c b/symbols.c
> > index 5d12a021c769..c1f09556d710 100644
> > --- a/symbols.c
> > +++ b/symbols.c
> > @@ -10385,6 +10385,12 @@ dump_offset_table(char *spec, ulong makestruct)
> > OFFSET(kset_list));
> > fprintf(fp, " request_list_count: %ld\n",
> > OFFSET(request_list_count));
> > + fprintf(fp, " request_cmd_flags: %ld\n",
> > + OFFSET(request_cmd_flags));
> > + fprintf(fp, " request_q: %ld\n",
> > + OFFSET(request_q));
> > + fprintf(fp, " request_state: %ld\n",
> > + OFFSET(request_state));
> > fprintf(fp, " request_queue_in_flight: %ld\n",
> > OFFSET(request_queue_in_flight));
> > fprintf(fp, " request_queue_rq: %ld\n",
> > @@ -10393,10 +10399,25 @@ dump_offset_table(char *spec, ulong makestruct)
> > OFFSET(request_queue_mq_ops));
> > fprintf(fp, " request_queue_queue_ctx: %ld\n",
> > OFFSET(request_queue_queue_ctx));
> > + fprintf(fp, " request_queue_queue_hw_ctx: %ld\n",
> > + OFFSET(request_queue_queue_hw_ctx));
> > + fprintf(fp, " request_queue_nr_hw_queues: %ld\n",
> > + OFFSET(request_queue_nr_hw_queues));
> > fprintf(fp, " blk_mq_ctx_rq_dispatched: %ld\n",
> > OFFSET(blk_mq_ctx_rq_dispatched));
> > fprintf(fp, " blk_mq_ctx_rq_completed: %ld\n",
> > OFFSET(blk_mq_ctx_rq_completed));
> > + fprintf(fp, " blk_mq_hw_ctx_tags: %ld\n",
> > + OFFSET(blk_mq_hw_ctx_tags));
> > + fprintf(fp, " blk_mq_tags_bitmap_tags: %ld\n",
> > + OFFSET(blk_mq_tags_bitmap_tags));
> > + fprintf(fp, " blk_mq_tags_breserved_tags: %ld\n",
> > + OFFSET(blk_mq_tags_breserved_tags));
> > + fprintf(fp, " blk_mq_tags_nr_reserved_tags: %ld\n",
> > + OFFSET(blk_mq_tags_nr_reserved_tags));
> > + fprintf(fp, " blk_mq_tags_rqs: %ld\n",
> > + OFFSET(blk_mq_tags_rqs));
>
> Good, thanks for adjusting these indents.
>
> Thanks,
> Kazu
>
> > +
> > fprintf(fp, " subsys_private_klist_devices: %ld\n",
> > OFFSET(subsys_private_klist_devices));
> > fprintf(fp, " subsystem_kset: %ld\n",
> > @@ -11003,6 +11024,7 @@ dump_offset_table(char *spec, ulong makestruct)
> > fprintf(fp, " sbitmap: %ld\n",
SIZE(sbitmap));
> > fprintf(fp, " sbitmap_queue: %ld\n",
SIZE(sbitmap_queue));
> > fprintf(fp, " sbq_wait_state: %ld\n",
SIZE(sbq_wait_state));
> > + fprintf(fp, " blk_mq_tags: %ld\n",
SIZE(blk_mq_tags));
> >
> > fprintf(fp, "\n array_table:\n");
> > /*
>
12:14 a.m.
New subject: [PATCH v3 1/2] Support for "dev -d|-D" options by parsing bitmap in blk-mq layer
On Thu, Jun 2, 2022 at 12:32 PM HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab(a)nec.com>
wrote:
On 2022/06/02 12:35, lijiang wrote:
> On Wed, Jun 1, 2022 at 4:51 PM HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab(a)nec.com
<mailto:k-hagio-ab@nec.com>> wrote:
>
> Hi Lianbo,
>
> Thank you for the update, much better structure :)
>
> Let's go into detail...
>
>
> Thank you for helping the review, it seems that we are getting closer to
this goal.
yes, thanks to your hard work :)
>
> On 2022/05/30 18:15, Lianbo Jiang wrote:
> > Currently, crash doesn't support to display disk I/O statistics
> > for blk-mq devices. For more details, please refer to the
following
> > commit: <98b417fc6346> ("Handle blk_mq_ctx member changes for
kernels
> > 5.16-rc1 and later").
> >
> > Lets parse the bitmap in blk-mq layer to achieve it.
> >
> > Signed-off-by: Lianbo Jiang <lijiang(a)redhat.com <mailto:
lijiang(a)redhat.com>>
> > ---
> > defs.h | 11 +++
> > dev.c | 261
++++++++++++++++++++++++++++++++++++++++++++++--------
> > symbols.c | 22 +++++
> > 3 files changed, 255 insertions(+), 39 deletions(-)
> >
> > diff --git a/defs.h b/defs.h
> > index c8444b4e54eb..2681586a33dc 100644
> > --- a/defs.h
> > +++ b/defs.h
> > @@ -2170,6 +2170,16 @@ struct offset_table {
/* stash of commonly-used offsets */
> > long sbq_wait_state_wait;
> > long sbitmap_alloc_hint;
> > long sbitmap_round_robin;
> > + long request_cmd_flags;
> > + long request_q;
> > + long request_state;
> > + long request_queue_queue_hw_ctx;
> > + long request_queue_nr_hw_queues;
> > + long blk_mq_hw_ctx_tags;
> > + long blk_mq_tags_bitmap_tags;
> > + long blk_mq_tags_breserved_tags;
> > + long blk_mq_tags_nr_reserved_tags;
> > + long blk_mq_tags_rqs;
> > };
> >
> > struct size_table { /* stash of commonly-used sizes */
> > @@ -2339,6 +2349,7 @@ struct size_table { /* stash of
commonly-used sizes */
> > long sbitmap;
> > long sbitmap_queue;
> > long sbq_wait_state;
> > + long blk_mq_tags;
> > };
> >
> > struct array_table {
> > diff --git a/dev.c b/dev.c
> > index a493e51ac95c..c0240f7b889b 100644
> > --- a/dev.c
> > +++ b/dev.c
> > @@ -4238,19 +4238,193 @@ get_one_mctx_diskio(unsigned long mctx,
struct diskio *io)
> > io->write = (dispatch[1] - comp[1]);
> > }
> >
> > +typedef bool (busy_tag_iter_fn)(ulong rq, void *data);
> > +
> > +struct bt_iter_data {
>
> I think that some names are confusing and not synced with the kernel.
> How about renaming to "mq_inflight"?
>
> > + ulong q;
> > + struct diskio *dio;
> > +};
> > +
> > +struct bt_tags_iter_data {
>
> And please rename this to "bt_iter_data".
>
> > + ulong tags;
> > + uint flags;
>
> rename to "reserved".
>
>
> OK, I will rename the above three variables.
>
> > + uint nr_reserved_tags;
> > + busy_tag_iter_fn *fn;
> > + void *data;
> > +};
> > +
> > +/*
> > + * See the include/linux/blk_types.h and include/linux/blk-mq.h
> > + */
> > +#define MQ_RQ_IN_FLIGHT 1
> > +#define REQ_OP_BITS 8
> > +#define REQ_OP_MASK ((1 << REQ_OP_BITS) - 1)
> > +
> > +static uint op_is_write(uint op)
> > +{
> > + return (op & REQ_OP_MASK) & 1;
> > +}
> > +
> > +static bool mq_check_inflight(ulong rq, void *data)
> > +{
> > + uint cmd_flags = 0, state = 0;
> > + ulong addr = 0, queue = 0;
> > + struct bt_iter_data *iter = data;
> > +
> > + if (!IS_KVADDR(rq))
> > + return TRUE;
>
> Why return TRUE?
> Does this mean that rq can be invalid with bit on?
>
> The intent is to filter out the zero address of rq, and crash cannot
completely believe that the address
> of rq is always valid. To be on the safe side, it should be good to do
this check.
> In addition, the kernel has the similar logic of code, for example:
> static bool bt_tags_iter(...)
> {
> ...
> if (!rq)
> return true;
> ^^^^
> ...
> }
Got it, thanks. I missed this comment:
/*
* We can hit rq == NULL here, because the tagging functions
* test and set the bit before assigning ->rqs[].
*/
>
> > +
> > + addr = rq + OFFSET(request_q);
> > + if (!readmem(addr, KVADDR, &queue, sizeof(ulong),
"request.q", RETURN_ON_ERROR))
> > + return FALSE;
> > +
> > + addr = rq + OFFSET(request_cmd_flags);
> > + if (!readmem(addr, KVADDR, &cmd_flags, sizeof(uint),
"request.cmd_flags", RETURN_ON_ERROR))
> > + return FALSE;
> > +
> > + addr = rq + OFFSET(request_state);
> > + if (!readmem(addr, KVADDR, &state, sizeof(uint),
"request.state", RETURN_ON_ERROR))
> > + return FALSE;
>
> Rethinking these, RETURN_ON_ERROR leads to stop counting (i.e. wrong
results)
> with error messages. I think it would be better to use
FAULT_ON_ERROR unless
> we can ignore or handle an error correctly. What do you think?
>
> (And for the RETURN_ON_ERRORs below, same as above.)
>
> At first, I had the same opinion as yours. But eventually, called the
readmem() with the "RETURN_ON_ERROR",
> the reason is that crash can help to output the statistics as much as
possible, which is helpful to user, and once the
> readmem() failed, it will print an appropriate error, and it can
explicitly tell users that the statistics may be unreliable.
Agree. I tested and got this:
crash> dev -d
MAJOR GENDISK NAME REQUEST_QUEUE TOTAL ASYNC SYNC
dev: invalid kernel virtual address: 0 type: "request.q"
Right, that is expected.
252 ffff88ee0188c800 vda ffff88ee11588ff0 0 0 0
253 ffff88ee109fa200 dm-0 ffff88ee01a7aa80 0 0
0
253 ffff88ee109fb000 dm-1 ffff88ee01a7da50 12 12 0
It would be better to print the remains as much as possible, let's go with
RETURN_ON_ERROR.
>
> If this is really confusing to the user, let's replace it with the
"FAULT_ON_ERROR".
>
> > +
> > + if (queue == iter->q && state == MQ_RQ_IN_FLIGHT) {
> > + if (op_is_write(cmd_flags))
> > + iter->dio->write++;
> > + else
> > + iter->dio->read++;
> > + }
> > +
> > + return TRUE;
> > +}
> > +
> > +static bool bt_tags_iter(uint bitnr, void *data)
I missed this, please rename also this to bt_iter, according to the kernel
path that we follow.
Ok, sounds good.
Thanks.
Lianbo
Thanks,
Kazu
> > +{
> > + ulong addr = 0, rqs_addr = 0, rq = 0;
> > + struct bt_tags_iter_data *tag_iter = data;
> > + ulong tag = tag_iter->tags;
> > +
> > + if (!tag_iter->flags)
> > + bitnr += tag_iter->nr_reserved_tags;
> > +
> > + /* rqs */
> > + addr = tag + OFFSET(blk_mq_tags_rqs);
> > + if (!readmem(addr, KVADDR, &rqs_addr, sizeof(void *),
"blk_mq_tags.rqs", RETURN_ON_ERROR))
> > + return FALSE;
> > +
> > + addr = rqs_addr + bitnr * sizeof(ulong); /* rqs[bitnr] */
> > + if (!readmem(addr, KVADDR, &rq, sizeof(ulong),
"blk_mq_tags.rqs[]", RETURN_ON_ERROR))
> > + return FALSE;
> > +
> > + return tag_iter->fn(rq, tag_iter->data);
> > +}
> > +
> > +static void bt_tags_for_each(ulong q, ulong tags, ulong sbq,
uint flags, uint nr_resvd_tags, struct diskio *dio)
>
> Please rename this to bt_for_each.
>
> No problem.
>
> > +{
> > + struct sbitmap_context sc = {0};
>
> > + struct diskio tmp = {0};
>
> This tmp can be removed?
>
> (The dio->{read,write} are incremented in mq_check_inflight(), so
> we don't need the local tmps for each function, I think)
>
> Let me double check.
>
> > + struct bt_iter_data iter_data = {
> > + .q = q,
> > + .dio = &tmp,
> > + };
> > + struct bt_tags_iter_data tags_iter_data = {
> > + .tags = tags,
> > + .flags = flags,
> > + .nr_reserved_tags = nr_resvd_tags,
> > + .fn = mq_check_inflight,
> > + .data = &iter_data,
> > + };
> > +
> > + sbitmap_context_load(sbq + OFFSET(sbitmap_queue_sb), &sc);
> > + sbitmap_for_each_set(&sc, bt_tags_iter, &tags_iter_data);
> > +
> > + dio->read = tmp.read;
> > + dio->write = tmp.write;
> > +}
> > +
> > +static void queue_for_each_hw_ctx(ulong q, ulong *hctx, uint
cnt, struct diskio *dio)
> > +{
> > + uint i;
>
> > + struct diskio tmp = {0};
>
> This also can be removed.
>
> > +
> > + if (!hctx || !dio)
> > + error(FATAL, "%s: Invalid parameter.\n",
__func__);
>
> This can be used?
>
> Will remove it.
>
> > +
> > + for (i = 0; i < cnt; i++) {
> > + ulong addr = 0, tags = 0;
> > + uint nr_reserved_tags = 0;
> > +
>
> > + if (!IS_KVADDR(hctx[i]))
> > + continue;
>
> Is this the remain of the v1 patch?
>
>
> Yes, this can be removed from the 'for loop'.
>
> > +
> > + /* Tags owned by the block driver */
> > + addr = hctx[i] + OFFSET(blk_mq_hw_ctx_tags);
> > + if (!readmem(addr, KVADDR, &tags, sizeof(ulong),
> > + "blk_mq_hw_ctx.tags",
RETURN_ON_ERROR))
> > + break;
> > +
> > + addr = tags + OFFSET(blk_mq_tags_nr_reserved_tags);
> > + if (!readmem(addr, KVADDR, &nr_reserved_tags,
sizeof(uint),
> > + "blk_mq_tags_nr_reserved_tags",
RETURN_ON_ERROR))
> > + break;
> > +
> > + if (nr_reserved_tags) {
> > + addr = tags +
OFFSET(blk_mq_tags_breserved_tags);
> > + bt_tags_for_each(q, tags, addr, 1,
nr_reserved_tags, &tmp);
> > + }
> > + addr = tags + OFFSET(blk_mq_tags_bitmap_tags);
> > + bt_tags_for_each(q, tags, addr, 0,
nr_reserved_tags, &tmp);
>
> (the local tmp set by the nr_reserved_tags case is overwritten here.)
>
> Thanks for pointing out this issue. It should be accumulated in the
bt_tags_for_each() as below:
> + dio->read += tmp.read;
> + dio->write += tmp.write;
>
> But anyway, I will rethink the variable 'tmp' and see what is the good
way.
>
> Thanks.
> Lianbo
>
> > + }
> > +
> > + dio->read = tmp.read;
> > + dio->write = tmp.write;
> > +}
> > +
> > +static void get_mq_diskio_from_hw_queues(ulong q, struct diskio
*dio)
> > +{
> > + uint cnt = 0;
> > + ulong addr = 0, hctx_addr = 0;
> > + ulong *hctx_array = NULL;
>
> > + struct diskio tmp = {0};
>
> This also can be removed.
>
> Ditto.
>
> > +
> > + addr = q + OFFSET(request_queue_nr_hw_queues);
> > + readmem(addr, KVADDR, &cnt, sizeof(uint),
> > + "request_queue.nr_hw_queues", FAULT_ON_ERROR);
> > +
> > + addr = q + OFFSET(request_queue_queue_hw_ctx);
> > + readmem(addr, KVADDR, &hctx_addr, sizeof(void *),
> > + "request_queue.queue_hw_ctx", FAULT_ON_ERROR);
> > +
> > + hctx_array = (ulong *)GETBUF(sizeof(void *) * cnt);
> > + if (!hctx_array)
> > + error(FATAL, "fail to get memory for the
hctx_array\n");
> > +
> > + if (!readmem(hctx_addr, KVADDR, hctx_array, sizeof(void *)
* cnt,
> > + "request_queue.queue_hw_ctx[]",
RETURN_ON_ERROR)) {
> > + FREEBUF(hctx_array);
> > + return;
> > + }
> > +
> > + queue_for_each_hw_ctx(q, hctx_array, cnt, &tmp);
> > + dio->read = tmp.read;
> > + dio->write = tmp.write;
> > +
> > + FREEBUF(hctx_array);
> > +}
> > +
> > static void
> > get_mq_diskio(unsigned long q, unsigned long *mq_count)
> > {
> > int cpu;
> > unsigned long queue_ctx;
> > unsigned long mctx_addr;
> > - struct diskio tmp;
> > + struct diskio tmp = {0};
> >
> > if (INVALID_MEMBER(blk_mq_ctx_rq_dispatched) ||
> > - INVALID_MEMBER(blk_mq_ctx_rq_completed))
> > + INVALID_MEMBER(blk_mq_ctx_rq_completed)) {
> > + get_mq_diskio_from_hw_queues(q, &tmp);
> > + mq_count[0] = tmp.read;
> > + mq_count[1] = tmp.write;
> > return;
> > -
> > - memset(&tmp, 0x00, sizeof(struct diskio));
> > + }
> >
> > readmem(q + OFFSET(request_queue_queue_ctx), KVADDR,
&queue_ctx,
> > sizeof(ulong), "request_queue.queue_ctx",
> > @@ -4479,41 +4653,24 @@ display_one_diskio(struct iter *i,
unsigned long gendisk, ulong flags)
> > && (io.read + io.write == 0))
> > return;
> >
> > - if (use_mq_interface(queue_addr) &&
> > - (INVALID_MEMBER(blk_mq_ctx_rq_dispatched) ||
> > - INVALID_MEMBER(blk_mq_ctx_rq_completed)))
> > - fprintf(fp, "%s%s%s %s%s%s%s %s%s%s",
> > - mkstring(buf0, 5, RJUST|INT_DEC, (char
*)(unsigned long)major),
> > - space(MINSPACE),
> > - mkstring(buf1, VADDR_PRLEN, LJUST|LONG_HEX,
(char *)gendisk),
> > - space(MINSPACE),
> > - mkstring(buf2, 10, LJUST, disk_name),
> > - space(MINSPACE),
> > - mkstring(buf3, VADDR_PRLEN <= 11 ? 11 :
VADDR_PRLEN,
> > - LJUST|LONG_HEX, (char
*)queue_addr),
> > - space(MINSPACE),
> > - mkstring(buf4, 17, RJUST, "(not
supported)"),
> > - space(MINSPACE));
> > -
> > - else
> > - fprintf(fp, "%s%s%s %s%s%s%s %s%5d%s%s%s%s%s",
> > - mkstring(buf0, 5, RJUST|INT_DEC, (char
*)(unsigned long)major),
> > - space(MINSPACE),
> > - mkstring(buf1, VADDR_PRLEN, LJUST|LONG_HEX,
(char *)gendisk),
> > - space(MINSPACE),
> > - mkstring(buf2, 10, LJUST, disk_name),
> > - space(MINSPACE),
> > - mkstring(buf3, VADDR_PRLEN <= 11 ? 11 :
VADDR_PRLEN,
> > - LJUST|LONG_HEX, (char
*)queue_addr),
> > - space(MINSPACE),
> > - io.read + io.write,
> > - space(MINSPACE),
> > - mkstring(buf4, 5, RJUST|INT_DEC,
> > - (char *)(unsigned long)io.read),
> > - space(MINSPACE),
> > - mkstring(buf5, 5, RJUST|INT_DEC,
> > - (char *)(unsigned long)io.write),
> > - space(MINSPACE));
> > + fprintf(fp, "%s%s%s %s%s%s%s %s%5d%s%s%s%s%s",
> > + mkstring(buf0, 5, RJUST|INT_DEC, (char *)(unsigned
long)major),
> > + space(MINSPACE),
> > + mkstring(buf1, VADDR_PRLEN, LJUST|LONG_HEX, (char
*)gendisk),
> > + space(MINSPACE),
> > + mkstring(buf2, 10, LJUST, disk_name),
> > + space(MINSPACE),
> > + mkstring(buf3, VADDR_PRLEN <= 11 ? 11 : VADDR_PRLEN,
> > + LJUST|LONG_HEX, (char *)queue_addr),
> > + space(MINSPACE),
> > + io.read + io.write,
> > + space(MINSPACE),
> > + mkstring(buf4, 5, RJUST|INT_DEC,
> > + (char *)(unsigned long)io.read),
> > + space(MINSPACE),
> > + mkstring(buf5, 5, RJUST|INT_DEC,
> > + (char *)(unsigned long)io.write),
> > + space(MINSPACE));
> >
> > if (VALID_MEMBER(request_queue_in_flight)) {
> > if (!use_mq_interface(queue_addr)) {
> > @@ -4597,6 +4754,9 @@ void diskio_init(void)
> > MEMBER_OFFSET_INIT(kobject_entry, "kobject",
"entry");
> > MEMBER_OFFSET_INIT(kset_list, "kset", "list");
> > MEMBER_OFFSET_INIT(request_list_count, "request_list",
"count");
> > + MEMBER_OFFSET_INIT(request_cmd_flags, "request",
"cmd_flags");
> > + MEMBER_OFFSET_INIT(request_q, "request", "q");
> > + MEMBER_OFFSET_INIT(request_state, "request",
"state");
> > MEMBER_OFFSET_INIT(request_queue_in_flight,
"request_queue",
> > "in_flight");
> > if (MEMBER_EXISTS("request_queue", "rq"))
> > @@ -4608,10 +4768,33 @@ void diskio_init(void)
> > "mq_ops");
> > ANON_MEMBER_OFFSET_INIT(request_queue_queue_ctx,
> > "request_queue", "queue_ctx");
> > + MEMBER_OFFSET_INIT(request_queue_queue_hw_ctx,
> > + "request_queue",
"queue_hw_ctx");
> > + MEMBER_OFFSET_INIT(request_queue_nr_hw_queues,
> > + "request_queue",
"nr_hw_queues");
> > MEMBER_OFFSET_INIT(blk_mq_ctx_rq_dispatched,
"blk_mq_ctx",
> > "rq_dispatched");
> > MEMBER_OFFSET_INIT(blk_mq_ctx_rq_completed,
"blk_mq_ctx",
> > "rq_completed");
> > + MEMBER_OFFSET_INIT(blk_mq_hw_ctx_tags,
"blk_mq_hw_ctx", "tags");
> > + MEMBER_OFFSET_INIT(blk_mq_tags_bitmap_tags,
"blk_mq_tags",
> > + "bitmap_tags");
> > + MEMBER_OFFSET_INIT(blk_mq_tags_breserved_tags,
"blk_mq_tags",
> > + "breserved_tags");
> > + MEMBER_OFFSET_INIT(blk_mq_tags_nr_reserved_tags,
"blk_mq_tags",
> > + "nr_reserved_tags");
> > + MEMBER_OFFSET_INIT(blk_mq_tags_rqs,
"blk_mq_tags",
"rqs");
> > + STRUCT_SIZE_INIT(blk_mq_tags, "blk_mq_tags");
> > + STRUCT_SIZE_INIT(sbitmap, "sbitmap");
> > + STRUCT_SIZE_INIT(sbitmap_word, "sbitmap_word");
> > + MEMBER_OFFSET_INIT(sbitmap_word_word,
"sbitmap_word", "word");
> > + MEMBER_OFFSET_INIT(sbitmap_word_cleared,
"sbitmap_word", "cleared");
> > + MEMBER_OFFSET_INIT(sbitmap_depth, "sbitmap",
"depth");
> > + MEMBER_OFFSET_INIT(sbitmap_shift, "sbitmap",
"shift");
> > + MEMBER_OFFSET_INIT(sbitmap_map_nr, "sbitmap",
"map_nr");
> > + MEMBER_OFFSET_INIT(sbitmap_map, "sbitmap",
"map");
> > + MEMBER_OFFSET_INIT(sbitmap_queue_sb,
"sbitmap_queue", "sb");
> > +
> > }
> > MEMBER_OFFSET_INIT(subsys_private_klist_devices,
"subsys_private",
> > "klist_devices");
> > diff --git a/symbols.c b/symbols.c
> > index 5d12a021c769..c1f09556d710 100644
> > --- a/symbols.c
> > +++ b/symbols.c
> > @@ -10385,6 +10385,12 @@ dump_offset_table(char *spec, ulong
makestruct)
> > OFFSET(kset_list));
> > fprintf(fp, " request_list_count: %ld\n",
> > OFFSET(request_list_count));
> > + fprintf(fp, " request_cmd_flags: %ld\n",
> > + OFFSET(request_cmd_flags));
> > + fprintf(fp, " request_q: %ld\n",
> > + OFFSET(request_q));
> > + fprintf(fp, " request_state: %ld\n",
> > + OFFSET(request_state));
> > fprintf(fp, " request_queue_in_flight: %ld\n",
> > OFFSET(request_queue_in_flight));
> > fprintf(fp, " request_queue_rq: %ld\n",
> > @@ -10393,10 +10399,25 @@ dump_offset_table(char *spec, ulong
makestruct)
> > OFFSET(request_queue_mq_ops));
> > fprintf(fp, " request_queue_queue_ctx: %ld\n",
> > OFFSET(request_queue_queue_ctx));
> > + fprintf(fp, " request_queue_queue_hw_ctx: %ld\n",
> > + OFFSET(request_queue_queue_hw_ctx));
> > + fprintf(fp, " request_queue_nr_hw_queues: %ld\n",
> > + OFFSET(request_queue_nr_hw_queues));
> > fprintf(fp, " blk_mq_ctx_rq_dispatched: %ld\n",
> > OFFSET(blk_mq_ctx_rq_dispatched));
> > fprintf(fp, " blk_mq_ctx_rq_completed: %ld\n",
> > OFFSET(blk_mq_ctx_rq_completed));
> > + fprintf(fp, " blk_mq_hw_ctx_tags: %ld\n",
> > + OFFSET(blk_mq_hw_ctx_tags));
> > + fprintf(fp, " blk_mq_tags_bitmap_tags: %ld\n",
> > + OFFSET(blk_mq_tags_bitmap_tags));
> > + fprintf(fp, " blk_mq_tags_breserved_tags: %ld\n",
> > + OFFSET(blk_mq_tags_breserved_tags));
> > + fprintf(fp, " blk_mq_tags_nr_reserved_tags: %ld\n",
> > + OFFSET(blk_mq_tags_nr_reserved_tags));
> > + fprintf(fp, " blk_mq_tags_rqs: %ld\n",
> > + OFFSET(blk_mq_tags_rqs));
>
> Good, thanks for adjusting these indents.
>
> Thanks,
> Kazu
>
> > +
> > fprintf(fp, " subsys_private_klist_devices: %ld\n",
> > OFFSET(subsys_private_klist_devices));
> > fprintf(fp, " subsystem_kset: %ld\n",
> > @@ -11003,6 +11024,7 @@ dump_offset_table(char *spec, ulong
makestruct)
> > fprintf(fp, " sbitmap: %ld\n",
SIZE(sbitmap));
> > fprintf(fp, " sbitmap_queue: %ld\n",
SIZE(sbitmap_queue));
> > fprintf(fp, " sbq_wait_state: %ld\n",
SIZE(sbq_wait_state));
> > + fprintf(fp, " blk_mq_tags: %ld\n",
SIZE(blk_mq_tags));
> >
> > fprintf(fp, "\n array_table:\n");
> > /*
>
4:15 a.m.
New subject: [PATCH v3 2/2] Fix for "dev -d|-D" options to correctly display disk I/O statistics on linux v5.18-rc1
Kernel commit 4e5cc99e1e48 ("blk-mq: manage hctx map via xarray") removed
the "queue_hw_ctx" member from struct request_queue at Linux v5.18-rc1,
and replaced it with a struct xarray "hctx_table".Without the patch,
the "dev -d|-D" options will print an error:
crash> dev -d
MAJOR GENDISK NAME REQUEST_QUEUE TOTAL READ WRITE
dev: invalid structure member offset: request_queue_queue_hw_ctx
With the patch:
crash> dev -d
MAJOR GENDISK NAME REQUEST_QUEUE TOTAL READ WRITE
8 ffff8e99d0a1ae00 sda ffff8e9c14c59980 10 6 4
Signed-off-by: Lianbo Jiang <lijiang(a)redhat.com>
---
defs.h | 1 +
dev.c | 44 ++++++++++++++++++++++++++++++++++----------
symbols.c | 2 ++
3 files changed, 37 insertions(+), 10 deletions(-)
diff --git a/defs.h b/defs.h
index 2681586a33dc..7d3b73422f48 100644
--- a/defs.h
+++ b/defs.h
@@ -2180,6 +2180,7 @@ struct offset_table { /* stash of commonly-used
offsets */
long blk_mq_tags_breserved_tags;
long blk_mq_tags_nr_reserved_tags;
long blk_mq_tags_rqs;
+ long request_queue_hctx_table;
};
struct size_table { /* stash of commonly-used sizes */
diff --git a/dev.c b/dev.c
index c0240f7b889b..cdfffe67744b 100644
--- a/dev.c
+++ b/dev.c
@@ -4383,24 +4383,46 @@ static void get_mq_diskio_from_hw_queues(ulong q, struct diskio
*dio)
uint cnt = 0;
ulong addr = 0, hctx_addr = 0;
ulong *hctx_array = NULL;
+ struct list_pair *lp = NULL;
struct diskio tmp = {0};
- addr = q + OFFSET(request_queue_nr_hw_queues);
- readmem(addr, KVADDR, &cnt, sizeof(uint),
- "request_queue.nr_hw_queues", FAULT_ON_ERROR);
+ if (VALID_MEMBER(request_queue_hctx_table)) {
+ addr = q + OFFSET(request_queue_hctx_table);
+ cnt = do_xarray(addr, XARRAY_COUNT, NULL);
+ lp = (struct list_pair *)GETBUF(sizeof(struct list_pair) * (cnt + 1));
+ if (!lp)
+ error(FATAL, "fail to get memory for list_pair.\n");
+ lp[0].index = cnt;
+ cnt = do_xarray(addr, XARRAY_GATHER, lp);
+ } else {
+ addr = q + OFFSET(request_queue_nr_hw_queues);
+ readmem(addr, KVADDR, &cnt, sizeof(uint),
+ "request_queue.nr_hw_queues", FAULT_ON_ERROR);
- addr = q + OFFSET(request_queue_queue_hw_ctx);
- readmem(addr, KVADDR, &hctx_addr, sizeof(void *),
- "request_queue.queue_hw_ctx", FAULT_ON_ERROR);
+ addr = q + OFFSET(request_queue_queue_hw_ctx);
+ readmem(addr, KVADDR, &hctx_addr, sizeof(void *),
+ "request_queue.queue_hw_ctx", FAULT_ON_ERROR);
+ }
hctx_array = (ulong *)GETBUF(sizeof(void *) * cnt);
- if (!hctx_array)
+ if (!hctx_array) {
+ if (lp)
+ FREEBUF(lp);
error(FATAL, "fail to get memory for the hctx_array\n");
+ }
+
+ if (lp && hctx_array) {
+ uint i;
- if (!readmem(hctx_addr, KVADDR, hctx_array, sizeof(void *) * cnt,
+ /* copy it from list_pair to hctx_array */
+ for (i = 0; i < cnt; i++) {
+ hctx_array[i] = (ulong)lp[i].value;
+ }
+ FREEBUF(lp);
+ } else if (!readmem(hctx_addr, KVADDR, hctx_array, sizeof(void *) * cnt,
"request_queue.queue_hw_ctx[]", RETURN_ON_ERROR)) {
- FREEBUF(hctx_array);
- return;
+ FREEBUF(hctx_array);
+ return;
}
queue_for_each_hw_ctx(q, hctx_array, cnt, &tmp);
@@ -4772,6 +4794,8 @@ void diskio_init(void)
"request_queue", "queue_hw_ctx");
MEMBER_OFFSET_INIT(request_queue_nr_hw_queues,
"request_queue", "nr_hw_queues");
+ MEMBER_OFFSET_INIT(request_queue_hctx_table,
+ "request_queue", "hctx_table");
MEMBER_OFFSET_INIT(blk_mq_ctx_rq_dispatched, "blk_mq_ctx",
"rq_dispatched");
MEMBER_OFFSET_INIT(blk_mq_ctx_rq_completed, "blk_mq_ctx",
diff --git a/symbols.c b/symbols.c
index c1f09556d710..bee1faf92c83 100644
--- a/symbols.c
+++ b/symbols.c
@@ -10403,6 +10403,8 @@ dump_offset_table(char *spec, ulong makestruct)
OFFSET(request_queue_queue_hw_ctx));
fprintf(fp, " request_queue_nr_hw_queues: %ld\n",
OFFSET(request_queue_nr_hw_queues));
+ fprintf(fp, " request_queue_hctx_table: %ld\n",
+ OFFSET(request_queue_hctx_table));
fprintf(fp, " blk_mq_ctx_rq_dispatched: %ld\n",
OFFSET(blk_mq_ctx_rq_dispatched));
fprintf(fp, " blk_mq_ctx_rq_completed: %ld\n",
--
2.20.1
760
days inactive
763
days old
devel@lists.crash-utility.osci.io
6 comments
3 participants
participants (3)
-
HAGIO KAZUHITO(萩尾 一仁) -
Lianbo Jiang -
lijiang