Skip to contentSkip to conversation

SCHED_DEADLINE server infrastructure

PATCH Superseded Daniel Bristot de Oliveira 3 participants 17 messages Apr 5, 2024 – May 2, 2024 Scheduler

v6 of 7 Superseded Latest is v7 →

lore
Share & tools
lore
#0 lore raw

v6 cover SCHED_DEADLINE server infrastructure

This is v6 of Peter's SCHED_DEADLINE server infrastructure implementation [1].

SCHED_DEADLINE servers can help fixing starvation issues of low priority tasks (e.g., SCHED_OTHER) when higher priority tasks monopolize CPU cycles. Today we have RT Throttling; DEADLINE servers should be able to replace and improve that.

In the v1 there was discussion raised about the consequence of using deadline based servers on the fixed-priority workloads. For a demonstration here is the baseline of timerlat scheduling latency as-is, with kernel build background workload:

 # rtla timerlat top -u -d 10m

--------------------- %< ------------------------
Timer Latency

  0 01:42:24   |          IRQ Timer Latency (us)        |         Thread Timer Latency (us)      |    Ret user Timer Latency (us)
CPU COUNT      |      cur       min       avg       max |      cur       min       avg       max |      cur       min       avg       max
  0 #6143559   |        0         0         0        92 |        2         1         3        98 |        4         1         5       100
  1 #6143559   |        1         0         0        97 |        7         1         5       101 |        9         1         7       103
  2 #6143559   |        0         0         0        88 |        3         1         5        95 |        5         1         7        99
  3 #6143559   |        0         0         0        90 |        6         1         5       103 |       10         1         7       126
  4 #6143558   |        1         0         0        81 |        7         1         4        86 |        9         1         7        90
  5 #6143558   |        0         0         0        74 |        3         1         5        79 |        4         1         7        83
  6 #6143558   |        0         0         0        83 |        2         1         5        89 |        3         0         7       108
  7 #6143558   |        0         0         0        85 |        3         1         4       126 |        5         1         6       137

--------------------- >% ------------------------

And this is the same tests with DL server activating without any delay:
--------------------- %< ------------------------

  0 00:10:01   |          IRQ Timer Latency (us)        |         Thread Timer Latency (us)      |    Ret user Timer Latency (us)
CPU COUNT      |      cur       min       avg       max |      cur       min       avg       max |      cur       min       avg       max
  0 #579147    |        0         0         0        54 |        2         1        52     61095 |        2         2        56     61102
  1 #578766    |        0         0         0        83 |        2         1        49     55824 |        3         2        53     55831
  2 #578559    |        0         0         1        59 |        2         1        50     55760 |        3         2        54     55770
  3 #578318    |        0         0         0        76 |        2         1        49     55751 |        3         2        54     55760
  4 #578611    |        0         0         0        64 |        2         1        49     55811 |        3         2        53     55820
  5 #578347    |        0         0         1        40 |        2         1        50     56121 |        3         2        55     56133
  6 #578938    |        0         0         1        75 |        2         1        49     55755 |        3         2        53     55764
  7 #578631    |        0         0         1        36 |        3         1        51     55528 |        4         2        55     55541

--------------------- >% ------------------------

The problem with DL server only implementation is that FIFO tasks might suffer preemption from NORMAL even when spare CPU cycles are available. In fact, fair deadline server is enqueued right away when NORMAL tasks wake up and they are first scheduled by the server, thus potentially preempting a well behaving FIFO task. This is of course not ideal.

We had discussions about it, and one of the possibilities would be using a different scheduling algorithm for this. But IMHO that is an overkill.

Juri and I discussed this and though about delaying the server activation for the (period - runtime), thus enabling the server only if the fair scheduler is about to starve.

The patch 2 adds the possibility to defer the server start to the (absolute deadline - runtime) point in time. This is achieved by starting the dl server throttled, with a next replenishing time set to activate the server at (absolute deadline - runtime).

The server is enqueued with the runtime replenished. As the fair scheduler runs without boost, its runtime is consumed. If the fair server has its runtime before the runtime - deadline time, the a new period is set, and the timer armed for the new deadline.

The patch 3 add a per_rq interface for the knobs:
fair_server_runtime (950 ms)
fair_server_period (1s)
fair_server_defer (enabled)

With defer enabled on CPUs [0:3], the results get better, having a behavior similar to the one we have with the rt throttling.

--------------------- %< ------------------------
Timer Latency

  0 00:10:01   |          IRQ Timer Latency (us)        |         Thread Timer Latency (us)      |    Ret user Timer Latency (us)
CPU COUNT      |      cur       min       avg       max |      cur       min       avg       max |      cur       min       avg       max
  0 #599979    |        0         0         0        64 |        4         1         4        67 |        6         1         5        69
  1 #599979    |        0         0         1        17 |        6         1         5        50 |       10         2         7        71
  2 #599984    |        1         0         1        22 |        4         1         5        78 |        5         2         7       107
  3 #599986    |        0         0         1        72 |        7         1         5        79 |       10         2         7        82
  4 #581580    |        1         0         1        37 |        6         1        38     52797 |       10         2        41     52805
  5 #583270    |        1         0         1        41 |        9         1        36     52617 |       12         2        38     52623
  6 #581240    |        0         0         1        25 |        7         1        39     52870 |       11         2        41     52876
  7 #581208    |        0         0         1        69 |        6         1        39     52917 |        9         2        41     52923

--------------------- >% ------------------------

Here are some osnoise measurement, with osnoise threads running as FIFO:1 with different setups (defer enabled):
- CPU 2 isolated
- CPU 3 isolated shared with a CFS busy loop task
- CPU 8 non-isolated
- CPU 9 non-isolated shared with a CFS busy loop task

--------------------- %< ------------------------
~# pgrep ktimer | while read pid; do chrt -p -f 2 $pid; done # for RT kernel ~# sysctl kernel.sched_rt_runtime_us=-1
~# tuna isolate -c 2
~# tuna isolate -c 3
~# taskset -c 3 ./f &
~# taskset -c 9 ./f &
~# osnoise -P f:1 -c 2,3,8,9 -T 1 -d 10m -H 1
Operating System Noise
duration: 0 00:10:00 | time is in us
CPU Period Runtime Noise % CPU Aval Max Noise Max Single HW NMI IRQ Softirq Thread

  2 #599       599000000          178    99.99997          18            2           0            0          270            0            0
  3 #598       598054434     31351553    94.75774      104442       104442           0            0      2837523            0         1794
  8 #599       599000001       567456    99.90526        3260         2375           2           89       620490            0        13539
  9 #598       598021196     31742537    94.69207       71707        53357           0           90      3411023            0         1762

--------------------- >% ------------------------

the system runs fine!
- no crashes (famous last words)
- FIFO property is kept
- per cpu interface because it is more flexible - and to detach this from the throttling concept.

Global is broken, but it will > /dev/null.

The selftest mentioned in the sched/core patches is here:

	https://lore.kernel.org/all/[email protected]/

Thanks people at google for testing/suggesting:
- Suleiman Souhlal <[email protected]>
- Youssef Esmat <[email protected]>
- Joel Fernandes (Google) <[email protected]>
- Vineeth Pillai <[email protected]>

