sched/deadline: Fix dl_server stop condition

Gabriel reported that the dl_server doesn't stop as expected.

The problem was found to be the fact that idle time and fair runtime are
treated equally. Both will count towards dl_server runtime and push the
activation forwards when it is in the zero-laxity wait state.

Notably:

  dl_server_update_idle()
    update_curr_dl_se()
      if (dl_defer && dl_throttled && dl_runtime_exceeded())
        hrtimer_try_to_cancel(); // stop timer
	replenish_dl_new_period()
	  deadline = now + dl_deadline; // fwd period
	  runtime = dl_runtime;
        start_dl_timer(); // restart timer

And while we do want idle time accounted towards the *current* activation of
the dl_server -- after all, a fair task could've ran if we had any -- we don't
necessarily want idle time to cause or push forward an activation.

Introduce dl_defer_idle to make this distinction. It will be set once idle time
pushed the activation forward, once set idle time will only be allowed to
consume any runtime but not push the activation. This will then cause
dl_server_timer() to fire, which will stop the dl_server.

Any non-idle time accounting during this phase will clear dl_defer_idle, so
only a full period of idle will cause the dl_server to stop.

Reported-by: Gabriele Monaco <gmonaco@redhat.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://patch.msgid.link/20251101000057.GA2184199@noisy.programming.kicks-ass.net

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 07576479c0edc2e4b1967fd9783809a78d207522..bb436ee1942d30387a0f44586bb8b440b45ddfb0 100644 (file)
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -685,20 +685,22 @@ struct sched_dl_entity {
         *
         * @dl_server tells if this is a server entity.
         *
-        * @dl_defer tells if this is a deferred or regular server. For
-        * now only defer server exists.
-        *
-        * @dl_defer_armed tells if the deferrable server is waiting
-        * for the replenishment timer to activate it.
-        *
         * @dl_server_active tells if the dlserver is active(started).
         * dlserver is started on first cfs enqueue on an idle runqueue
         * and is stopped when a dequeue results in 0 cfs tasks on the
         * runqueue. In other words, dlserver is active only when cpu's
         * runqueue has atleast one cfs task.
         *
+        * @dl_defer tells if this is a deferred or regular server. For
+        * now only defer server exists.
+        *
+        * @dl_defer_armed tells if the deferrable server is waiting
+        * for the replenishment timer to activate it.
+        *
         * @dl_defer_running tells if the deferrable server is actually
         * running, skipping the defer phase.
+        *
+        * @dl_defer_idle tracks idle state
         */
        unsigned int                    dl_throttled      : 1;
        unsigned int                    dl_yielded        : 1;
@@ -709,6 +711,7 @@ struct sched_dl_entity {
        unsigned int                    dl_defer          : 1;
        unsigned int                    dl_defer_armed    : 1;
        unsigned int                    dl_defer_running  : 1;
+       unsigned int                    dl_defer_idle     : 1;
 
        /*
         * Bandwidth enforcement timer. Each -deadline task has its

diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
index ece25caf379cc3b4146316d6e56edaa931127348..8307f24b8900830c3a0c467039e1683c6ad453ca 100644 (file)
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -1173,6 +1173,11 @@ static enum hrtimer_restart dl_server_timer(struct hrtimer *timer, struct sched_
                 */
                rq->donor->sched_class->update_curr(rq);
 
+               if (dl_se->dl_defer_idle) {
+                       dl_server_stop(dl_se);
+                       return HRTIMER_NORESTART;
+               }
+
                if (dl_se->dl_defer_armed) {
                        /*
                         * First check if the server could consume runtime in background.
@@ -1420,10 +1425,11 @@ s64 dl_scaled_delta_exec(struct rq *rq, struct sched_dl_entity *dl_se, s64 delta
 }
 
 static inline void
-update_stats_dequeue_dl(struct dl_rq *dl_rq, struct sched_dl_entity *dl_se,
-                       int flags);
+update_stats_dequeue_dl(struct dl_rq *dl_rq, struct sched_dl_entity *dl_se, int flags);
+
 static void update_curr_dl_se(struct rq *rq, struct sched_dl_entity *dl_se, s64 delta_exec)
 {
+       bool idle = rq->curr == rq->idle;
        s64 scaled_delta_exec;
 
        if (unlikely(delta_exec <= 0)) {
@@ -1444,6 +1450,9 @@ static void update_curr_dl_se(struct rq *rq, struct sched_dl_entity *dl_se, s64
 
        dl_se->runtime -= scaled_delta_exec;
 
+       if (dl_se->dl_defer_idle && !idle)
+               dl_se->dl_defer_idle = 0;
+
        /*
         * The fair server can consume its runtime while throttled (not queued/
         * running as regular CFS).
@@ -1453,6 +1462,29 @@ static void update_curr_dl_se(struct rq *rq, struct sched_dl_entity *dl_se, s64
         * starting a new period, pushing the activation.
         */
        if (dl_se->dl_defer && dl_se->dl_throttled && dl_runtime_exceeded(dl_se)) {
+               /*
+                * Non-servers would never get time accounted while throttled.
+                */
+               WARN_ON_ONCE(!dl_server(dl_se));
+
+               /*
+                * While the server is marked idle, do not push out the
+                * activation further, instead wait for the period timer
+                * to lapse and stop the server.
+                */
+               if (dl_se->dl_defer_idle && idle) {
+                       /*
+                        * The timer is at the zero-laxity point, this means
+                        * dl_server_stop() / dl_server_start() can happen
+                        * while now < deadline. This means update_dl_entity()
+                        * will not replenish. Additionally start_dl_timer()
+                        * will be set for 'deadline - runtime'. Negative
+                        * runtime will not do.
+                        */
+                       dl_se->runtime = 0;
+                       return;
+               }
+
                /*
                 * If the server was previously activated - the starving condition
                 * took place, it this point it went away because the fair scheduler
@@ -1465,6 +1497,9 @@ static void update_curr_dl_se(struct rq *rq, struct sched_dl_entity *dl_se, s64
 
                replenish_dl_new_period(dl_se, dl_se->rq);
 
+               if (idle)
+                       dl_se->dl_defer_idle = 1;
+
                /*
                 * Not being able to start the timer seems problematic. If it could not
                 * be started for whatever reason, we need to "unthrottle" the DL server
@@ -1590,6 +1625,7 @@ void dl_server_stop(struct sched_dl_entity *dl_se)
        hrtimer_try_to_cancel(&dl_se->dl_timer);
        dl_se->dl_defer_armed = 0;
        dl_se->dl_throttled = 0;
+       dl_se->dl_defer_idle = 0;
        dl_se->dl_server_active = 0;
 }