Messages in this thread | | | Subject | Re: [PATCH 1/3] block: don't show disk stats if io accounting is disabled | From | "yukuai (C)" <> | Date | Fri, 18 Mar 2022 14:12:19 +0800 |
| |
在 2022/03/18 11:10, Bart Van Assche 写道: > On 3/17/22 18:36, yukuai (C) wrote: >> 在 2022/03/17 22:06, Bart Van Assche 写道: >>> On 3/17/22 04:26, Yu Kuai wrote: >>>> If io accounting is disabled, there is no point to handle such device >>>> in diskstats_show(), and it can be confused for users because all >>>> fields >>>> in iostat are zero while the disk is handling io. >>>> >>>> Signed-off-by: Yu Kuai <yukuai3@huawei.com> >>>> --- >>>> block/genhd.c | 5 +++++ >>>> 1 file changed, 5 insertions(+) >>>> >>>> diff --git a/block/genhd.c b/block/genhd.c >>>> index c3b32c665aec..e5307f512185 100644 >>>> --- a/block/genhd.c >>>> +++ b/block/genhd.c >>>> @@ -937,6 +937,9 @@ ssize_t part_stat_show(struct device *dev, >>>> struct disk_stats stat; >>>> unsigned int inflight; >>>> + if (!blk_queue_io_stat(q)) >>>> + return sprintf(buf, "io accounting is disabled\n"); >>>> + >>> >>> Hmm ... the above looks sub-optimal to me. Has it been considered to >>> return an error code instead or even better to hide the stat >>> attribute if I/O accounting is disabled? The latter can be achieved >>> by modifying disk_visible(). > > >> It's right this way is much better, i'll hide the 'stat' in next >> iteration. > > Please note that modifying disk_visible() only is not sufficient. > sysfs_update_group() needs to be called to trigger a call to > disk_visible() if a variable has changed that affects the return value > of disk_visible().
Hi,
Thanks for your advice, I wonder can the queue flag 'QUEUE_FLAG_IO_STAT' change after the disk is created? I don't see it can change anywhere. In this case, perhaps we don't need to consider calling sysfs_update_group().
> >> BTW, do you have any suggestion about patch 3? > > I need more time to analyze that patch.
Thanks for sending time on the patch.
Kuai
| |