Changes from V5:
- Fixes DL server for core scheduling (patches 3 and 4) (Joel/Vineeth/Suleiman)
- Add a function to attach the fair server bandwidth to the
new root domain when the rq changes root domain (Daniel)
- Postpone the replenishment timer of the server if the defer reservation could consume runtime while waiting to boost (patch 2) (Daniel)
- Add the running state to defer mode avoid forcing the defer mechanism if the server continues to be activated due to starvation (patch 2) (Daniel)
- Consider idle time as time for the defer server to avoid penalty on RT tasks (patch 2) (Daniel)
- Mark DL server as unthrottled before enqueue (patch 2)(Joel)
- Make start_dl_timer callers more robust (patch 2) (Joel)
- Do not restart the DL server on replenish from timer (patch 2)(Joel)
- Fix Reverse args to dl_time_before in replenish (patch 2) (Suleiman Souhlal)
- Removed the negative runtime optimization (patch 2)
- Start the dl server as disabled in patch 1, enabling it only after removing the RT throttling, to avoid having two mechanism together by default (patch 1) (Daniel).
- Added a check need resched after dl_server_start (patch 1) (Daniel)
- reset dl_server pointer at put_prev_task_balance (patch 1) (Joel)
- Do not include the already merged patches
- Rebased to 6.9-rc2
Changes from V4:
- Enable the server when nr fair tasks is > 0 (peter)
- Consume runtime if the zerolax server is not boosted (peterz)
- Adjust interface to deal with admission control (peterz)
- Rebased to 6.6
Changes from V3:
- Add the defer server (Daniel)
- Add an per rq interface (Daniel with peter's feedback)
- Add an option not defer the server
- Typos and 1-liner fixes (Valentin, Luca, Peter)
- Fair scheduler running on dl server do not account as RT task (Daniel)
- Changed the condition to enable the server (RT & fair tasks) (Daniel)
Changes from v2:
- Refactor/rephrase/typos changes
- Defferable server using throttling
- The server starts when RT && Fair tasks are enqueued
- Interface with runtime/period/defer option
Changes from v1:
- rebased on 6.4-rc1 tip/sched/core

Daniel Bristot de Oliveira (2):
sched/deadline: Deferrable dl server
sched/fair: Fair server interface

Joel Fernandes (Google) (2):
sched/core: Fix priority checking for DL server picks sched/core: Fix picking of tasks for core scheduling with DL server

Peter Zijlstra (2):
sched/fair: Add trivial fair server
sched/rt: Remove default bandwidth control

 include/linux/sched.h   |   6 +-
 kernel/sched/core.c     |  56 +++--
 kernel/sched/deadline.c | 452 ++++++++++++++++++++++++++++++++++------
 kernel/sched/debug.c    | 209 +++++++++++++++++++
 kernel/sched/fair.c     |  66 +++++-
 kernel/sched/idle.c     |   2 +
 kernel/sched/rt.c       | 242 ++++++++++-----------
 kernel/sched/sched.h    |  17 +-
 kernel/sched/topology.c |   8 +
 9 files changed, 843 insertions(+), 215 deletions(-)
Signature · 1 line
2.44.0
#1 lore raw

PATCH v6 1/6 sched/fair: Add trivial fair server

From: Peter Zijlstra <[email protected]>

Use deadline servers to service fair tasks.

This patch adds a fair_server deadline entity which acts as a container for fair entities and can be used to fix starvation when higher priority (wrt fair) tasks are monopolizing CPU(s).

---
 kernel/sched/core.c     | 24 ++++++++++++++++--------
 kernel/sched/deadline.c | 23 +++++++++++++++++++++++
 kernel/sched/fair.c     | 25 +++++++++++++++++++++++++
 kernel/sched/sched.h    |  4 ++++
 4 files changed, 68 insertions(+), 8 deletions(-)
kernel/sched/core.c +16 -8
index 7019a40457a6..04e2270487b7 100644
@@ -6007,6 +6007,14 @@ static void put_prev_task_balance(struct rq *rq, struct task_struct *prev,
 #endif
 
 	put_prev_task(rq, prev);
+
+	/*
+	 * We've updated @prev and no longer need the server link, clear it.
+	 * Must be done before ->pick_next_task() because that can (re)set
+	 * ->dl_server.
+	 */
+	if (prev->dl_server)
+		prev->dl_server = NULL;
 }
 
 /*
@@ -6037,6 +6045,13 @@ __pick_next_task(struct rq *rq, struct task_struct *prev, struct rq_flags *rf)
 			p = pick_next_task_idle(rq);
 		}
 
+		/*
+		 * This is a normal CFS pick, but the previous could be a DL pick.
+		 * Clear it as previous is no longer picked.
+		 */
+		if (prev->dl_server)
+			prev->dl_server = NULL;
+
 		/*
 		 * This is the fast path; it cannot be a DL server pick;
 		 * therefore even if @p == @prev, ->dl_server must be NULL.
@@ -6050,14 +6065,6 @@ __pick_next_task(struct rq *rq, struct task_struct *prev, struct rq_flags *rf)
 restart:
 	put_prev_task_balance(rq, prev, rf);
 
-	/*
-	 * We've updated @prev and no longer need the server link, clear it.
-	 * Must be done before ->pick_next_task() because that can (re)set
-	 * ->dl_server.
-	 */
-	if (prev->dl_server)
-		prev->dl_server = NULL;
-
 	for_each_class(class) {
 		p = class->pick_next_task(rq);
 		if (p)
@@ -10051,6 +10058,7 @@ void __init sched_init(void)
 #endif /* CONFIG_SMP */
 		hrtick_rq_init(rq);
 		atomic_set(&rq->nr_iowait, 0);
+		fair_server_init(rq);
 
 #ifdef CONFIG_SCHED_CORE
 		rq->core = rq;
kernel/sched/deadline.c +23 -0
index a04a436af8cc..db5dc5c09106 100644
@@ -1382,6 +1382,13 @@ static void update_curr_dl_se(struct rq *rq, struct sched_dl_entity *dl_se, s64
 			resched_curr(rq);
 	}
 
+	/*
+	 * The fair server (sole dl_server) does not account for real-time
+	 * workload because it is running fair work.
+	 */
+	if (dl_se == &rq->fair_server)
+		return;
+
 	/*
 	 * Because -- for now -- we share the rt bandwidth, we need to
 	 * account our runtime there too, otherwise actual rt tasks
@@ -1415,15 +1422,31 @@ void dl_server_update(struct sched_dl_entity *dl_se, s64 delta_exec)
 
 void dl_server_start(struct sched_dl_entity *dl_se)
 {
+	struct rq *rq = dl_se->rq;
+
 	if (!dl_server(dl_se)) {
+		/* Disabled */
+		dl_se->dl_runtime = 0;
+		dl_se->dl_deadline = 1000 * NSEC_PER_MSEC;
+		dl_se->dl_period = 1000 * NSEC_PER_MSEC;
+
 		dl_se->dl_server = 1;
 		setup_new_dl_entity(dl_se);
 	}
+
+	if (!dl_se->dl_runtime)
+		return;
+
 	enqueue_dl_entity(dl_se, ENQUEUE_WAKEUP);
+	if (!dl_task(dl_se->rq->curr) || dl_entity_preempt(dl_se, &rq->curr->dl))
+		resched_curr(dl_se->rq);
 }
 
 void dl_server_stop(struct sched_dl_entity *dl_se)
 {
+	if (!dl_se->dl_runtime)
+		return;
+
 	dequeue_dl_entity(dl_se, DEQUEUE_SLEEP);
 }
 
kernel/sched/fair.c +25 -0
index 03be0d1330a6..304697a80e9e 100644
@@ -6722,6 +6722,9 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
 	 */
 	util_est_enqueue(&rq->cfs, p);
 
+	if (!rq->cfs.h_nr_running)
+		dl_server_start(&rq->fair_server);
+
 	/*
 	 * If in_iowait is set, the code below may not trigger any cpufreq
 	 * utilization updates, so do it here explicitly with the IOWAIT flag
@@ -6866,6 +6869,9 @@ static void dequeue_task_fair(struct rq *rq, struct task_struct *p, int flags)
 		rq->next_balance = jiffies;
 
 dequeue_throttle:
+	if (!rq->cfs.h_nr_running)
+		dl_server_stop(&rq->fair_server);
+
 	util_est_update(&rq->cfs, p, task_sleep);
 	hrtick_update(rq);
 }
@@ -8538,6 +8544,25 @@ static struct task_struct *__pick_next_task_fair(struct rq *rq)
 	return pick_next_task_fair(rq, NULL, NULL);
 }
 
+static bool fair_server_has_tasks(struct sched_dl_entity *dl_se)
+{
+	return !!dl_se->rq->cfs.nr_running;
+}
+
+static struct task_struct *fair_server_pick(struct sched_dl_entity *dl_se)
+{
+	return pick_next_task_fair(dl_se->rq, NULL, NULL);
+}
+
+void fair_server_init(struct rq *rq)
+{
+	struct sched_dl_entity *dl_se = &rq->fair_server;
+
+	init_dl_entity(dl_se);
+
+	dl_server_init(dl_se, rq, fair_server_has_tasks, fair_server_pick);
+}
+
 /*
  * Account for a descheduled task:
  */
kernel/sched/sched.h +4 -0
index d2242679239e..205e56929e15 100644
@@ -340,6 +340,8 @@ extern void dl_server_init(struct sched_dl_entity *dl_se, struct rq *rq,
 		    dl_server_has_tasks_f has_tasks,
 		    dl_server_pick_f pick);
 
+extern void fair_server_init(struct rq *rq);
+
 #ifdef CONFIG_CGROUP_SCHED
 
 struct cfs_rq;
