On 2022/05/27 15:32, lijiang wrote:
>      >> +    /* static_rqs */
>      >> +    ret = 0;
>      >> +    addr = tag + OFFSET(blk_mq_tags_static_rqs);
>      >> +    if (!readmem(addr, KVADDR, &rqs_addr, sizeof(void *), "blk_mq_tags.static_rqs", RETURN_ON_ERROR))
>      >> +            return ret;
>      >> +    addr = rqs_addr + bitnr * sizeof(ulong); /* static_rqs[bitnr] */
>      >> +    if (!readmem(addr, KVADDR, &rq, sizeof(ulong), "blk_mq_tags.static_rqs[]", RETURN_ON_ERROR))
>      >> +            return ret;
>      >> +    ret = tag_iter->fn(rq, tag_iter->data);
>      >> +
>      >> +    return ret;
>      >
>      > I'm not familiar with blk-mq, I'd like some information.
>      >
>      > I think that the "dev -d" displays the same inflight stats in /proc/diskstats
>      > for devices without mq:
>      >
>      >     diskstats_show
>      >       part_in_flight
>      >         count up per_cpu disk_stats->in_flight
>      >
>      > so, if we do the same thing for mq devices, we should imitate this path:
>      >
>      >     diskstats_show
>      >       blk_mq_in_flight
>      >         blk_mq_queue_tag_busy_iter
>      >           queue_for_each_hw_ctx
>      >             bt_for_each
>      >               sbitmap_for_each_set
>      >                 bt_iter
>      >                   blk_mq_check_inflight
>      >
>      > On the path, I cannot see the static_rqs being counted.
>      > Why does it need to be counted?
>      >
>
> You are right. Basically, the implementation imitates the above path in the kernel. But not exactly, there are
> two changes in this patch:
> [1] add additional statistics for the static_rqs[] and sched_tags
> [2] the calculate_disk_io() imitates the blk_mq_check_inflight(), but doesn't check if the status of rq is in flight.
>
> For the [1], because the rqs[] only contains the requests from the driver tag, once the io scheduler is used, crash
> may miss the check of sched_tags and can not watch its value . You mean that crash shouldn't cover this case,
> or do I misunderstand the sched_tags and static_rqs[]?

hmm, if so, I would like to know the reason why kernel doesn't count them.

> For the [2], the intent is to calculate all requests in the current queue from the bitmap(not only inflight, but also handling
> rq), that can help to get actual read and write request counts. But, to be honest, I'm not sure whether this is a reasonable
> method. Are you concerned that it might have repeat read/write request counts? Or should the crash follow the above
> kernel path exactly? Any idea about this?

Yes, it's one of my concerns.  It does not count a request twice?

IMHO, basically crash should follow the kernel path exactly, including the
check for MQ_RQ_IN_FLIGHT.  The "dev -d" option displays the same stats as
/proc/diskstats for non-mq devices, so displaying different stats for mq
devices will be confusing.  And it will make tracking kernel changes easier.

Thanks,
Kazu