-----Original Message----- > Hi, Kazu > Thank you for the comment. > > On Thu, May 12, 2022 at 1:59 PM HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab@xxxxxxx <mailto:k-hagio-ab@xxxxxxx> > > wrote: > > > Hi Lianbo, > > Thank you for working on this, we know the stats are very useful but > it's a tough work to support :-( > > I have a few questions about basic design: > > - Is it possible to re-use sbitmap_for_each_set() etc. in sbitmap.c? > They were implemented like the sbitmap functions in kernel, maybe > we can imitate bt_for_each() in kernel. Otherwise, we will have to > update sbitmap.c and dev.c when a change in sbitmap structure occurs. > or did you find any reason that they cannot be used? > > > > The main reason is that they have different code logic, and not sure if that > is easy to reuse, just like the sbitmap_bitmap_show(), which doesn't reuse > the sbitmap_for_each_set() in the sbitmap.c. ^^ Got it. crash's sbitmap_bitmap_show() imitates kernel's sbitmap_bitmap_show(), they print the whole bitmap: # cat /sys/kernel/debug/block/sda/hctx0/tags_bitmap 00000000: 0000 0000 crash's sbitmap_for_each_set() imitates kernel's sbitmap_for_each_set(), they do callback on each set bit. This time we want to check this like blk_mq_queue_tag_busy_iter(), it should be used. > But if it can be easily reused, that would be a good idea. Yes, if we can imitate bt_for_each(), it will make tracking changes easier. > Note: the following kernel commit(mentioned in the cover-letter) may make > that the sbitmap can not work as expected. > [3] commit <3301bc53358a> ("lib/sbitmap: kill 'depth' from sbitmap_word") Thanks for the info. so do you mean we need to fix sbitmap.c for 5.18? I think this can be done later. > > > > - Is it possible to use the new logic when blk-mq has sbitmap, even if > a kernel has rq_dispatched and rq_completed? > > > > If we would like to replace the rq_dispatched/rq_completed with the sbitmap > to calculate the disk I/O statistics, that might be extra work. > > But anyway, if it's worth doing, we might consider doing it at the next stage. > What do you think? sorry, cannot get it. I just mean this flow: if blk-mq uses sbitmap then get_mq_diskio_from_hw_queues(q, &tmp); return; else if rq_dispatched/rq_completed exists then ... get_one_mctx_diskio(mctx_addr, &tmp); What is the extra work? > > BTW: I will improve the v1 and post v2 later, because I have got some feedback > from Nitin and John pittman. Also discussed with you in another email thread. Thanks, I see. 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
