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

[hanjinke]

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