@@ -1016,6 +1018,8 @@ struct rq {
 	struct rt_rq		rt;
 	struct dl_rq		dl;
 
+	struct sched_dl_entity	fair_server;
+
 #ifdef CONFIG_FAIR_GROUP_SCHED
 	/* list of leaf cfs_rq on this CPU: */
 	struct list_head	leaf_cfs_rq_list;
Signature · 1 line
2.44.0
maintainer
#2 lore raw

Re: [PATCH V6 1/6] sched/fair: Add trivial fair server

Quoted 55 lines · Daniel Bristot de Oliveira

From: Peter Zijlstra <[email protected]>

Use deadline servers to service fair tasks.

This patch adds a fair_server deadline entity which acts as a container for fair entities and can be used to fix starvation when higher priority (wrt fair) tasks are monopolizing CPU(s).

---
 kernel/sched/core.c     | 24 ++++++++++++++++--------
 kernel/sched/deadline.c | 23 +++++++++++++++++++++++
 kernel/sched/fair.c     | 25 +++++++++++++++++++++++++
 kernel/sched/sched.h    |  4 ++++
 4 files changed, 68 insertions(+), 8 deletions(-)
kernel/sched/core.c +15 -1
index 7019a40457a6..04e2270487b7 100644
@@ -6007,6 +6007,14 @@ static void put_prev_task_balance(struct rq *rq, struct task_struct *prev,
 #endif
 
 	put_prev_task(rq, prev);
+
+	/*
+	 * We've updated @prev and no longer need the server link, clear it.
+	 * Must be done before ->pick_next_task() because that can (re)set
+	 * ->dl_server.
+	 */
+	if (prev->dl_server)
+		prev->dl_server = NULL;
 }
 
 /*
@@ -6037,6 +6045,13 @@ __pick_next_task(struct rq *rq, struct task_struct *prev, struct rq_flags *rf)
 			p = pick_next_task_idle(rq);
 		}
 
+		/*
+		 * This is a normal CFS pick, but the previous could be a DL pick.
+		 * Clear it as previous is no longer picked.
+		 */
+		if (prev->dl_server)
+			prev->dl_server = NULL;
+
 		/*
 		 * This is the fast path; it cannot be a DL server pick;
 		 * therefore even if @p == @prev, ->dl_server must be NULL.
@@ -6050,14 +6065,6 @@ __pick_next_task(struct rq *rq, struct task_struct *prev, struct rq_flags *rf)
 restart:
 	put_prev_task_balance(rq, prev, rf);
 
-	/*

kernel/sched/core.c __pick_next_task(struct rq *rq, struct task_struct *prev, struct rq_flags *rf) · line 6070

Quoted 10 lines
kernel/sched/core.c +0 -7
-	 * We've updated @prev and no longer need the server link, clear it.
-	 * Must be done before ->pick_next_task() because that can (re)set
-	 * ->dl_server.
-	 */
-	if (prev->dl_server)
-		prev->dl_server = NULL;
-
 	for_each_class(class) {
 		p = class->pick_next_task(rq);
 		if (p)

This bit seems like a fix for 63ba8422f876 ("sched/deadline: Introduce deadline servers"), should it be a separate patch?

#3 lore raw

Re: [PATCH V6 1/6] sched/fair: Add trivial fair server

Quoted 55 lines · Peter Zijlstra
Quoted 54 lines · Daniel Bristot de Oliveira

From: Peter Zijlstra <[email protected]>

Use deadline servers to service fair tasks.

This patch adds a fair_server deadline entity which acts as a container for fair entities and can be used to fix starvation when higher priority (wrt fair) tasks are monopolizing CPU(s).

---
 kernel/sched/core.c     | 24 ++++++++++++++++--------
 kernel/sched/deadline.c | 23 +++++++++++++++++++++++
 kernel/sched/fair.c     | 25 +++++++++++++++++++++++++
 kernel/sched/sched.h    |  4 ++++
 4 files changed, 68 insertions(+), 8 deletions(-)
kernel/sched/core.c +15 -0
index 7019a40457a6..04e2270487b7 100644
@@ -6007,6 +6007,14 @@ static void put_prev_task_balance(struct rq *rq, struct task_struct *prev,
 #endif
 
 	put_prev_task(rq, prev);
+
+	/*
+	 * We've updated @prev and no longer need the server link, clear it.
+	 * Must be done before ->pick_next_task() because that can (re)set
+	 * ->dl_server.
+	 */
+	if (prev->dl_server)
+		prev->dl_server = NULL;
 }
 
 /*
@@ -6037,6 +6045,13 @@ __pick_next_task(struct rq *rq, struct task_struct *prev, struct rq_flags *rf)
 			p = pick_next_task_idle(rq);
 		}
 
+		/*
+		 * This is a normal CFS pick, but the previous could be a DL pick.
+		 * Clear it as previous is no longer picked.
+		 */
+		if (prev->dl_server)
+			prev->dl_server = NULL;
+
 		/*
 		 * This is the fast path; it cannot be a DL server pick;
 		 * therefore even if @p == @prev, ->dl_server must be NULL.
@@ -6050,14 +6065,6 @@ __pick_next_task(struct rq *rq, struct task_struct *prev, struct rq_flags *rf)
 restart:
 	put_prev_task_balance(rq, prev, rf);
 
Quoted 14 lines

kernel/sched/core.c __pick_next_task(struct rq *rq, struct task_struct *prev, struct rq_flags *rf) · line 6070

Quoted 11 lines
kernel/sched/core.c +0 -8
-	/*
-	 * We've updated @prev and no longer need the server link, clear it.
-	 * Must be done before ->pick_next_task() because that can (re)set
-	 * ->dl_server.
-	 */
-	if (prev->dl_server)
-		prev->dl_server = NULL;
-
 	for_each_class(class) {
 		p = class->pick_next_task(rq);
 		if (p)

This bit seems like a fix for 63ba8422f876 ("sched/deadline: Introduce deadline servers"), should it be a separate patch?

It was actually reported in a separated patch as a pre-review of this series. So I think you can pick them as fixes already from there, and add the fixes tag?

https://lore.kernel.org/lkml/[email protected]/
https://lore.kernel.org/lkml/[email protected]/

Also add the Reviewed-by me...

-- Daniel

#4 lore raw

PATCH v6 2/6 sched/deadline: Deferrable dl server

In reply to Daniel Bristot de Oliveira

Among the motivations for the DL servers is the real-time throttling mechanism. This mechanism works by throttling the rt_rq after running for a long period without leaving space for fair tasks.

The base dl server avoids this problem by boosting fair tasks instead of throttling the rt_rq. The point is that it boosts without waiting for potential starvation, causing some non-intuitive cases.

For example, an IRQ dispatches two tasks on an idle system, a fair and an RT. The DL server will be activated, running the fair task before the RT one. This problem can be avoided by deferring the dl server activation.

By setting the defer option, the dl_server will dispatch an SCHED_DEADLINE reservation with replenished runtime, but throttled.

The dl_timer will be set for the defer time at (period - runtime) ns from start time. Thus boosting the fair rq at defer time.

If the fair scheduler has the opportunity to run while waiting for defer time, the dl server runtime will be consumed. If the runtime is completely consumed before the defer time, the server will be replenished while still in a throttled state. Then, the dl_timer will be reset to the new defer time

If the fair server reaches the defer time without consuming its runtime, the server will start running, following CBS rules (thus without breaking SCHED_DEADLINE). Then the server will continue the running state (without deferring) until it fair tasks are able to execute as regular fair scheduler (end of the starvation).

---
 include/linux/sched.h   |   3 +
 kernel/sched/deadline.c | 296 ++++++++++++++++++++++++++++++++++------
 kernel/sched/fair.c     |  24 +++-
 kernel/sched/idle.c     |   2 +
 kernel/sched/sched.h    |   4 +-
 5 files changed, 284 insertions(+), 45 deletions(-)
include/linux/sched.h +3 -0
index 3c2abbc587b4..4a405f0e64f8 100644
@@ -643,6 +643,9 @@ struct sched_dl_entity {
 	unsigned int			dl_non_contending : 1;
 	unsigned int			dl_overrun	  : 1;
 	unsigned int			dl_server         : 1;
+	unsigned int			dl_defer	  : 1;
+	unsigned int			dl_defer_armed	  : 1;
+	unsigned int			dl_defer_running  : 1;
 
 	/*
 	 * Bandwidth enforcement timer. Each -deadline task has its
kernel/sched/deadline.c +256 -40

395 lines not on this page · raw message

kernel/sched/fair.c +20 -4
index 304697a80e9e..fdeb4a61575c 100644
@@ -1156,12 +1156,13 @@ s64 update_curr_common(struct rq *rq)
 static void update_curr(struct cfs_rq *cfs_rq)
 {
 	struct sched_entity *curr = cfs_rq->curr;
+	struct rq *rq = rq_of(cfs_rq);
 	s64 delta_exec;
 
 	if (unlikely(!curr))
 		return;
 
-	delta_exec = update_curr_se(rq_of(cfs_rq), curr);
+	delta_exec = update_curr_se(rq, curr);
 	if (unlikely(delta_exec <= 0))
 		return;
 
@@ -1169,8 +1170,19 @@ static void update_curr(struct cfs_rq *cfs_rq)
 	update_deadline(cfs_rq, curr);
 	update_min_vruntime(cfs_rq);
 
-	if (entity_is_task(curr))
-		update_curr_task(task_of(curr), delta_exec);
+	if (entity_is_task(curr)) {
+		struct task_struct *p = task_of(curr);
+
+		update_curr_task(p, delta_exec);
+
+		/*
+		 * Any fair task that runs outside of fair_server should
+		 * account against fair_server such that it can account for
+		 * this time and possibly avoid running this period.
+		 */
+		if (p->dl_server != &rq->fair_server)
+			dl_server_update(&rq->fair_server, delta_exec);
+	}
 
 	account_cfs_rq_runtime(cfs_rq, delta_exec);
 }
@@ -6722,8 +6734,12 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
 	 */
 	util_est_enqueue(&rq->cfs, p);
 
-	if (!rq->cfs.h_nr_running)
+	if (!rq->cfs.h_nr_running) {
+		/* Account for idle runtime */
+		if (!rq->nr_running)
+			dl_server_update_idle_time(rq, rq->curr);
 		dl_server_start(&rq->fair_server);
+	}
 
 	/*
 	 * If in_iowait is set, the code below may not trigger any cpufreq
kernel/sched/idle.c +2 -0
index 6135fbe83d68..5f8806bc6924 100644
@@ -458,12 +458,14 @@ static void wakeup_preempt_idle(struct rq *rq, struct task_struct *p, int flags)
 
 static void put_prev_task_idle(struct rq *rq, struct task_struct *prev)
 {
+	dl_server_update_idle_time(rq, prev);
 }
 
 static void set_next_task_idle(struct rq *rq, struct task_struct *next, bool first)
 {
 	update_idle_core(rq);
 	schedstat_inc(rq->sched_goidle);
+	next->se.exec_start = rq_clock_task(rq);
 }
 
 #ifdef CONFIG_SMP
kernel/sched/sched.h +3 -1
index 205e56929e15..e70e17be83c3 100644
@@ -312,7 +312,7 @@ extern bool __checkparam_dl(const struct sched_attr *attr);
 extern bool dl_param_changed(struct task_struct *p, const struct sched_attr *attr);
 extern int  dl_cpuset_cpumask_can_shrink(const struct cpumask *cur, const struct cpumask *trial);
 extern int  dl_bw_check_overflow(int cpu);
-
+extern s64 dl_scalled_delta_exec(struct rq *rq, struct sched_dl_entity *dl_se, s64 delta_exec);
 /*
  * SCHED_DEADLINE supports servers (nested scheduling) with the following
  * interface:
@@ -340,6 +340,8 @@ extern void dl_server_init(struct sched_dl_entity *dl_se, struct rq *rq,
 		    dl_server_has_tasks_f has_tasks,
 		    dl_server_pick_f pick);
 
+extern void dl_server_update_idle_time(struct rq *rq,
+		    struct task_struct *p);
 extern void fair_server_init(struct rq *rq);
 
 #ifdef CONFIG_CGROUP_SCHED
Signature · 1 line
2.44.0
maintainer
#5 lore raw

Re: [PATCH V6 2/6] sched/deadline: Deferrable dl server

Quoted 18 lines · Daniel Bristot de Oliveira
kernel/sched/deadline.c +14 -0
@@ -874,6 +895,37 @@ static void replenish_dl_entity(struct sched_dl_entity *dl_se)
 		dl_se->dl_yielded = 0;
 	if (dl_se->dl_throttled)
 		dl_se->dl_throttled = 0;
+
+	/*
+	 * If this is the replenishment of a deferred reservation,
+	 * clear the flag and return.
+	 */
+	if (dl_se->dl_defer_armed) {
+		dl_se->dl_defer_armed = 0;
+		return;
+	}
+
+	/*
+	 * A this point, if the deferred server is not armed, and the deadline
+	 * is in the future, if it is not running already, throttle the server
+	 * and arm the defer timer.

kernel/sched/deadline.c static void replenish_dl_entity(struct sched_dl_entity *dl_se) · line 921

Quoted 10 lines
kernel/sched/deadline.c +10 -0
+	 */
+	if (dl_se->dl_defer && !dl_se->dl_defer_running &&
+	    dl_time_before(rq_clock(dl_se->rq), dl_se->deadline - dl_se->runtime)) {
+		if (!is_dl_boosted(dl_se) && dl_se->server_has_tasks(dl_se)) {
+			dl_se->dl_defer_armed = 1;
+			dl_se->dl_throttled = 1;
+			if (!start_dl_timer(dl_se)) {
+				/*
+				 * If for whatever reason (delays), if a previous timer was
+				 *  queued but not serviced, cancel it.

(whitespace damage)

kernel/sched/deadline.c static void replenish_dl_entity(struct sched_dl_entity *dl_se) · line 926

Quoted 5 lines
kernel/sched/deadline.c +5 -0
+				 */
+				hrtimer_try_to_cancel(&dl_se->dl_timer);
+				dl_se->dl_defer_armed = 0;
+				dl_se->dl_throttled = 0;
+			}

This looks funny in that it 'obviously' should only set the variables to 1 on success, but I'm thinking it is this way because the timer (when programming is successful) needs to observe the 1s.

That is, there is an implicit memory ordering here, perhaps put in a comment to avoid someone 'fixing' this later?

Quoted 7 lines
kernel/sched/deadline.c +2 -0
+		}
+	}
 }
 
 /*

kernel/sched/deadline.c static int start_dl_timer(struct sched_dl_entity *dl_se) · line 1125

Quoted 9 lines
kernel/sched/deadline.c +6 -0
@@ -1056,8 +1117,20 @@ static int start_dl_timer(struct sched_dl_entity *dl_se)
 	 * We want the timer to fire at the deadline, but considering
 	 * that it is actually coming from rq->clock and not from
 	 * hrtimer's time base reading.
+	 *
+	 * The deferred reservation will have its timer set to
+	 * (deadline - runtime). At that point, the CBS rule will decide
+	 * if the current deadline can be used, or if a replenishment is
+	 * required to avoid add too much pressure on the system
+	 * (current u > U).

(I wanted to type a comment about how this comment might not do the subtlety justice, OTOH, fixing that might require much more text and become unwieldy, so meh..)

kernel/sched/deadline.c static int start_dl_timer(struct sched_dl_entity *dl_se) · line 1130

Quoted 6 lines
kernel/sched/deadline.c +4 -1
 	 */
-	act = ns_to_ktime(dl_next_period(dl_se));
+	if (dl_se->dl_defer_armed) {
+		WARN_ON_ONCE(!dl_se->dl_throttled);
+		act = ns_to_ktime(dl_se->deadline - dl_se->runtime);
+	} else {
		/* act = deadline - rel-deadline + period */

kernel/sched/deadline.c static int start_dl_timer(struct sched_dl_entity *dl_se) · line 1131

Quoted 1 line
kernel/sched/deadline.c +1 -0
+		act = ns_to_ktime(dl_next_period(dl_se));

I had to look up what that function does, it either needs a better name or I just need more exposure to this code I suppose :-)

Quoted 10 lines
kernel/sched/deadline.c +3 -0
+	}
+
 	now = hrtimer_cb_get_time(timer);
 	delta = ktime_to_ns(now) - rq_clock(rq);
 	act = ktime_add_ns(act, delta);
