[Crash-utility] [PATCH] Handle blk_mq_ctx member changes for kernels v5.16-rc1~75^2~44

[lijiang]

On Fri, Dec 24, 2021 at 1:14 PM HAGIO KAZUHITO(萩尾　一仁)
<k-hagio-ab at nec.com> 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 at redhat.com>
> > ---
> >  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
>