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

Tao Cui posted 4 patches 4 days, 16 hours ago
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(-)
[PATCH 0/4] block: drain per-cpu latency stats over possible CPUs
Posted by Tao Cui 4 days, 16 hours ago
From: Tao Cui <cuitao@kylinos.cn>

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