From: Bart Van Assche <bvanassche@acm.org>
To: "yukuai (C)" <yukuai3@huawei.com>,
axboe@kernel.dk, mpatocka@redhat.com, snitzer@redhat.com
Cc: linux-block@vger.kernel.org, linux-kernel@vger.kernel.org,
yi.zhang@huawei.com
Subject: Re: [PATCH 1/3] block: don't show disk stats if io accounting is disabled
Date: Thu, 17 Mar 2022 20:10:43 -0700 [thread overview]
Message-ID: <132f9c86-5a2b-6770-377a-9323ba344402@acm.org> (raw)
In-Reply-To: <32a69b87-270e-479a-6207-24de3d70e9ed@huawei.com>
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().
> BTW, do you have any suggestion about patch 3?
I need more time to analyze that patch.
Thanks,
Bart.
next prev parent reply other threads:[~2022-03-18 3:10 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-03-17 11:26 [PATCH 0/3] optimizations for io accounting Yu Kuai
2022-03-17 11:26 ` [PATCH 1/3] block: don't show disk stats if io accounting is disabled Yu Kuai
2022-03-17 14:06 ` Bart Van Assche
2022-03-18 1:36 ` yukuai (C)
2022-03-18 3:10 ` Bart Van Assche [this message]
2022-03-18 6:12 ` yukuai (C)
2022-03-25 8:46 ` Christoph Hellwig
2022-03-17 11:26 ` [PATCH 2/3] block: factor out common code for part_stat_show() and diskstats_show() Yu Kuai
2022-03-25 8:46 ` Christoph Hellwig
2022-03-26 1:11 ` yukuai (C)
2022-03-17 11:26 ` [PATCH -next 3/3] block: update nsecs[] in part_get_stat() Yu Kuai
2022-04-01 3:42 ` yukuai (C)
2022-04-08 6:52 ` yukuai (C)
2022-03-25 7:29 ` [PATCH 0/3] optimizations for io accounting yukuai (C)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=132f9c86-5a2b-6770-377a-9323ba344402@acm.org \
--to=bvanassche@acm.org \
--cc=axboe@kernel.dk \
--cc=linux-block@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mpatocka@redhat.com \
--cc=snitzer@redhat.com \
--cc=yi.zhang@huawei.com \
--cc=yukuai3@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).