[PATCH] sched/fair: use rq_clock_task() in update_tg_load_avg() rate-limit

[Rik van Riel]

update_tg_load_avg() is called once per leaf cfs_rq from the
__update_blocked_fair() walk that runs inside the NOHZ idle-balance
softirq, and again from update_load_avg() with UPDATE_TG.  Its first
operation after the trivial early-outs is unconditionally:

	now = sched_clock_cpu(cpu_of(rq_of(cfs_rq)));
	if (now - cfs_rq->last_update_tg_load_avg < NSEC_PER_MSEC)
		return;

Jakub ran into a system where nohz_idle_balance() was taking 75%
of a CPU (which is handling network traffic and doing many irq_exit_cpu
calls), with 35% of that CPU spent in update_load_avg, and 17% of the
CPU in sched_clock_cpu(), reading the TSC.

There are some optimizations upstream already to reduce that overhead,
but it also looks like those rdtsc calls may not me necessary at all,
giving another easy win.

Switch the rate-limit to read rq_clock_task(rq_of(cfs_rq)) instead.
This does two things:

1. Eliminates the rdtsc.  rq->clock_task is already updated by the
   enclosing update_rq_clock(rq), sits in a hot cacheline, and reads
   as a single load.

2. Aligns the rate-limit clock with the clock the rate-limited data
   is computed on.  cfs_rq->avg.load_avg (the value being published
   to tg->load_avg) is computed by
     update_cfs_rq_load_avg(cfs_rq_clock_pelt(cfs_rq), cfs_rq)
   where cfs_rq_clock_pelt() is derived from rq->clock_pelt, which is
   derived from rq->clock_task.  PELT intentionally excludes IRQ-handling
   time and steal time from its decay (commit 23127296889f
   "sched/fair: Update scale invariance of PELT"), so
   cfs_rq->avg.load_avg already evolves in clock_task time.  The
   rate-limit was the only outlier in the propagation chain using a
   different clock (wall time, via sched_clock_cpu).

   Under normal load and on idle CPUs (where clock_task advances at
   wall-clock rate) behaviour is unchanged.  Under heavy IRQ load
   clock_task advances slower than wall time, so the rate-limit fires
   less often -- consistent with the fact that the underlying
   cfs_rq->avg.load_avg is also changing slower under the same
   conditions.  The publish cadence tracks the signal.

Note: update_tg_load_avg() propagates the *already-decayed*
cfs_rq->avg.load_avg to tg->load_avg; it does not drive decay.  Decay
happens in update_cfs_rq_load_avg() on the PELT clock regardless of
what clock the rate-limit uses, so this change cannot lose decay
information.  The rate-limit governs how often we publish, not how
fast load decays.

All callers of update_tg_load_avg() and clear_tg_load_avg() hold
rq->lock and have called update_rq_clock(rq) within microseconds:

  caller                                   pre-state
  __update_blocked_fair                    encloser did update_rq_clock(rq)
  update_load_avg's three UPDATE_TG sites  under rq->lock after enqueue/dequeue/update_curr
  attach_/detach_entity_cfs_rq             preceded by update_load_avg(...)
  clear_tg_load_avg via offline path       rq_clock_start_loop_update(rq) upfront

so rq->clock_task is fresh at every call.  Since cfs_rqs are per-CPU
per-task_group, cfs_rq->last_update_tg_load_avg is always compared
against the same rq's clock; no cross-rq drift.

The same hoisting pattern was recently applied to find_new_ilb() in
commit 76504bce4ee6 ("sched/fair: Get this cpu once in find_new_ilb()").

Signed-off-by: Rik van Riel <riel@xxxxxxxxxxx>
Assisted-by: Claude (Anthropic)
---
 kernel/sched/fair.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 863de57a8a2c..096bcb00fa62 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -4429,8 +4429,15 @@ static inline void update_tg_load_avg(struct cfs_rq *cfs_rq)
 	/*
 	 * For migration heavy workloads, access to tg->load_avg can be
 	 * unbound. Limit the update rate to at most once per ms.
-	 */
-	now = sched_clock_cpu(cpu_of(rq_of(cfs_rq)));
+	 *
+	 * The enclosing PELT update paths always hold rq->lock and have
+	 * called update_rq_clock(rq) within microseconds, so rq->clock_task
+	 * is fresh.  Use it instead of sched_clock_cpu() to avoid an rdtsc
+	 * (plus pipeline serialisation) per call -- this function is invoked
+	 * once per leaf cfs_rq in __update_blocked_fair(), so on hosts with
+	 * many cgroups the rdtsc cost dominates the rate-limit check itself.
+	 */
+	now = rq_clock_task(rq_of(cfs_rq));
 	if (now - cfs_rq->last_update_tg_load_avg < NSEC_PER_MSEC)
 		return;
 
@@ -4453,7 +4460,8 @@ static inline void clear_tg_load_avg(struct cfs_rq *cfs_rq)
 	if (cfs_rq->tg == &root_task_group)
 		return;
 
-	now = sched_clock_cpu(cpu_of(rq_of(cfs_rq)));
+	/* See update_tg_load_avg() for the rq_clock_task() rationale. */
+	now = rq_clock_task(rq_of(cfs_rq));
 	delta = 0 - cfs_rq->tg_load_avg_contrib;
 	atomic_long_add(delta, &cfs_rq->tg->load_avg);
 	cfs_rq->tg_load_avg_contrib = 0;
-- 
2.53.0-Meta