[PATCH v3] sched/deadline: Make dl-server nohz full aware

[Ionut Nechita (Wind River)]

From: Ionut Nechita <ionut.nechita@xxxxxxxxxxxxx>

On a nohz_full core, a dl-server has to be armed when an RT task coexists
with runnable CFS (or SCX) tasks, so the lower-priority class still gets
its reserved bandwidth (fair_server default 50ms/1s) instead of being
starved by the RT task.

Doing that the way v2 did, by arming the server and then returning false
from sched_can_stop_tick() for as long as a CFS/SCX task is queued, keeps
the tick running for the whole time that task waits, not just while the
server actually runs. On an isolated nohz_full core that is the more
expensive of the two costs: measured with a SCHED_RR hog and a CFS task
visiting the core about once per second, the tick never stopped again
(200 irq_vectors:local_timer_entry per second, tick_stop reporting
dependency=SCHED), against ~19/s with the fair server disabled.

The deferred server spends most of its period in the zero-laxity wait
state: throttled, off the dl_rq, waiting for its timer. That wait is
driven by an hrtimer and needs no tick. Keep the two concerns separate:

  - sched_can_stop_tick() arms rq->fair_server (and rq->ext_server for
    SCX) when RT coexists with the respective class, but does not force
    the tick for a merely deferred server. A non-deferred start enqueues
    the server immediately; that is rechecked right after via
    rq->dl.dl_nr_running and does keep the tick.

  - inc_dl_tasks()/dec_dl_tasks() call sched_update_tick_dependency()
    for dl_server entities. Servers skip add_nr_running()/
    sub_nr_running(), so until now nothing re-evaluated the tick when
    the timer enqueued the server or when it was throttled again. The
    tick is now started when the server is enqueued, so its runtime is
    enforced from the tick, and dropped again once it leaves the dl_rq.

The servers are deliberately never stopped from sched_can_stop_tick().
dl_server_start()/dl_server_stop() carry the GRUB accounting of
rq->dl.running_bw through the "active contending" / "active non
contending" state machine, with an inactive_timer in between. Stopping a
server from a path that runs on every enqueue and dequeue cycles that
state machine at a rate it is not meant for: a stop that finds the
inactive timer still armed subtracts running_bw that the matching start
did not add back, which underflows running_bw and trips WARN_ON_ONCE()
in __sub_running_bw() and WARN_ON() in task_non_contending(). Teardown is
left to the existing lazy path in __pick_task_dl(), which stops a server
when it has no task of its class to pick. The cost is that a tickless
core keeps taking the server's defer and inactive timers, which is a few
interrupts per period, not a tick.

All of this is confined to nohz_full CPUs: sched_update_tick_dependency()
returns early for housekeeping CPUs, so sched_can_stop_tick() (and with
it the new arming) only runs where the tick can be stopped.

Fixes: 557a6bfc662c ("sched/fair: Add trivial fair server")
Reported-by: David Haufe <dhaufe@xxxxxxxxxxxxxxxxxx>
Closes: https://lore.kernel.org/lkml/CAKJHwtOw_G67edzuHVtL1xC5Vyt6StcZzihtDd0yaKudW=rwVw@xxxxxxxxxxxxxx
Suggested-by: Juri Lelli <juri.lelli@xxxxxxxxxx>
Signed-off-by: Ionut Nechita <ionut.nechita@xxxxxxxxxxxxx>
---
Changes from v2 [1]

- Rework the mechanism following the discussion on v2 [2]. Instead of
  returning false from sched_can_stop_tick() while a CFS/SCX task is
  queued (which pins the tick on the isolated core for the whole wait),
  only arm the server there and let the tick stop while the server is
  deferred. The tick is kept, and dropped again, from
  inc_dl_tasks()/dec_dl_tasks() as the server enters and leaves the
  dl_rq, so it runs only during the server's service window. Measured
  tick rate on the isolated core drops from CONFIG_HZ to ~the service
  window (see commit message).
- Do not stop the servers from sched_can_stop_tick(). Cycling
  dl_server_start()/stop() at enqueue/dequeue rate underflows
  rq->dl.running_bw and trips the WARN_ON_ONCE() in __sub_running_bw()
  and WARN_ON() in task_non_contending(); splats and a reproducer were
  posted in the v2 thread [3]. Teardown is left to the lazy path in
  __pick_task_dl(). As a result dl_servers_stop_all() is no longer
  needed and is dropped.