@@ -1107,6 +1180,64 @@ static void __push_dl_task(struct rq *rq, struct rq_flags *rf)
 #endif
 }
 
+/* a defer timer will not be reset if the runtime consumed was < dl_server_min_res */

kernel/sched/deadline.c static void __push_dl_task(struct rq *rq, struct rq_flags *rf) · line 1193

Quoted 10 lines
kernel/sched/deadline.c +10 -0
+static const u64 dl_server_min_res = 1 * NSEC_PER_MSEC;
+
+static enum hrtimer_restart dl_server_timer(struct hrtimer *timer, struct sched_dl_entity *dl_se)
+{
+	struct rq *rq = rq_of_dl_se(dl_se);
+	enum hrtimer_restart restart = 0;
+	struct rq_flags rf;
+	u64 fw;
+
+	rq_lock(rq, &rf);
	guard(rq_lock)(rq, &rf);
Quoted 11 lines
kernel/sched/deadline.c +11 -0
+	if (dl_se->dl_throttled) {
+		sched_clock_tick();
+		update_rq_clock(rq);
+
+		if (!dl_se->dl_runtime)
+			goto unlock;
+
+		if (!dl_se->server_has_tasks(dl_se)) {
+			replenish_dl_entity(dl_se);
+			goto unlock;
+		}

kernel/sched/deadline.c static void __push_dl_task(struct rq *rq, struct rq_flags *rf) · line 1214

Quoted 10 lines
kernel/sched/deadline.c +10 -0
+
+		if (dl_se->dl_defer_armed) {
+			/*
+			 * First check if the server could consume runtime in background.
+			 * If so, it is possible to push the defer timer for this amount
+			 * of time. The dl_server_min_res serves as a limit to avoid
+			 * forwarding the timer for a too small amount of time.
+			 */
+			if (dl_time_before(rq_clock(dl_se->rq),
+				(dl_se->deadline - dl_se->runtime - dl_server_min_res))) {

:se cino=(0:0

that is, this wants to be something like:

			if (dl_time_before(rq_clock(dl_se->rq),
					   (dl_se->deadline - dl_se->runtime - dl_server_min_res))) {

kernel/sched/deadline.c static void __push_dl_task(struct rq *rq, struct rq_flags *rf) · line 1221

Quoted 8 lines
kernel/sched/deadline.c +7 -0
+
+				/* reset the defer timer */
+				fw = dl_se->deadline - rq_clock(dl_se->rq) - dl_se->runtime;
+
+				hrtimer_forward_now(timer, ns_to_ktime(fw));
 
+				restart = 1;
+				goto unlock;
				return HRTIMER_RESTART;
Quoted 7 lines
kernel/sched/deadline.c +7 -0
+			}
+
+			dl_se->dl_defer_running = 1;
+		}
+
+		enqueue_dl_entity(dl_se, ENQUEUE_REPLENISH);
+

kernel/sched/deadline.c static void __push_dl_task(struct rq *rq, struct rq_flags *rf) · line 1238

Quoted 10 lines
kernel/sched/deadline.c +10 -0
+		if (!dl_task(dl_se->rq->curr) ||
+			dl_entity_preempt(dl_se, &dl_se->rq->curr->dl))
+			resched_curr(rq);
+
+		__push_dl_task(rq, &rf);
+	}
+unlock:
+	rq_unlock(rq, &rf);
+
+	return restart ? HRTIMER_RESTART : HRTIMER_NORESTART;
	return HRTIMER_NORESTART;
Quoted 7 lines
kernel/sched/deadline.c +2 -0
+}
+
 /*
  * This is the bandwidth enforcement timer callback. If here, we know

* a task is not on its dl_rq, since the fact that the timer was running

kernel/sched/deadline.c static u64 grub_reclaim(u64 delta, struct rq *rq, struct sched_dl_entity *dl_se) · line 1434

Quoted 8 lines
kernel/sched/deadline.c +1 -4
@@ -1320,22 +1431,10 @@ static u64 grub_reclaim(u64 delta, struct rq *rq, struct sched_dl_entity *dl_se)
 	return (delta * u_act) >> BW_SHIFT;
 }
 
-static inline void
-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)
+s64 dl_scalled_delta_exec(struct rq *rq, struct sched_dl_entity *dl_se, s64 delta_exec)

%s/_scalled_/_scaled_/g ?

Quoted 47 lines
kernel/sched/deadline.c +26 -9
 {
 	s64 scaled_delta_exec;
 
-	if (unlikely(delta_exec <= 0)) {
-		if (unlikely(dl_se->dl_yielded))
-			goto throttle;
-		return;
-	}
-
-	if (dl_entity_is_special(dl_se))
-		return;
-
 	/*
 	 * For tasks that participate in GRUB, we implement GRUB-PA: the
 	 * spare reclaimed bandwidth is used to clock down frequency.
@@ -1354,8 +1453,64 @@ static void update_curr_dl_se(struct rq *rq, struct sched_dl_entity *dl_se, s64
 		scaled_delta_exec = cap_scale(scaled_delta_exec, scale_cpu);
 	}
 
+	return scaled_delta_exec;
+}
+
+static inline void
+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)
+{
+	s64 scaled_delta_exec;
+
+	if (unlikely(delta_exec <= 0)) {
+		if (unlikely(dl_se->dl_yielded))
+			goto throttle;
+		return;
+	}
+
+	if (dl_server(dl_se) && dl_se->dl_throttled && !dl_se->dl_defer)
+		return;
+
+	if (dl_entity_is_special(dl_se))
+		return;
+
+	scaled_delta_exec = dl_scalled_delta_exec(rq, dl_se, delta_exec);
+
 	dl_se->runtime -= scaled_delta_exec;
 
+	/*
+	 * The fair server can consume its runtime while throttled (not queued/

kernel/sched/deadline.c static void update_curr_dl_se(struct rq *rq, struct sched_dl_entity *dl_se, s64 · line 1493

Quoted 10 lines
kernel/sched/deadline.c +10 -0
+	 * running as regular CFS).
+	 *
+	 * If the server consumes its entire runtime in this state. The server
+	 * is not required for the current period. Thus, reset the server by
+	 * starting a new period, pushing the activation.
+	 */
+	if (dl_se->dl_defer && dl_se->dl_throttled && dl_runtime_exceeded(dl_se)) {
+		/*
+		 * If the server was previously activated - the starving condition
+		 * took place, it this point it went away because the fair scheduler
                               ^ at ?
Quoted 25 lines
kernel/sched/deadline.c +20 -0
+		 * was able to get runtime in background. So return to the initial
+		 * state.
+		 */
+		dl_se->dl_defer_running = 0;
+
+		hrtimer_try_to_cancel(&dl_se->dl_timer);
+
+		replenish_dl_new_period(dl_se, dl_se->rq);
+
+		/*
+		 * 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
+		 * and queue right away. Otherwise nothing might queue it. That's similar
+		 * to what enqueue_dl_entity() does on start_dl_timer==0. For now, just warn.
+		 */
+		WARN_ON_ONCE(!start_dl_timer(dl_se));
+
+		return;
+	}
+
 throttle:
 	if (dl_runtime_exceeded(dl_se) || dl_se->dl_yielded) {
 		dl_se->dl_throttled = 1;
@@ -1415,9 +1570,47 @@ static void update_curr_dl_se(struct rq *rq, struct sched_dl_entity *dl_se, s64
 	}

kernel/sched/deadline.c static void update_curr_dl_se(struct rq *rq, struct sched_dl_entity *dl_se, s64 · line 1580

Quoted 10 lines
kernel/sched/deadline.c +8 -0
 }
 
+/*
+ * In the non-defer mode, the idle time is not accounted, as the
+ * server provides a guarantee.
+ *
+ * If the dl_server is in defer mode, the idle time is also considered
+ * as time available for the fair server. This avoids creating a
+ * regression with the rt throttling behavior where the idle time did
+ * not create a penalty to the rt schedulers.

I don't think it makes sense to refer to rt throttle behaviour here. The goal is to delete that code, at which this comment becomes a hysterical artifact.

I think we can easily give a rational reason for this behaviour without referring current behaviour. That is, the point of all this is to grant fair a chance to run, but if there is no fair task (idle), there is no point in trying to run.

Hmm?

Also, this is done to avoid having to reprogram the timer muck when cfs_rq::nr_running changes to/from 0 ?

Quoted 28 lines
kernel/sched/deadline.c +28 -0
+ */
+void dl_server_update_idle_time(struct rq *rq, struct task_struct *p)
+{
+	s64 delta_exec, scaled_delta_exec;
+
+	if (!rq->fair_server.dl_defer)
+		return;
+
+	/* no need to discount more */
+	if (rq->fair_server.runtime < 0)
+		return;
+
+	delta_exec = rq_clock_task(rq) - p->se.exec_start;
+	if (delta_exec < 0)
+		return;
+
+	scaled_delta_exec = dl_scalled_delta_exec(rq, &rq->fair_server, delta_exec);
+
+	rq->fair_server.runtime -= scaled_delta_exec;
+
+	if (rq->fair_server.runtime < 0) {
+		rq->fair_server.dl_defer_running = 0;
+		rq->fair_server.runtime = 0;
+	}
+
+	p->se.exec_start = rq_clock_task(rq);
+}
+
#6 lore raw

Re: [PATCH V6 2/6] sched/deadline: Deferrable dl server

Quoted 23 lines · Peter Zijlstra
Quoted 22 lines · Daniel Bristot de Oliveira
kernel/sched/deadline.c +18 -0
@@ -874,6 +895,37 @@ static void replenish_dl_entity(struct sched_dl_entity *dl_se)
 		dl_se->dl_yielded = 0;
 	if (dl_se->dl_throttled)
 		dl_se->dl_throttled = 0;
+
+	/*
+	 * If this is the replenishment of a deferred reservation,
+	 * clear the flag and return.
+	 */
+	if (dl_se->dl_defer_armed) {
+		dl_se->dl_defer_armed = 0;
+		return;
+	}
+
+	/*
+	 * A this point, if the deferred server is not armed, and the deadline
+	 * is in the future, if it is not running already, throttle the server
+	 * and arm the defer timer.
+	 */
+	if (dl_se->dl_defer && !dl_se->dl_defer_running &&
+	    dl_time_before(rq_clock(dl_se->rq), dl_se->deadline - dl_se->runtime)) {
+		if (!is_dl_boosted(dl_se) && dl_se->server_has_tasks(dl_se)) {
Quoted 8 lines

kernel/sched/deadline.c static void replenish_dl_entity(struct sched_dl_entity *dl_se) · line 921

Quoted 6 lines
kernel/sched/deadline.c +6 -0
+			dl_se->dl_defer_armed = 1;
+			dl_se->dl_throttled = 1;
+			if (!start_dl_timer(dl_se)) {
+				/*
+				 * If for whatever reason (delays), if a previous timer was
+				 *  queued but not serviced, cancel it.

(whitespace damage)

OOops...

Quoted 10 lines

kernel/sched/deadline.c static void replenish_dl_entity(struct sched_dl_entity *dl_se) · line 926

Quoted 5 lines
kernel/sched/deadline.c +5 -0
+				 */
+				hrtimer_try_to_cancel(&dl_se->dl_timer);
+				dl_se->dl_defer_armed = 0;
+				dl_se->dl_throttled = 0;
+			}

This looks funny in that it 'obviously' should only set the variables to 1 on success, but I'm thinking it is this way because the timer (when programming is successful) needs to observe the 1s.

Quoted 2 lines

That is, there is an implicit memory ordering here, perhaps put in a comment to avoid someone 'fixing' this later?

Yes, I will add a comment.

Quoted 6 lines
Quoted 5 lines
kernel/sched/deadline.c +2 -0
+		}
+	}
 }
 
 /*
Quoted 14 lines

kernel/sched/deadline.c static int start_dl_timer(struct sched_dl_entity *dl_se) · line 1125

Quoted 10 lines
kernel/sched/deadline.c +6 -0
@@ -1056,8 +1117,20 @@ static int start_dl_timer(struct sched_dl_entity *dl_se)
 	 * We want the timer to fire at the deadline, but considering
 	 * that it is actually coming from rq->clock and not from
 	 * hrtimer's time base reading.
+	 *
+	 * The deferred reservation will have its timer set to
+	 * (deadline - runtime). At that point, the CBS rule will decide
+	 * if the current deadline can be used, or if a replenishment is
+	 * required to avoid add too much pressure on the system
+	 * (current u > U).

(I wanted to type a comment about how this comment might not do the subtlety justice, OTOH, fixing that might require much more text and become unwieldy, so meh..)

Yeah, I will write documentation about it, it has a "composed set of reasons" long enough to fill more than a page. I will do it once we get the final version...

Quoted 8 lines

kernel/sched/deadline.c static int start_dl_timer(struct sched_dl_entity *dl_se) · line 1130

Quoted 6 lines
kernel/sched/deadline.c +4 -1
 	 */
-	act = ns_to_ktime(dl_next_period(dl_se));
+	if (dl_se->dl_defer_armed) {
+		WARN_ON_ONCE(!dl_se->dl_throttled);
+		act = ns_to_ktime(dl_se->deadline - dl_se->runtime);
+	} else {
		/* act = deadline - rel-deadline + period */

ack

Quoted 14 lines

kernel/sched/deadline.c static int start_dl_timer(struct sched_dl_entity *dl_se) · line 1131

Quoted 1 line
kernel/sched/deadline.c +1 -0
+		act = ns_to_ktime(dl_next_period(dl_se));

I had to look up what that function does, it either needs a better name or I just need more exposure to this code I suppose :-)

Quoted 9 lines
kernel/sched/deadline.c +2 -0
+	}
+
 	now = hrtimer_cb_get_time(timer);
 	delta = ktime_to_ns(now) - rq_clock(rq);
 	act = ktime_add_ns(act, delta);
@@ -1107,6 +1180,64 @@ static void __push_dl_task(struct rq *rq, struct rq_flags *rf)
 #endif
 }
 
