Re: [PATCH 0/4] block: drain per-cpu latency stats over possible CPUs

=E5=9C=A8 2026/7/20 17:37, Tao Cui =E5=86=99=E9=81=93:

> From: Tao Cui <[email protected]>
>
> Several block-layer latency statistics accumulate samples in per-cpu
> buckets and drain them periodically -- in timer callbacks, or at each
> check / reporting site -- using for_each_online_cpu().  When a CPU that
> holds pending samples is taken offline, that bucket is skipped during
> the drain: the samples are neither accumulated into the current window
> nor, where the drain also resets, cleared.  They sit in the bucket until
> the CPU is brought back online, at which point they are flushed into
> whatever window happens to be running.
>
> The effect cuts both ways and is harmful in each direction:
>
>    - while the CPU is offline, its samples are under-counted, so
>      throttle / latency / scheduling decisions derived from these stats
>      miss the work that actually happened ("should have throttled, but
>      didn't");
>
>    - on re-online, a burst of stale samples lands in a later window and
>      can spuriously trip a throttle or skew a vrate / latency estimate
>      ("shouldn't throttle, but did").
>
> Fix: drain over for_each_possible_cpu() instead.  The per-cpu areas are
> allocated for the full possible set, and the init paths already iterate
> it; an offline CPU has no concurrent writer, so reading and resetting
> its bucket is safe.
>
> Cost is bounded: the extra work is over offline buckets that have no
> writer and -- because each drain sums and resets -- hold a zeroed stat
> after the first post-offline drain.  It amounts to O(possible - online)
> trivial per-cpu reads on a timer / check / show path, alongside the
> per-CPU stat work already done for online CPUs; on systems where all
> possible CPUs are online it is exactly zero.
>
> This is the same one-line mistake in four sites; each patch switches its
> drain loop from for_each_online_cpu() to for_each_possible_cpu():
>
>    - block/blk-stat.c        blk_stat_timer_fn() -- shared infrastructure
>                              that also feeds wbt and blk-mq latency
>    - block/blk-iolatency.c   iolatency_check_latencies() (throttle) and
>                              iolatency_ssd_stat() (io.stat reporting)
>    - block/blk-iocost.c      ioc_lat_stat() -- missed-ppm / rq_wait delta
>    - block/kyber-iosched.c   kyber_timer_fn() -- latency histogram flush
>
> blk-mq.c's for_each_online_cpu() at the hctx-has-online-cpu check is
> correct and left untouched -- it is not a statistics drain.
>
> Tao Cui (4):
>    block/blk-stat: drain per-cpu callback stats over possible CPUs
>    block/blk-iolatency: account per-cpu latency stats over possible CPUs
>    block/blk-iocost: collect per-cpu latency stats over possible CPUs
>    block/kyber-iosched: flush per-cpu latency buckets over possible CPUs
>
>   block/blk-iocost.c     | 2 +-
>   block/blk-iolatency.c  | 4 ++--
>   block/blk-stat.c       | 2 +-
>   block/kyber-iosched.c  | 2 +-
>   4 files changed, 5 insertions(+), 5 deletions(-)
>
> --
> 2.43.0'

For this set:

Reviewed-by: Yu Kuai <[email protected]>

>
--=20
Thanks,
Kuai