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

[lijiang]

On Fri, Dec 24, 2021 at 3:34 PM HAGIO KAZUHITO(萩尾　一仁)
<k-hagio-ab at nec.com> wrote:
>
> > -----Original Message-----
> > 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
>
> 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?
>
Yes, thanks for the suggestion, it looks good. Will post v2 later.

crash> dev -d
MAJOR GENDISK             NAME       REQUEST_QUEUE      TOTAL ASYNC  SYNC
    8        ffff996f81fb1800   sda            ffff996f82545750
    (not supported)
   11       ffff99722a8d1000  sr0            ffff996f82540fe0
   (not supported)
  253      ffff9970b4671c00  dm-0         ffff9970b6505750
(not supported)
  253      ffff9970b3493200  dm-1         ffff9970b5d8e730
(not supported)
  253      ffff99722a8d5c00  dm-2         ffff996f851a5750
(not supported)

> >
> > 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
> > >