Re: [PATCH V3 2/8] md: add io accounting for raid0 and raid5

On Wed, May 26, 2021 at 7:00 PM Guoqing Jiang [off-list ref] wrote:

[...]

quoted

quoted

(bioset_exit -> bioset_integrity_free) is triggered.

I thought we probably need a comment here given it is not explicit.

I think it is better to handle it within this function. Does it have
any downside
to call bioset_integrity_free(&mddev->bio_set) here?

[...]

md_run has to deal with failure path by call bioset_exit, which
already call bioset_integrity_free implicitly. Why the additional
call of bioset_integrity_free would be helpful? Or do you want
to remove bioset_exit from md_run as well?

I guess you are right. Removing this one.

quoted

quoted

quoted

quoted

+
+       if (!blk_queue_io_stat((*bio)->bi_bdev->bd_disk->queue))
+               return;

Added blk_queue_flag_set(QUEUE_FLAG_IO_STAT, mddev->queue); to md_run.
We still need it as md doesn't use mq. Without it, the default iostats is 0.

It enables io accounting by default, so raid5 and raid0 users have to
disable it if they don't want the additional latency.

iostats was on by default before this set, as we didn't check
blk_queue_io_stat().
So it is better to keep the same behavior.

Could you point the place where md enables iostats before the set?
I can't find relevant code for it.

Before this set, we did not set QUEUE_FLAG_IO_STAT, and we didn't
check blk_queue_io_stat() either. So even with /sys/block/mdX/queue/iostats
of 0, we still get iostats for the md device. By setting QUEUE_FLAG_IO_STAT
with the patch, the users still get iostats working by default.

Does this make sense?
Thanks,
Song