Quoted 13 lines

kernel/sched/deadline.c static void __push_dl_task(struct rq *rq, struct rq_flags *rf) · line 1193

Quoted 11 lines
kernel/sched/deadline.c +11 -0
+/* a defer timer will not be reset if the runtime consumed was < dl_server_min_res */
+static const u64 dl_server_min_res = 1 * NSEC_PER_MSEC;
+
+static enum hrtimer_restart dl_server_timer(struct hrtimer *timer, struct sched_dl_entity *dl_se)
+{
+	struct rq *rq = rq_of_dl_se(dl_se);
+	enum hrtimer_restart restart = 0;
+	struct rq_flags rf;
+	u64 fw;
+
+	rq_lock(rq, &rf);
	guard(rq_lock)(rq, &rf);

Arrhg... I knew it, just did not use it... boooh Daniel.

Quoted 24 lines
Quoted 11 lines
kernel/sched/deadline.c +11 -0
+	if (dl_se->dl_throttled) {
+		sched_clock_tick();
+		update_rq_clock(rq);
+
+		if (!dl_se->dl_runtime)
+			goto unlock;
+
+		if (!dl_se->server_has_tasks(dl_se)) {
+			replenish_dl_entity(dl_se);
+			goto unlock;
+		}

kernel/sched/deadline.c static void __push_dl_task(struct rq *rq, struct rq_flags *rf) · line 1214

Quoted 10 lines
kernel/sched/deadline.c +10 -0
+
+		if (dl_se->dl_defer_armed) {
+			/*
+			 * First check if the server could consume runtime in background.
+			 * If so, it is possible to push the defer timer for this amount
+			 * of time. The dl_server_min_res serves as a limit to avoid
+			 * forwarding the timer for a too small amount of time.
+			 */
+			if (dl_time_before(rq_clock(dl_se->rq),
+				(dl_se->deadline - dl_se->runtime - dl_server_min_res))) {

:se cino=(0:0

Quoted 4 lines

that is, this wants to be something like:

			if (dl_time_before(rq_clock(dl_se->rq),
					   (dl_se->deadline - dl_se->runtime - dl_server_min_res))) {

ack

Quoted 41 lines

kernel/sched/deadline.c static void __push_dl_task(struct rq *rq, struct rq_flags *rf) · line 1221

Quoted 8 lines
kernel/sched/deadline.c +7 -0
+
+				/* reset the defer timer */
+				fw = dl_se->deadline - rq_clock(dl_se->rq) - dl_se->runtime;
+
+				hrtimer_forward_now(timer, ns_to_ktime(fw));
 
+				restart = 1;
+				goto unlock;
				return HRTIMER_RESTART;
Quoted 7 lines
kernel/sched/deadline.c +7 -0
+			}
+
+			dl_se->dl_defer_running = 1;
+		}
+
+		enqueue_dl_entity(dl_se, ENQUEUE_REPLENISH);
+

kernel/sched/deadline.c static void __push_dl_task(struct rq *rq, struct rq_flags *rf) · line 1238

Quoted 10 lines
kernel/sched/deadline.c +10 -0
+		if (!dl_task(dl_se->rq->curr) ||
+			dl_entity_preempt(dl_se, &dl_se->rq->curr->dl))
+			resched_curr(rq);
+
+		__push_dl_task(rq, &rf);
+	}
+unlock:
+	rq_unlock(rq, &rf);
+
+	return restart ? HRTIMER_RESTART : HRTIMER_NORESTART;
	return HRTIMER_NORESTART;
Quoted 10 lines
kernel/sched/deadline.c +2 -0
+}
+
 /*
  * This is the bandwidth enforcement timer callback. If here, we know

* a task is not on its dl_rq, since the fact that the timer was running

kernel/sched/deadline.c +0 -0
@@ -1320,22 +1431,10 @@ static u64 grub_reclaim(u64 delta, struct rq *rq, struct sched_dl_entity *dl_se)
 	return (delta * u_act) >> BW_SHIFT;
 }
 
Quoted 7 lines

kernel/sched/deadline.c static u64 grub_reclaim(u64 delta, struct rq *rq, struct sched_dl_entity *dl_se) · line 1434

Quoted 5 lines
kernel/sched/deadline.c +1 -4
-static inline void
-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)
+s64 dl_scalled_delta_exec(struct rq *rq, struct sched_dl_entity *dl_se, s64 delta_exec)

%s/_scalled_/_scaled_/g ?

Yeah, it is a typo, my head is constantly fighting with: "should I put a single or a double consonant?" because of italian... I often put it when I should not :-).

[...]

Quoted 11 lines

kernel/sched/deadline.c static void update_curr_dl_se(struct rq *rq, struct sched_dl_entity *dl_se, s64 · line 1493

Quoted 9 lines
kernel/sched/deadline.c +9 -0
+	 *
+	 * If the server consumes its entire runtime in this state. The server
+	 * is not required for the current period. Thus, reset the server by
+	 * starting a new period, pushing the activation.
+	 */
+	if (dl_se->dl_defer && dl_se->dl_throttled && dl_runtime_exceeded(dl_se)) {
+		/*
+		 * If the server was previously activated - the starving condition
+		 * took place, it this point it went away because the fair scheduler
                               ^ at ?

at

Quoted 27 lines
Quoted 27 lines
kernel/sched/deadline.c +20 -0
+		 * was able to get runtime in background. So return to the initial
+		 * state.
+		 */
+		dl_se->dl_defer_running = 0;
+
+		hrtimer_try_to_cancel(&dl_se->dl_timer);
+
+		replenish_dl_new_period(dl_se, dl_se->rq);
+
+		/*
+		 * 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
+		 * and queue right away. Otherwise nothing might queue it. That's similar
+		 * to what enqueue_dl_entity() does on start_dl_timer==0. For now, just warn.
+		 */
+		WARN_ON_ONCE(!start_dl_timer(dl_se));
+
+		return;
+	}
+
 throttle:
 	if (dl_runtime_exceeded(dl_se) || dl_se->dl_yielded) {
 		dl_se->dl_throttled = 1;
@@ -1415,9 +1570,47 @@ static void update_curr_dl_se(struct rq *rq, struct sched_dl_entity *dl_se, s64
 	}
 }
 
Quoted 12 lines

kernel/sched/deadline.c static void update_curr_dl_se(struct rq *rq, struct sched_dl_entity *dl_se, s64 · line 1580

Quoted 8 lines
kernel/sched/deadline.c +8 -0
+/*
+ * In the non-defer mode, the idle time is not accounted, as the
+ * server provides a guarantee.
+ *
+ * If the dl_server is in defer mode, the idle time is also considered
+ * as time available for the fair server. This avoids creating a
+ * regression with the rt throttling behavior where the idle time did
+ * not create a penalty to the rt schedulers.

I don't think it makes sense to refer to rt throttle behaviour here. The goal is to delete that code, at which this comment becomes a hysterical artifact.

Agreed!

Quoted 6 lines

I think we can easily give a rational reason for this behaviour without referring current behaviour. That is, the point of all this is to grant fair a chance to run, but if there is no fair task (idle), there is no point in trying to run.

Hmm?

right right, it was worth to explain why in the submission, but it is not worth to keep in the code creating confusion.

Quoted 2 lines

Also, this is done to avoid having to reprogram the timer muck when cfs_rq::nr_running changes to/from 0 ?

When the runtime is totally consumed in idle, the timer will be pushed away on the change 0 to 1. IOW, to avoid boosting cfs when RT is behaving - giving space for !rt.

-- Daniel

#7 lore raw

Re: [PATCH V6 2/6] sched/deadline: Deferrable dl server

In reply to Peter Zijlstra
Quoted 9 lines · Peter Zijlstra

kernel/sched/deadline.c static void __push_dl_task(struct rq *rq, struct rq_flags *rf) · line 1193

Quoted 8 lines
kernel/sched/deadline.c +8 -0
+static enum hrtimer_restart dl_server_timer(struct hrtimer *timer, struct sched_dl_entity *dl_se)
+{
+	struct rq *rq = rq_of_dl_se(dl_se);
+	enum hrtimer_restart restart = 0;
+	struct rq_flags rf;
+	u64 fw;
+
+	rq_lock(rq, &rf);
	guard(rq_lock)(rq, &rf);

this is the way I found to get the &rf... inspired on sched_rr_get_interval():

        scoped_guard (rq_lock, rq) {
                struct rq_flags *rf = &scope.rf;

is that right?
-- Daniel

maintainer
#8 lore raw

Re: [PATCH V6 2/6] sched/deadline: Deferrable dl server

Quoted 11 lines · Daniel Bristot de Oliveira
Quoted 9 lines · Peter Zijlstra

kernel/sched/deadline.c static void __push_dl_task(struct rq *rq, struct rq_flags *rf) · line 1193

Quoted 8 lines
kernel/sched/deadline.c +8 -0
+static enum hrtimer_restart dl_server_timer(struct hrtimer *timer, struct sched_dl_entity *dl_se)
+{
+	struct rq *rq = rq_of_dl_se(dl_se);
+	enum hrtimer_restart restart = 0;
+	struct rq_flags rf;
+	u64 fw;
+
+	rq_lock(rq, &rf);
	guard(rq_lock)(rq, &rf);
Quoted 7 lines

this is the way I found to get the &rf... inspired on sched_rr_get_interval():

        scoped_guard (rq_lock, rq) {
                struct rq_flags *rf = &scope.rf;

is that right?

Yeah, it's a bit ugly, but yes.

#9 lore raw

PATCH v6 3/6 sched/fair: Fair server interface

In reply to Daniel Bristot de Oliveira

Add an interface for fair server setup on debugfs.

Each CPU has three files under /debug/sched/fair_server/cpu{ID}:

- runtime: set runtime in ns
- period: set period in ns
- defer: on/off for the defer mechanism

This then leaves /proc/sys/kernel/sched_rt_{period,runtime}_us to set bounds on admission control.

The interface also add the server to the dl bandwidth accounting.

---
 kernel/sched/deadline.c | 111 ++++++++++++++++++----
 kernel/sched/debug.c    | 206 ++++++++++++++++++++++++++++++++++++++++
 kernel/sched/sched.h    |   3 +
 kernel/sched/topology.c |   8 ++
 4 files changed, 311 insertions(+), 17 deletions(-)
kernel/sched/deadline.c +94 -17
index 6ea9c05711ce..dd38370aa276 100644
@@ -321,19 +321,12 @@ void sub_running_bw(struct sched_dl_entity *dl_se, struct dl_rq *dl_rq)
 		__sub_running_bw(dl_se->dl_bw, dl_rq);
 }
 
-static void dl_change_utilization(struct task_struct *p, u64 new_bw)
+static void dl_rq_change_utilization(struct rq *rq, struct sched_dl_entity *dl_se, u64 new_bw)
 {
-	struct rq *rq;
-
-	WARN_ON_ONCE(p->dl.flags & SCHED_FLAG_SUGOV);
-
-	if (task_on_rq_queued(p))
-		return;
+	if (dl_se->dl_non_contending) {
+		sub_running_bw(dl_se, &rq->dl);
+		dl_se->dl_non_contending = 0;
 
-	rq = task_rq(p);
-	if (p->dl.dl_non_contending) {
-		sub_running_bw(&p->dl, &rq->dl);
-		p->dl.dl_non_contending = 0;
 		/*
 		 * If the timer handler is currently running and the
 		 * timer cannot be canceled, inactive_task_timer()
@@ -341,13 +334,25 @@ static void dl_change_utilization(struct task_struct *p, u64 new_bw)
 		 * will not touch the rq's active utilization,
 		 * so we are still safe.
 		 */
-		if (hrtimer_try_to_cancel(&p->dl.inactive_timer) == 1)
-			put_task_struct(p);
+		if (hrtimer_try_to_cancel(&dl_se->inactive_timer) == 1) {
+			if (!dl_server(dl_se))
+				put_task_struct(dl_task_of(dl_se));
+		}
 	}
-	__sub_rq_bw(p->dl.dl_bw, &rq->dl);
+	__sub_rq_bw(dl_se->dl_bw, &rq->dl);
 	__add_rq_bw(new_bw, &rq->dl);
 }
 
+static void dl_change_utilization(struct task_struct *p, u64 new_bw)
+{
+	WARN_ON_ONCE(p->dl.flags & SCHED_FLAG_SUGOV);
+
+	if (task_on_rq_queued(p))
+		return;
+
+	dl_rq_change_utilization(task_rq(p), &p->dl, new_bw);
+}
+
 static void __dl_clear_params(struct sched_dl_entity *dl_se);
 
 /*
@@ -1191,6 +1196,11 @@ static enum hrtimer_restart dl_server_timer(struct hrtimer *timer, struct sched_
 	u64 fw;
 
 	rq_lock(rq, &rf);
+
+	if (!dl_se->dl_runtime) {
+		goto unlock;
+	}
+
 	if (dl_se->dl_throttled) {
 		sched_clock_tick();
 		update_rq_clock(rq);
@@ -1617,11 +1627,17 @@ void dl_server_start(struct sched_dl_entity *dl_se)
 {
 	struct rq *rq = dl_se->rq;
 
+	/*
+	 * XXX: the apply do not work fine at the init phase for the
+	 * fair server because things are not yet set. We need to improve
+	 * this before getting generic.
+	 */
 	if (!dl_server(dl_se)) {
 		/* Disabled */
-		dl_se->dl_runtime = 0;
-		dl_se->dl_deadline = 1000 * NSEC_PER_MSEC;
-		dl_se->dl_period = 1000 * NSEC_PER_MSEC;
+		u64 runtime = 0;
+		u64 period = 1000 * NSEC_PER_MSEC;
+
+		dl_server_apply_params(dl_se, runtime, period, 1);
 
 		dl_se->dl_server = 1;
 		dl_se->dl_defer = 1;
@@ -1656,6 +1672,67 @@ void dl_server_init(struct sched_dl_entity *dl_se, struct rq *rq,
 	dl_se->server_pick = pick;
 }
 
+void __dl_server_attach_root(struct sched_dl_entity *dl_se, struct rq *rq)
+{
+	u64 new_bw = dl_se->dl_bw;
+	struct dl_bw *dl_b;
+	int cpu = cpu_of(rq);
+
+	dl_b = dl_bw_of(cpu_of(rq));
+	raw_spin_lock(&dl_b->lock);
+
+	__dl_add(dl_b, new_bw, dl_bw_cpus(cpu));
+
+	raw_spin_unlock(&dl_b->lock);
+}
+
+int dl_server_apply_params(struct sched_dl_entity *dl_se, u64 runtime, u64 period, bool init)
+{
+	u64 old_bw = init ? 0 : to_ratio(dl_se->dl_period, dl_se->dl_runtime);
+	u64 new_bw = to_ratio(period, runtime);
+	struct rq *rq = dl_se->rq;
+	int cpu = cpu_of(rq);
+	struct dl_bw *dl_b;
+	unsigned long cap;
+	int retval = 0;
+	int cpus;
+
+	dl_b = dl_bw_of(cpu);
+	raw_spin_lock(&dl_b->lock);
+	cpus = dl_bw_cpus(cpu);
+	cap = dl_bw_capacity(cpu);
+
+	if (__dl_overflow(dl_b, cap, old_bw, new_bw)) {
+		retval = -EBUSY;
+		goto out;
+	}
+
+	if (init) {
+		__add_rq_bw(new_bw, &rq->dl);
+		__dl_add(dl_b, new_bw, cpus);
+	} else {
+		__dl_sub(dl_b, dl_se->dl_bw, cpus);
+		__dl_add(dl_b, new_bw, cpus);
+
+		dl_rq_change_utilization(rq, dl_se, new_bw);
+	}
+
+	dl_se->dl_runtime = runtime;
+	dl_se->dl_deadline = period;
+	dl_se->dl_period = period;
+
+	dl_se->runtime = 0;
+	dl_se->deadline = 0;
+
+	dl_se->dl_bw = to_ratio(dl_se->dl_period, dl_se->dl_runtime);
+	dl_se->dl_density = to_ratio(dl_se->dl_deadline, dl_se->dl_runtime);
+
+out:
+	raw_spin_unlock(&dl_b->lock);
+
+	return retval;
+}
+
 /*
  * Update the current task's runtime statistics (provided it is still
  * a -deadline task and has not been removed from the dl_rq).
kernel/sched/debug.c +206 -0

220 lines not on this page · raw message

kernel/sched/sched.h +3 -0
index e70e17be83c3..a80a236da57c 100644
@@ -343,6 +343,9 @@ extern void dl_server_init(struct sched_dl_entity *dl_se, struct rq *rq,
 extern void dl_server_update_idle_time(struct rq *rq,
 		    struct task_struct *p);
 extern void fair_server_init(struct rq *rq);
+extern void __dl_server_attach_root(struct sched_dl_entity *dl_se, struct rq *rq);
+extern int dl_server_apply_params(struct sched_dl_entity *dl_se,
+		    u64 runtime, u64 period, bool init);
 
 #ifdef CONFIG_CGROUP_SCHED
 
kernel/sched/topology.c +8 -0
index 99ea5986038c..ecb089c4967f 100644
@@ -517,6 +517,14 @@ void rq_attach_root(struct rq *rq, struct root_domain *rd)
 	if (cpumask_test_cpu(rq->cpu, cpu_active_mask))
 		set_rq_online(rq);
 
+	/*
+	 * Because the rq is not a task, dl_add_task_root_domain() did not
+	 * move the fair server bw to the rd if it already started.
+	 * Add it now.
+	 */
+	if (rq->fair_server.dl_server)
+		__dl_server_attach_root(&rq->fair_server, rq);
+
 	rq_unlock_irqrestore(rq, &rf);
 
 	if (old_rd)
Signature · 1 line
2.44.0
maintainer
#10 lore raw

Re: [PATCH V6 3/6] sched/fair: Fair server interface

Quoted 4 lines · Daniel Bristot de Oliveira

Add an interface for fair server setup on debugfs.

Each CPU has three files under /debug/sched/fair_server/cpu{ID}:

Quoted 8 lines

- runtime: set runtime in ns
- period: set period in ns
- defer: on/off for the defer mechanism

This then leaves /proc/sys/kernel/sched_rt_{period,runtime}_us to set bounds on admission control.

The interface also add the server to the dl bandwidth accounting.

I suppose most people will want to use it like:

for i in /debug/sched/fair_server/cpu*
do

	  echo $PERIOD > ${i}/period
	  ecoh $RUNTIME > ${i}/runtime

done

And I think we agreed to keep this loop in userspace, but memory is vague.

The 'defer' thing is dubious though, I don't suppose anybody would ever want to actually change that, other than you while poking around at this code, right?

#11 lore raw

Re: [PATCH V6 3/6] sched/fair: Fair server interface

Quoted 16 lines · Peter Zijlstra
Quoted 4 lines · Daniel Bristot de Oliveira

Add an interface for fair server setup on debugfs.

Each CPU has three files under /debug/sched/fair_server/cpu{ID}:

Quoted 8 lines

- runtime: set runtime in ns
- period: set period in ns
- defer: on/off for the defer mechanism

This then leaves /proc/sys/kernel/sched_rt_{period,runtime}_us to set bounds on admission control.

The interface also add the server to the dl bandwidth accounting.

I suppose most people will want to use it like:

Quoted 8 lines

for i in /debug/sched/fair_server/cpu*
do

	  echo $PERIOD > ${i}/period
	  ecoh $RUNTIME > ${i}/runtime

done

And I think we agreed to keep this loop in userspace, but memory is vague.

correct, we agreed to keep loop in user-space. It is important to have per-cpu for large systems, like we have at red hat customers (kubernets): each container can have a different setup... and they do. Like, DPDK people would like to keep some few us runtime, while other CPUs it is better to keep the default.

Quoted 3 lines

The 'defer' thing is dubious though, I don't suppose anybody would ever want to actually change that, other than you while poking around at this code, right?

In a setup where all real-time tasks are DL (without fixed-priority tasks (FIFO/RR)) the defer = 0 makes more sense because the bandwidth is reserved anyways, and the DL server would have a relatively low prio (long period).

Believe it or not, we are getting there in some cases with automation systems :-)

If it does not hurt, I would like keep it... Otherwise, we can think about it in the future.

-- Daniel

maintainer
#12 lore raw

Re: [PATCH V6 3/6] sched/fair: Fair server interface

Quoted 9 lines · Daniel Bristot de Oliveira
Quoted 3 lines · Peter Zijlstra

The 'defer' thing is dubious though, I don't suppose anybody would ever want to actually change that, other than you while poking around at this code, right?

In a setup where all real-time tasks are DL (without fixed-priority tasks (FIFO/RR)) the defer = 0 makes more sense because the bandwidth is reserved anyways, and the DL server would have a relatively low prio (long period).

Tell me more -- how is it better in that case? I would think it wouldn't matter much.

#13 lore raw

Re: [PATCH V6 3/6] sched/fair: Fair server interface

Quoted 7 lines · Peter Zijlstra
Quoted 6 lines · Daniel Bristot de Oliveira
Quoted 3 lines · Peter Zijlstra

The 'defer' thing is dubious though, I don't suppose anybody would ever want to actually change that, other than you while poking around at this code, right?

Quoted 6 lines
Quoted 3 lines

In a setup where all real-time tasks are DL (without fixed-priority tasks (FIFO/RR)) the defer = 0 makes more sense because the bandwidth is reserved anyways, and the DL server would have a relatively low prio (long period).

Tell me more -- how is it better in that case? I would think it wouldn't matter much.

I agree, it does not metter much... but, as an exercise...

When we have only FIFO and Fair tasks, we expect the highest priority FIFO task to run first. In this case, the scheduling latency is a major metric. That is why we need to defer. This is most of the cases now.

When we have only DL and fair tasks, there should not be much expectation of which task will have the highest priority because the priority is dynamic (the earliest deadline changes over time... there is no fixed highest priority task). In this case, the response time is the major metric. Given that the bandwidth of the server is reserved and that the default period is long, the dl server would not become the highest priority task often, naturally running in the spare time. Thus, the server can run without defer... without breaking the major metric.

Yeah, it is a remote case... and deferring is also valid as the user will likely want the real DL tasks to run before the fair scheduler... and activate the server only if near starvation (defer). Also, it is hard to think of a system without FIFO/RR tasks for now...

So, yeah... it is a remote case. It is better to remove it now... We can add a comment explaining this possibility for the next generations...

-- Daniel

#14 lore raw

PATCH v6 4/6 sched/core: Fix priority checking for DL server picks

In reply to Daniel Bristot de Oliveira

From: "Joel Fernandes (Google)" <[email protected]>

In core scheduling, a DL server pick (which is CFS task) should be given higher priority than tasks in other classes.

Not doing so causes CFS starvation. A kselftest is added later to demonstrate this. A CFS task that is competing with RT tasks can be completely starved without this and the DL server's boosting completely ignored.

Fix these problems.

---
 kernel/sched/core.c | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)
