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 -- Crash-utility mailing list Crash-utility@xxxxxxxxxx https://listman.redhat.com/mailman/listinfo/crash-utility Contribution Guidelines: https://github.com/crash-utility/crash/wiki
