On Fri, Dec 24, 2021 at 1:14 PM HAGIO KAZUHITO(萩尾 一仁)
<k-hagio-ab@xxxxxxx> 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
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@xxxxxxxxxx>
> > ---
> > 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
>
--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/crash-utility