kernel/sched/core.c +21 -2
index 04e2270487b7..4881e797ae07 100644
@@ -162,6 +162,9 @@ static inline int __task_prio(const struct task_struct *p)
 	if (p->sched_class == &stop_sched_class) /* trumps deadline */
 		return -2;
 
+	if (p->dl_server)
+		return -1; /* deadline */
+
 	if (rt_prio(p->prio)) /* includes deadline */
 		return p->prio; /* [-1, 99] */
 
@@ -191,8 +194,24 @@ static inline bool prio_less(const struct task_struct *a,
 	if (-pb < -pa)
 		return false;
 
-	if (pa == -1) /* dl_prio() doesn't work because of stop_class above */
-		return !dl_time_before(a->dl.deadline, b->dl.deadline);
+	if (pa == -1) { /* dl_prio() doesn't work because of stop_class above */
+		const struct sched_dl_entity *a_dl, *b_dl;
+
+		a_dl = &a->dl;
+		/*
+		 * Since,'a' and 'b' can be CFS tasks served by DL server,
+		 * __task_prio() can return -1 (for DL) even for those. In that
+		 * case, get to the dl_server's DL entity.
+		 */
+		if (a->dl_server)
+			a_dl = a->dl_server;
+
+		b_dl = &b->dl;
+		if (b->dl_server)
+			b_dl = b->dl_server;
+
+		return !dl_time_before(a_dl->deadline, b_dl->deadline);
+	}
 
 	if (pa == MAX_RT_PRIO + MAX_NICE)	/* fair */
 		return cfs_prio_less(a, b, in_fi);
Signature · 1 line
2.44.0
#15 lore raw

PATCH v6 5/6 sched/core: Fix picking of tasks for core scheduling with DL server

In reply to Daniel Bristot de Oliveira

From: "Joel Fernandes (Google)" <[email protected]>

* Use simple CFS pick_task for DL pick_task

DL server's pick_task calls CFS's pick_next_task_fair(), this is wrong because core scheduling's pick_task only calls CFS's pick_task() for evaluation / checking of the CFS task (comparing across CPUs), not for actually affirmatively picking the next task. This causes RB tree corruption issues in CFS that were found by syzbot.

* Make pick_task_fair clear DL server

A DL task pick might set ->dl_server, but it is possible the task will never run (say the other HT has a stop task). If the CFS task is picked in the future directly (say without DL server), ->dl_server will be set. So clear it in pick_task_fair().

This fixes the KASAN issue reported by syzbot in set_next_entity().

(DL refactoring suggestions by Vineeth Pillai).

---
 include/linux/sched.h   |  3 ++-
 kernel/sched/deadline.c | 27 ++++++++++++++++++++++-----
 kernel/sched/fair.c     | 23 +++++++++++++++++++++--
 kernel/sched/sched.h    |  3 ++-
 4 files changed, 47 insertions(+), 9 deletions(-)
include/linux/sched.h +2 -1
index 4a405f0e64f8..b0a5983cf3d1 100644
@@ -673,7 +673,8 @@ struct sched_dl_entity {
 	 */
 	struct rq			*rq;
 	dl_server_has_tasks_f		server_has_tasks;
-	dl_server_pick_f		server_pick;
+	dl_server_pick_f		server_pick_next;
+	dl_server_pick_f		server_pick_task;
 
 #ifdef CONFIG_RT_MUTEXES
 	/*
kernel/sched/deadline.c +22 -5
index dd38370aa276..45fde2fd3a1b 100644
@@ -1665,11 +1665,13 @@ void dl_server_stop(struct sched_dl_entity *dl_se)
 
 void dl_server_init(struct sched_dl_entity *dl_se, struct rq *rq,
 		    dl_server_has_tasks_f has_tasks,
-		    dl_server_pick_f pick)
+		    dl_server_pick_f pick_next,
+		    dl_server_pick_f pick_task)
 {
 	dl_se->rq = rq;
 	dl_se->server_has_tasks = has_tasks;
-	dl_se->server_pick = pick;
+	dl_se->server_pick_next = pick_next;
+	dl_se->server_pick_task = pick_task;
 }
 
 void __dl_server_attach_root(struct sched_dl_entity *dl_se, struct rq *rq)
@@ -2398,7 +2400,12 @@ static struct sched_dl_entity *pick_next_dl_entity(struct dl_rq *dl_rq)
 	return __node_2_dle(left);
 }
 
-static struct task_struct *pick_task_dl(struct rq *rq)
+/*
+ * __pick_next_task_dl - Helper to pick the next -deadline task to run.
+ * @rq: The runqueue to pick the next task from.
+ * @peek: If true, just peek at the next task. Only relevant for dlserver.
+ */
+static struct task_struct *__pick_next_task_dl(struct rq *rq, bool peek)
 {
 	struct sched_dl_entity *dl_se;
 	struct dl_rq *dl_rq = &rq->dl;
@@ -2412,7 +2419,10 @@ static struct task_struct *pick_task_dl(struct rq *rq)
 	WARN_ON_ONCE(!dl_se);
 
 	if (dl_server(dl_se)) {
-		p = dl_se->server_pick(dl_se);
+		if (IS_ENABLED(CONFIG_SMP) && peek)
+			p = dl_se->server_pick_task(dl_se);
+		else
+			p = dl_se->server_pick_next(dl_se);
 		if (!p) {
 			WARN_ON_ONCE(1);
 			dl_se->dl_yielded = 1;
@@ -2427,11 +2437,18 @@ static struct task_struct *pick_task_dl(struct rq *rq)
 	return p;
 }
 
+#ifdef CONFIG_SMP
+static struct task_struct *pick_task_dl(struct rq *rq)
+{
+	return __pick_next_task_dl(rq, true);
+}
+#endif
+
 static struct task_struct *pick_next_task_dl(struct rq *rq)
 {
 	struct task_struct *p;
 
-	p = pick_task_dl(rq);
+	p = __pick_next_task_dl(rq, false);
 	if (!p)
 		return p;
 
kernel/sched/fair.c +21 -2
index fdeb4a61575c..b86bb3f23fb2 100644
@@ -8406,6 +8406,14 @@ static struct task_struct *pick_task_fair(struct rq *rq)
 		cfs_rq = group_cfs_rq(se);
 	} while (cfs_rq);
 
+	/*
+	 * This can be called from directly from CFS's ->pick_task() or indirectly
+	 * from DL's ->pick_task when fair server is enabled. In the indirect case,
+	 * DL will set ->dl_server just after this function is called, so its Ok to
+	 * clear. In the direct case, we are picking directly so we must clear it.
+	 */
+	task_of(se)->dl_server = NULL;
+
 	return task_of(se);
 }
 #endif
@@ -8565,7 +8573,16 @@ static bool fair_server_has_tasks(struct sched_dl_entity *dl_se)
 	return !!dl_se->rq->cfs.nr_running;
 }
 
-static struct task_struct *fair_server_pick(struct sched_dl_entity *dl_se)
+static struct task_struct *fair_server_pick_task(struct sched_dl_entity *dl_se)
+{
+#ifdef CONFIG_SMP
+	return pick_task_fair(dl_se->rq);
+#else
+	return NULL;
+#endif
+}
+
+static struct task_struct *fair_server_pick_next(struct sched_dl_entity *dl_se)
 {
 	return pick_next_task_fair(dl_se->rq, NULL, NULL);
 }
@@ -8576,7 +8593,9 @@ void fair_server_init(struct rq *rq)
 
 	init_dl_entity(dl_se);
 
-	dl_server_init(dl_se, rq, fair_server_has_tasks, fair_server_pick);
+	dl_server_init(dl_se, rq, fair_server_has_tasks, fair_server_pick_next,
+		       fair_server_pick_task);
+
 }
 
 /*
kernel/sched/sched.h +2 -1
index a80a236da57c..b200f09038db 100644
@@ -338,7 +338,8 @@ extern void dl_server_start(struct sched_dl_entity *dl_se);
 extern void dl_server_stop(struct sched_dl_entity *dl_se);
 extern void dl_server_init(struct sched_dl_entity *dl_se, struct rq *rq,
 		    dl_server_has_tasks_f has_tasks,
-		    dl_server_pick_f pick);
+		    dl_server_pick_f pick_next,
+		    dl_server_pick_f pick_task);
 
 extern void dl_server_update_idle_time(struct rq *rq,
 		    struct task_struct *p);
Signature · 1 line
2.44.0
#16 lore raw

PATCH v6 6/6 sched/rt: Remove default bandwidth control

In reply to Daniel Bristot de Oliveira

From: Peter Zijlstra <[email protected]>

Now that fair_server exists, we no longer need RT bandwidth control unless RT_GROUP_SCHED.

Enable fair_server with parameters equivalent to RT throttling.

---
 kernel/sched/core.c     |   9 +-
 kernel/sched/deadline.c |   5 +-
 kernel/sched/debug.c    |   3 +
 kernel/sched/rt.c       | 242 ++++++++++++++++++----------------------
 kernel/sched/sched.h    |   3 +-
 5 files changed, 120 insertions(+), 142 deletions(-)
kernel/sched/core.c +6 -3
index 4881e797ae07..d70bfc7e3e7b 100644
@@ -9988,8 +9988,6 @@ void __init sched_init(void)
 #endif /* CONFIG_RT_GROUP_SCHED */
 	}
 
