Hi Lianbo, -----Original Message----- > Kernel commit <9a14d6ce4135> ("block: remove debugfs blk_mq_ctx > dispatched/merged/completed attributes") removed the member > rq_dispatched and rq_completed from struct blk_mq_ctx. Without > this patch, crash will fail with the following error: > > crash> dev -d > MAJOR GENDISK NAME REQUEST_QUEUE TOTAL ASYNC SYNC > > dev: invalid structure member offset: blk_mq_ctx_rq_dispatched > FILE: dev.c LINE: 4229 FUNCTION: get_one_mctx_diskio() Thank you for working on this. IIUC, crash will need sbitmap functionality to support this fully. Meanwhile, I think it's OK to skip the count, but it should say "actually zero" or "not supported" so that users can know it, for example:

crash> dev -d MAJOR GENDISK NAME REQUEST_QUEUE TOTAL ASYNC SYNC 8 ffff92bbd102e400 sdb ffff92bbf04e3678 0 0 0 dev: sdb: Disk I/O statistics is not supported in this kernel 8 ffff92bbd1014400 sdc ffff92bbf04e26e8 0 0 0 dev: sdc: Disk I/O statistics is not supported in this kernel 11 ffff92c347da5e00 sr0 ffff92bbd9c3e528 0 0 0 253 ffff92bbca726e00 dm-0 ffff92bbcad7dd60 0 0 0 What do you think?

> > Signed-off-by: Lianbo Jiang <lijiang(a)redhat.com&gt; > --- > dev.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/dev.c b/dev.c > index effe789f38d8..dd21511e5dfc 100644 > --- a/dev.c > +++ b/dev.c > @@ -4246,6 +4246,10 @@ get_mq_diskio(unsigned long q, unsigned long *mq_count) > unsigned long mctx_addr; > struct diskio tmp; > > + if (!MEMBER_EXISTS("blk_mq_ctx", "rq_dispatched") && > + !MEMBER_EXISTS("blk_mq_ctx", "rq_completed")) > + return; > + > memset(&tmp, 0x00, sizeof(struct diskio)); > > readmem(q + OFFSET(request_queue_queue_ctx), KVADDR, &queue_ctx, > -- > 2.20.1

> -----Original Message----- > On Fri, Dec 24, 2021 at 1:14 PM HAGIO KAZUHITO(萩尾　一仁) > <k-hagio-ab(a)nec.com&gt; wrote: > > > > Hi Lianbo, > > > > -----Original Message----- > > > Kernel commit <9a14d6ce4135> ("block: remove debugfs blk_mq_ctx > > > dispatched/merged/completed attributes") removed the member > > > rq_dispatched and rq_completed from struct blk_mq_ctx. Without > > > this patch, crash will fail with the following error: > > > > > > crash> dev -d > > > MAJOR GENDISK NAME REQUEST_QUEUE TOTAL ASYNC SYNC > > > > > > dev: invalid structure member offset: blk_mq_ctx_rq_dispatched > > > FILE: dev.c LINE: 4229 FUNCTION: get_one_mctx_diskio() > > > > Thank you for working on this. > > IIUC, crash will need sbitmap functionality to support this fully. > > > > Meanwhile, I think it's OK to skip the count, but it should say "actually zero" > > or "not supported" so that users can know it, for example: > > > Thank you for the comment, Kazu. > > > crash> dev -d > > MAJOR GENDISK NAME REQUEST_QUEUE TOTAL ASYNC SYNC > > 8 ffff92bbd102e400 sdb ffff92bbf04e3678 0 0 0 > > dev: sdb: Disk I/O statistics is not supported in this kernel > > 8 ffff92bbd1014400 sdc ffff92bbf04e26e8 0 0 0 > > dev: sdc: Disk I/O statistics is not supported in this kernel > > 11 ffff92c347da5e00 sr0 ffff92bbd9c3e528 0 0 0 > > 253 ffff92bbca726e00 dm-0 ffff92bbcad7dd60 0 0 0 > > > > What do you think? > > > > I agree with you that it should be good to have the above information > for users, but it looks like the statistics result is redundant and > the readability is not very good. What do you think of removing that > statistical result and only outputting prompt information? For > example: > > crash> dev -d > MAJOR GENDISK NAME REQUEST_QUEUE TOTAL ASYNC SYNC > dev: sda: Disk I/O statistics is not supported in this kernel > dev: sr0: Disk I/O statistics is not supported in this kernel > dev: dm-0: Disk I/O statistics is not supported in this kernel > dev: dm-1: Disk I/O statistics is not supported in this kernel > dev: dm-2: Disk I/O statistics is not supported in this kernel I would prefer to display information like the addresses of request_queue as far as we can. No statistics, but still we will often want to see the structures. So for example: crash> dev -d MAJOR GENDISK NAME REQUEST_QUEUE TOTAL ASYNC SYNC 8 ffff92bbd102e400 sdb ffff92bbf04e3678 (not supported) 8 ffff92bbd1014400 sdc ffff92bbf04e26e8 0 0 0 ... crash> request_queue ffff92bbf04e3678 | head -n 3 struct request_queue { last_merge = 0x0, elevator = 0xffff92bbc449b000, Is this possible?

> > How about the following changes? > --- > dev.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/dev.c b/dev.c > index effe789f38d8..3cf70c41cd17 100644 > --- a/dev.c > +++ b/dev.c > @@ -4465,6 +4465,12 @@ display_one_diskio(struct iter *i, unsigned > long gendisk, ulong flags) > if (is_skipped_disk(disk_name)) > return; > > + if (!MEMBER_EXISTS("blk_mq_ctx", "rq_dispatched") && > + !MEMBER_EXISTS("blk_mq_ctx", "rq_completed")) { > + fprintf(fp, "dev: %s: Disk I/O statistics is not > supported in this kernel\n", disk_name); > + return; > + } > + > readmem(gendisk + OFFSET(gendisk_queue), KVADDR, &queue_addr, > sizeof(ulong), "gen_disk.queue", FAULT_ON_ERROR); > readmem(gendisk + OFFSET(gendisk_major), KVADDR, &major, sizeof(int), > -- > > > Thanks. > Lianbo > > > > > > > > > Signed-off-by: Lianbo Jiang <lijiang(a)redhat.com&gt; > > > --- > > > dev.c | 4 ++++ > > > 1 file changed, 4 insertions(+) > > > > > > diff --git a/dev.c b/dev.c > > > index effe789f38d8..dd21511e5dfc 100644 > > > --- a/dev.c > > > +++ b/dev.c > > > @@ -4246,6 +4246,10 @@ get_mq_diskio(unsigned long q, unsigned long *mq_count) > > > unsigned long mctx_addr; > > > struct diskio tmp; > > > > > > + if (!MEMBER_EXISTS("blk_mq_ctx", "rq_dispatched") && > > > + !MEMBER_EXISTS("blk_mq_ctx", "rq_completed")) > > > + return; > > > + > > > memset(&tmp, 0x00, sizeof(struct diskio)); > > > > > > readmem(q + OFFSET(request_queue_queue_ctx), KVADDR, &queue_ctx, > > > -- > > > 2.20.1 > >