On 3/17/22 18:36, yukuai (C) wrote:

在 2022/03/17 22:06, Bart Van Assche 写道:

quoted

On 3/17/22 04:26, Yu Kuai wrote:

quoted

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 <redacted>
---
  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.