Re: [External] Re: [PATCH v2] blk-throttle: Fix io statistics for cgroup v1

From: hanjinke
Date: Mon Apr 03 2023 - 13:57:09 EST




在 2023/4/3 下午11:30, Michal Koutný 写道:
On Sat, Apr 01, 2023 at 05:47:08PM +0800, Jinke Han <hanjinke.666@xxxxxxxxxxxxx> wrote:
From: Jinke Han <hanjinke.666@xxxxxxxxxxxxx>

After commit f382fb0bcef4 ("block: remove legacy IO schedulers"),
blkio.throttle.io_serviced and blkio.throttle.io_service_bytes become
the only stable io stats interface of cgroup v1,

There is also blkio.bfq.{io_serviced,io_service_bytes} couple, so it's
not the only. Or do you mean stable in terms of used IO scheduler?


Oh, the stable here means that it always exists, and when the bfq scheduler is not used, the bfq interface may not exist.

and these statistics are done in the blk-throttle code. But the
current code only counts the bios that are actually throttled. When
the user does not add the throttle limit,

... "or the limit doesn't kick in"


Agree.

the io stats for cgroup v1 has nothing.


I fix it according to the statistical method of v2, and made it count
all ios accurately.

s/all ios/all bios and split ios/

(IIUC you fix two things)

Fixes: a7b36ee6ba29 ("block: move blk-throtl fast path inline")

Good catch.

Does it also undo the performance gain from that commit? (Or rather,
have you observed effect of your patch on v2-only performance?)


Under v1, this statistical overhead is unavoidable. Under v2, the static key is friendly to judging branches, so I think the performance difference before and after the patch is negligible.

Signed-off-by: Jinke Han <hanjinke.666@xxxxxxxxxxxxx>
---
block/blk-cgroup.c | 6 ++++--
block/blk-throttle.c | 6 ------
block/blk-throttle.h | 9 +++++++++
3 files changed, 13 insertions(+), 8 deletions(-)

The code looks correct.

Thanks,
Michal

Thanks.