-	init_rt_bandwidth(&def_rt_bandwidth, global_rt_period(), global_rt_runtime());
-
 #ifdef CONFIG_SMP
 	init_defrootdomain();
 #endif
@@ -10044,8 +10042,13 @@ void __init sched_init(void)
 		init_tg_cfs_entry(&root_task_group, &rq->cfs, NULL, i, NULL);
 #endif /* CONFIG_FAIR_GROUP_SCHED */
 
-		rq->rt.rt_runtime = def_rt_bandwidth.rt_runtime;
 #ifdef CONFIG_RT_GROUP_SCHED
+		/*
+		 * This is required for init cpu because rt.c:__enable_runtime()
+		 * starts working after scheduler_running, which is not the case
+		 * yet.
+		 */
+		rq->rt.rt_runtime = global_rt_runtime();
 		init_tg_rt_entry(&root_task_group, &rq->rt, NULL, i, NULL);
 #endif
 #ifdef CONFIG_SMP
kernel/sched/deadline.c +3 -2
index 45fde2fd3a1b..f0a7f0e43ff0 100644
@@ -1554,6 +1554,7 @@ static void update_curr_dl_se(struct rq *rq, struct sched_dl_entity *dl_se, s64
 	if (dl_se == &rq->fair_server)
 		return;
 
+#ifdef CONFIG_RT_GROUP_SCHED
 	/*
 	 * Because -- for now -- we share the rt bandwidth, we need to
 	 * account our runtime there too, otherwise actual rt tasks
@@ -1578,6 +1579,7 @@ static void update_curr_dl_se(struct rq *rq, struct sched_dl_entity *dl_se, s64
 			rt_rq->rt_time += delta_exec;
 		raw_spin_unlock(&rt_rq->rt_runtime_lock);
 	}
+#endif
 }
 
 /*
@@ -1633,8 +1635,7 @@ void dl_server_start(struct sched_dl_entity *dl_se)
 	 * this before getting generic.
 	 */
 	if (!dl_server(dl_se)) {
-		/* Disabled */
-		u64 runtime = 0;
+		u64 runtime =  50 * NSEC_PER_MSEC;
 		u64 period = 1000 * NSEC_PER_MSEC;
 
 		dl_server_apply_params(dl_se, runtime, period, 1);
kernel/sched/debug.c +3 -0
index 5da3297270cd..2e1f0ecdde38 100644
@@ -935,9 +935,12 @@ void print_rt_rq(struct seq_file *m, int cpu, struct rt_rq *rt_rq)
 	SEQ_printf(m, "  .%-30s: %Ld.%06ld\n", #x, SPLIT_NS(rt_rq->x))
 
 	PU(rt_nr_running);
+
+#ifdef CONFIG_RT_GROUP_SCHED
 	P(rt_throttled);
 	PN(rt_time);
 	PN(rt_runtime);
+#endif
 
 #undef PN
 #undef PU
kernel/sched/rt.c +107 -135

338 lines not on this page · raw message

kernel/sched/sched.h +1 -2
index b200f09038db..fb8826cf80a9 100644
@@ -713,13 +713,13 @@ struct rt_rq {
 #endif /* CONFIG_SMP */
 	int			rt_queued;
 
+#ifdef CONFIG_RT_GROUP_SCHED
 	int			rt_throttled;
 	u64			rt_time;
 	u64			rt_runtime;
 	/* Nests inside the rq lock: */
 	raw_spinlock_t		rt_runtime_lock;
 
-#ifdef CONFIG_RT_GROUP_SCHED
 	unsigned int		rt_nr_boosted;
 
 	struct rq		*rq;
@@ -2475,7 +2475,6 @@ extern void reweight_task(struct task_struct *p, int prio);
 extern void resched_curr(struct rq *rq);
 extern void resched_cpu(int cpu);
 
-extern struct rt_bandwidth def_rt_bandwidth;
 extern void init_rt_bandwidth(struct rt_bandwidth *rt_b, u64 period, u64 runtime);
 extern bool sched_rt_bandwidth_account(struct rt_rq *rt_rq);
 
Signature · 1 line
2.44.0