- Keep the SCX ext_server handling from v2 (arm rq->ext_server when RT
  coexists with SCX tasks).
- Rebased onto tip/sched/core.

Open question for maintainers: because the servers are no longer stopped
here, a tickless isolated core keeps taking the server's defer/inactive
hrtimers (a few interrupts per period, not a tick). Peter's idea in the
v2 thread of marking the dl-server timers PINNED would keep those on the
isolated core itself and address the housekeeping-side wakeups the
original v2 targeted; it was parked pending a remote hrtimer_start_on().
This patch does not depend on it.

The running_bw underflow splats and reproducer mentioned above, and
Peter's PINNED-timer suggestion, are both in the v2 thread [2].

[1] https://lore.kernel.org/lkml/20260513-upstream-fix-dlserver-nohzfull-b4-v2-1-d3e9cbe5c845@xxxxxxxxxx/
[2] https://lore.kernel.org/lkml/aszViUxPLBuH0h7L@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/

 kernel/sched/core.c     | 32 +++++++++++++++++++++++++++++++-
 kernel/sched/deadline.c |  4 ++++
 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 71d3c948e14ae..0177c950a4885 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -1440,10 +1440,40 @@ bool sched_can_stop_tick(struct rq *rq)
 {
 	int fifo_nr_running;
 
-	/* Deadline tasks, even if single, need the tick */
+	/*
+	 * Deadline tasks, even if single, need the tick. This includes a
+	 * dl-server while it is enqueued and serving its class: its runtime
+	 * is enforced from the tick.
+	 */
 	if (rq->dl.dl_nr_running)
 		return false;
 
+	/*
+	 * RT coexists with a lower-priority class: make sure the matching
+	 * dl-server(s) are armed, so CFS/SCX still get their reserved
+	 * bandwidth even though the tick is about to be stopped. While a
+	 * server is only deferred (throttled, waiting for its timer) it is
+	 * not on the dl_rq and needs no tick; once the timer enqueues it,
+	 * inc_dl_tasks() re-evaluates the dependency.
+	 *
+	 * dl_server_start() is a no-op for an already active server (or one
+	 * with no runtime configured), and the servers are never stopped
+	 * from here: their start/stop pair carries the GRUB accounting of
+	 * rq->dl.running_bw, which must not be cycled at enqueue/dequeue
+	 * rate.
+	 */
+	if (rq->rt.rt_nr_running) {
+		if (rq->cfs.h_nr_queued)
+			dl_server_start(&rq->fair_server);
+#ifdef CONFIG_SCHED_CLASS_EXT
+		if (rq->scx.nr_running)
+			dl_server_start(&rq->ext_server);
+#endif
+		/* A non-deferred start enqueues a server right away. */
+		if (rq->dl.dl_nr_running)
+			return false;
+	}
+
 	/*
 	 * If there are more than one RR tasks, we need the tick to affect the
 	 * actual RR behaviour.
diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
index c0ebdcde5fe5d..589c4c881c31b 100644
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -2259,6 +2259,8 @@ void inc_dl_tasks(struct sched_dl_entity *dl_se, struct dl_rq *dl_rq)
 
 	if (!dl_server(dl_se))
 		add_nr_running(rq_of_dl_rq(dl_rq), 1);
+	else
+		sched_update_tick_dependency(rq_of_dl_rq(dl_rq));
 
 	inc_dl_deadline(dl_rq, deadline);
 }
@@ -2271,6 +2273,8 @@ void dec_dl_tasks(struct sched_dl_entity *dl_se, struct dl_rq *dl_rq)
 
 	if (!dl_server(dl_se))
 		sub_nr_running(rq_of_dl_rq(dl_rq), 1);
+	else
+		sched_update_tick_dependency(rq_of_dl_rq(dl_rq));
 
 	dec_dl_deadline(dl_rq, dl_se->deadline);
 }

base-commit: 4a3b51aab6e25244d97936aa65e6d5425adf98e1
-- 
2.43.0