Add a deadline server for sched_ext tasks
Subsystems (4)
Andrea Righi
reviewerPATCH v9 1/1 Add a deadline server for sched_ext tasks
sched_ext tasks can be starved by long-running RT tasks, especially since RT throttling was replaced by deadline servers to boost only SCHED_NORMAL tasks.
Several users in the community have reported issues with RT stalling sched_ext tasks. This is fairly common on distributions or environments where applications like video compositors, audio services, etc. run as RT tasks by default.
Example trace (showing a per-CPU kthread stalled due to the sway Wayland compositor running as an RT task):
runnable task stall (kworker/0:0[106377] failed to run for 5.043s)
...
CPU 0 : nr_run=3 flags=0xd cpu_rel=0 ops_qseq=20646200 pnt_seq=45388738
curr=sway[994] class=rt_sched_class
R kworker/0:0[106377] -5043ms
scx_state/flags=3/0x1 dsq_flags=0x0 ops_state/qseq=0/0
sticky/holding_cpu=-1/-1 dsq_id=0x8000000000000002 dsq_vtime=0 slice=20000000
cpus=01This is often perceived as a bug in the BPF schedulers, but in reality they can't do much: RT tasks run outside their control and can potentially consume 100% of the CPU bandwidth.
Fix this by adding a sched_ext deadline server as well so that sched_ext tasks are also boosted and do not suffer starvation.
Two kselftests are also provided to verify the starvation fixes and bandwidth allocation is correct.
This patchset is also available in the following git branch:
git://git.kernel.org/pub/scm/linux/kernel/git/arighi/linux.git scx-dl-server
Changes in v9:
- Drop the ->balance() logic as its functionality is now integrated into ->pick_task(), allowing dl_server to call pick_task_scx() directly
- Link to v8:
https://lore.kernel.org/all/[email protected]/
Changes in v8:
- Add tj's patch to de-couple balance and pick_task and avoid changing sched/core callbacks to propagate @rf
- Simplify dl_se->dl_server check (suggested by PeterZ)
- Small coding style fixes in the kselftests
- Link to v7: https://lore.kernel.org/all/[email protected]/
Changes in v7:
- Rebased to Linus master
- Link to v6: https://lore.kernel.org/all/[email protected]/
Changes in v6:
- Added Acks to few patches
- Fixes to few nits suggested by Tejun
- Link to v5: https://lore.kernel.org/all/[email protected]/
Changes in v5:
- Added a kselftest (total_bw) to sched_ext to verify bandwidth values from debugfs
- Address comment from Andrea about redundant rq clock invalidation
- Link to v4: https://lore.kernel.org/all/[email protected]/
Changes in v4:
- Fixed issues with hotplugged CPUs having their DL server bandwidth altered due to loading SCX
- Fixed other issues
- Rebased on Linus master
- All sched_ext kselftests reliably pass now, also verified that the total_bw in debugfs (CONFIG_SCHED_DEBUG) is conserved with these patches
- Link to v3: https://lore.kernel.org/all/[email protected]/
Changes in v3:
- Removed code duplication in debugfs. Made ext interface separate
- Fixed issue where rq_lock_irqsave was not used in the relinquish patch
- Fixed running bw accounting issue in dl_server_remove_params
- Link to v2: https://lore.kernel.org/all/[email protected]/
Changes in v2:
- Fixed a hang related to using rq_lock instead of rq_lock_irqsave
- Added support to remove BW of DL servers when they are switched to/from EXT
- Link to v1: https://lore.kernel.org/all/[email protected]/
Andrea Righi (5):
sched/deadline: Add support to remove DL server's bandwidth contribution sched/deadline: Account ext server bandwidth sched/deadline: Allow to initialize DL server when needed
sched_ext: Selectively enable ext and fair DL servers selftests/sched_ext: Add test for sched_ext dl_server
Joel Fernandes (9):
sched/debug: Fix updating of ppos on server write ops sched/debug: Stop and start server based on if it was active sched/deadline: Clear the defer params sched/deadline: Return EBUSY if dl_bw_cpus is zero
sched: Add a server arg to dl_server_update_idle_time()
sched_ext: Add a DL server for sched_ext tasks sched/debug: Add support to change sched_ext server params sched/deadline: Fix DL server crash in inactive_timer callback selftests/sched_ext: Add test for DL server total_bw consistency
kernel/sched/core.c | 3 + kernel/sched/deadline.c | 138 ++++++++--- kernel/sched/debug.c | 163 ++++++++++--- kernel/sched/ext.c | 148 +++++++++++- kernel/sched/fair.c | 2 +- kernel/sched/idle.c | 2 +- kernel/sched/sched.h | 7 +- kernel/sched/topology.c | 5 + tools/testing/selftests/sched_ext/Makefile | 2 + tools/testing/selftests/sched_ext/rt_stall.bpf.c | 23 ++ tools/testing/selftests/sched_ext/rt_stall.c | 214 +++++++++++++++++ tools/testing/selftests/sched_ext/total_bw.c | 281 +++++++++++++++++++++++ 12 files changed, 918 insertions(+), 70 deletions(-) create mode 100644 tools/testing/selftests/sched_ext/rt_stall.bpf.c create mode 100644 tools/testing/selftests/sched_ext/rt_stall.c create mode 100644 tools/testing/selftests/sched_ext/total_bw.c
Andrea Righi
reviewerPATCH 1/14 sched/debug: Fix updating of ppos on server write ops
From: Joel Fernandes <[email protected]>
Updating "ppos" on error conditions does not make much sense. The pattern is to return the error code directly without modifying the position, or modify the position on success and return the number of bytes written.
Since on success, the return value of apply is 0, there is no point in modifying ppos either. Fix it by removing all this and just returning error code or number of bytes written on success.
--- kernel/sched/debug.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
kernel/sched/debug.c +4 -3
@@ -345,8 +345,8 @@ static ssize_t sched_fair_server_write(struct file *filp, const char __user *ubulong cpu = (long ) ((struct seq_file *) filp->private_data)->private;struct rq *rq =cpu_rq (cpu);u64 runtime, period; +int retval =0 ;size_t err; -int retval;u64 value; err =kstrtoull_from_user (ubuf, cnt,10 , &value);
@@ -380,8 +380,6 @@ static ssize_t sched_fair_server_write(struct file *filp, const char __user *ubudl_server_stop (&rq->fair_server); retval =dl_server_apply_params (&rq->fair_server, runtime, period,0 ); -if (retval) - cnt = retval;if (!runtime) printk_deferred("Fair server disabled in CPU %d, system may crash due to starvation.\n",
@@ -389,6 +387,9 @@ static ssize_t sched_fair_server_write(struct file *filp, const char __user *ubuif (rq->cfs.h_nr_queued)dl_server_start (&rq->fair_server); + +if (retval <0 ) +return retval; } *ppos += cnt;
Signature · 1 line
2.51.0
Re: [PATCH 01/14] sched/debug: Fix updating of ppos on server write ops
Hi!
Quoted 6 lines · Andrea Righi
From: Joel Fernandes <[email protected]>
Updating "ppos" on error conditions does not make much sense. The pattern is to return the error code directly without modifying the position, or modify the position on success and return the number of bytes written.
Quoted 7 lines
Since on success, the return value of apply is 0, there is no point in modifying ppos either. Fix it by removing all this and just returning error code or number of bytes written on success.
Acked-by Tejun Heo <[email protected]>Reviewed-by Andrea Righi <[email protected]>Signed-off-by Joel Fernandes <[email protected]>
Thanks,
Juri
Andrea Righi
reviewerPATCH 2/14 sched/debug: Stop and start server based on if it was active
In reply to Andrea RighiFrom: Joel Fernandes <[email protected]>
Currently the DL server interface for applying parameters checks CFS-internals to identify if the server is active. This is error-prone and makes it difficult when adding new servers in the future.
Fix it, by using dl_server_active() which is also used by the DL server code to determine if the DL server was started.
--- kernel/sched/debug.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)
kernel/sched/debug.c +8 -3
@@ -354,6 +354,8 @@ static ssize_t sched_fair_server_write(struct file *filp, const char __user *ubureturn err; scoped_guard (rq_lock_irqsave, rq) { +bool is_active; + runtime = rq->fair_server.dl_runtime; period = rq->fair_server.dl_period;
@@ -376,8 +378,11 @@ static ssize_t sched_fair_server_write(struct file *filp, const char __user *ubureturn -EINVAL ; } -update_rq_clock (rq); -dl_server_stop (&rq->fair_server); + is_active =dl_server_active (&rq->fair_server); +if (is_active) { +update_rq_clock (rq); +dl_server_stop (&rq->fair_server); + } retval =dl_server_apply_params (&rq->fair_server, runtime, period,0 );
@@ -385,7 +390,7 @@ static ssize_t sched_fair_server_write(struct file *filp, const char __user *ubuprintk_deferred ("Fair server disabled in CPU %d, system may crash due to starvation.\n" ,cpu_of (rq)); -if (rq->cfs.h_nr_queued) +if (is_active)dl_server_start (&rq->fair_server);if (retval <0 )
Signature · 1 line
2.51.0
Juri Lelli
maintainerRe: [PATCH 02/14] sched/debug: Stop and start server based on if it was active
Hi!
Quoted 30 lines · Andrea Righi
From: Joel Fernandes <[email protected]>
Currently the DL server interface for applying parameters checks CFS-internals to identify if the server is active. This is error-prone and makes it difficult when adding new servers in the future.
Fix it, by using
dl_server_active()which is also used by the DL server code to determine if the DL server was started.Acked-by Tejun Heo <[email protected]>Reviewed-by Andrea Righi <[email protected]>Signed-off-by Joel Fernandes <[email protected]>--- kernel/sched/debug.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)kernel/sched/debug.c +2
-0@@ -354,6 +354,8 @@ static ssize_t sched_fair_server_write(struct file *filp, const char __user *ubureturn err; scoped_guard (rq_lock_irqsave, rq) { +bool is_active; + runtime = rq->fair_server.dl_runtime; period = rq->fair_server.dl_period;
kernel/sched/debug.c static ssize_t sched_fair_server_write(struct file *filp, const char __user *ubu · line 385
Quoted 10 lines
kernel/sched/debug.c +5
-2@@ -376,8 +378,11 @@ static ssize_t sched_fair_server_write(struct file *filp, const char __user *ubureturn -EINVAL ; } -update_rq_clock (rq); -dl_server_stop (&rq->fair_server); + is_active =dl_server_active (&rq->fair_server); +if (is_active) { +update_rq_clock (rq); +dl_server_stop (&rq->fair_server); + }
Won't this reintroduce what bb4700adc3abe ("sched/deadline: Always stop dl-server before changing parameters") fixed?
Thanks,
Juri
Re: [PATCH 02/14] sched/debug: Stop and start server based on if it was active
Quoted 36 lines · Juri Lelli
Hi!
Quoted 33 lines · Andrea Righi
From: Joel Fernandes <[email protected]>
Currently the DL server interface for applying parameters checks CFS-internals to identify if the server is active. This is error-prone and makes it difficult when adding new servers in the future.
Fix it, by using
dl_server_active()which is also used by the DL server code to determine if the DL server was started.Acked-by Tejun Heo <[email protected]>Reviewed-by Andrea Righi <[email protected]>Signed-off-by Joel Fernandes <[email protected]>--- kernel/sched/debug.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)kernel/sched/debug.c +2
-0@@ -354,6 +354,8 @@ static ssize_t sched_fair_server_write(struct file *filp, const char __user *ubureturn err; scoped_guard (rq_lock_irqsave, rq) { +bool is_active; + runtime = rq->fair_server.dl_runtime; period = rq->fair_server.dl_period;@@ -376,8 +378,11 @@ static ssize_t sched_fair_server_write(struct file *filp, const char __user *ubureturn -EINVAL ; }
Quoted 10 lines
kernel/sched/debug.c static ssize_t sched_fair_server_write(struct file *filp, const char __user *ubu · line 385
Quoted 7 lines
kernel/sched/debug.c +5
-2-update_rq_clock (rq); -dl_server_stop (&rq->fair_server); + is_active =dl_server_active (&rq->fair_server); +if (is_active) { +update_rq_clock (rq); +dl_server_stop (&rq->fair_server); + }Won't this reintroduce what
bb4700adc3abe("sched/deadline: Always stop dl-server before changing parameters") fixed?
Ah, OK. It looks like it doesn't, as dl_server_active() is the correct thing to use/check. Also in case the server was not active is should be enqueued in defer mode, so no need to have an updated clock just yet.
Thanks,
Juri
Andrea Righi
reviewerPATCH 3/14 sched/deadline: Clear the defer params
In reply to Andrea RighiFrom: Joel Fernandes <[email protected]>
The defer params were not cleared in __dl_clear_params. Clear them.
Without this is some of my test cases are flaking and the DL timer is not starting correctly AFAICS.
--- kernel/sched/deadline.c | 3 +++ 1 file changed, 3 insertions(+)
kernel/sched/deadline.c +3 -0
@@ -3387,6 +3387,9 @@ static void __dl_clear_params(struct sched_dl_entity *dl_se) dl_se->dl_non_contending =0 ; dl_se->dl_overrun =0 ; dl_se->dl_server =0 ; + dl_se->dl_defer =0 ; + dl_se->dl_defer_running =0 ; + dl_se->dl_defer_armed =0 ;#ifdef CONFIG_RT_MUTEXES dl_se->pi_se = dl_se;
Signature · 1 line
2.51.0
Andrea Righi
reviewerPATCH 4/14 sched/deadline: Return EBUSY if dl_bw_cpus is zero
In reply to Andrea RighiFrom: Joel Fernandes <[email protected]>
Hotplugged CPUs coming online do an enqueue but are not a part of any root domain containing cpu_active() CPUs. So in this case, don't mess with accounting and we can retry later. Without this patch, we see crashes with sched_ext selftest's hotplug test due to divide by zero.
--- kernel/sched/deadline.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
kernel/sched/deadline.c +6 -1
@@ -1665,7 +1665,12 @@ int dl_server_apply_params(struct sched_dl_entity *dl_se, u64 runtime, u64 perio cpus =dl_bw_cpus (cpu); cap =dl_bw_capacity (cpu); -if (__dl_overflow (dl_b, cap, old_bw, new_bw)) +/* +* Hotplugged CPUs coming online do an enqueue but are not a part of any +* root domain containing cpu_active() CPUs. So in this case, don't mess +* with accounting and we can retry later. +*/ +if (!cpus ||__dl_overflow (dl_b, cap, old_bw, new_bw))return -EBUSY ;if (init) {
Signature · 1 line
2.51.0
Juri Lelli
maintainerRe: [PATCH 04/14] sched/deadline: Return EBUSY if dl_bw_cpus is zero
Hi!
Quoted 17 lines · Andrea Righi
From: Joel Fernandes <[email protected]>
Hotplugged CPUs coming online do an enqueue but are not a part of any root domain containing
cpu_active()CPUs. So in this case, don't mess with accounting and we can retry later. Without this patch, we see crashes withsched_extselftest's hotplug test due to divide by zero.Signed-off-by Joel Fernandes <[email protected]>--- kernel/sched/deadline.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)kernel/sched/deadline.c +0
-0
kernel/sched/deadline.c int dl_server_apply_params(struct sched_dl_entity *dl_se, u64 runtime, u64 perio · line 1671
Quoted 8 lines
kernel/sched/deadline.c +4
-1@@ -1665,7 +1665,12 @@ int dl_server_apply_params(struct sched_dl_entity *dl_se, u64 runtime, u64 perio cpus =dl_bw_cpus (cpu); cap =dl_bw_capacity (cpu); -if (__dl_overflow (dl_b, cap, old_bw, new_bw)) +/* +* Hotplugged CPUs coming online do an enqueue but are not a part of any +* root domain containing cpu_active() CPUs. So in this case, don't mess +* with accounting and we can retry later.
Later when? It seems a little vague. :)
Thanks,
Juri
Andrea Righi
reviewerRe: [PATCH 04/14] sched/deadline: Return EBUSY if dl_bw_cpus is zero
Quoted 23 lines · Juri Lelli
Hi!
Quoted 20 lines · Andrea Righi
From: Joel Fernandes <[email protected]>
Hotplugged CPUs coming online do an enqueue but are not a part of any root domain containing
cpu_active()CPUs. So in this case, don't mess with accounting and we can retry later. Without this patch, we see crashes withsched_extselftest's hotplug test due to divide by zero.Signed-off-by Joel Fernandes <[email protected]>--- kernel/sched/deadline.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)kernel/sched/deadline.c +0
-0@@ -1665,7 +1665,12 @@ int dl_server_apply_params(struct sched_dl_entity *dl_se, u64 runtime, u64 perio cpus =dl_bw_cpus (cpu); cap =dl_bw_capacity (cpu);
Quoted 7 lines
kernel/sched/deadline.c int dl_server_apply_params(struct sched_dl_entity *dl_se, u64 runtime, u64 perio · line 1671
Quoted 5 lines
kernel/sched/deadline.c +4
-1-if (__dl_overflow (dl_b, cap, old_bw, new_bw)) +/* +* Hotplugged CPUs coming online do an enqueue but are not a part of any +* root domain containing cpu_active() CPUs. So in this case, don't mess +* with accounting and we can retry later. Later when? It seems a little vague. :)
Yeah, this comment is actually incorrect, we're not "retrying later" anymore (we used to do that in a previous version), now the params are applied via:
ext.c:handle_hotplug() -> dl_server_on() -> dl_server_apply_params()
Or via scx_enable() when an scx scheduler is loaded. So, I'm wondering if this condition is still needed. Will do some tests.
Thanks!
-Andrea
Andrea Righi
reviewerRe: [PATCH 04/14] sched/deadline: Return EBUSY if dl_bw_cpus is zero
Quoted 32 lines · Andrea Righi
Quoted 23 lines · Juri Lelli
Hi!
Quoted 20 lines · Andrea Righi
From: Joel Fernandes <[email protected]>
Hotplugged CPUs coming online do an enqueue but are not a part of any root domain containing
cpu_active()CPUs. So in this case, don't mess with accounting and we can retry later. Without this patch, we see crashes withsched_extselftest's hotplug test due to divide by zero.Signed-off-by Joel Fernandes <[email protected]>--- kernel/sched/deadline.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)kernel/sched/deadline.c +0
-0@@ -1665,7 +1665,12 @@ int dl_server_apply_params(struct sched_dl_entity *dl_se, u64 runtime, u64 perio cpus =dl_bw_cpus (cpu); cap =dl_bw_capacity (cpu);Quoted 7 lines
kernel/sched/deadline.c int dl_server_apply_params(struct sched_dl_entity *dl_se, u64 runtime, u64 perio · line 1671
Quoted 5 lines
kernel/sched/deadline.c +4
-1-if (__dl_overflow (dl_b, cap, old_bw, new_bw)) +/* +* Hotplugged CPUs coming online do an enqueue but are not a part of any +* root domain containing cpu_active() CPUs. So in this case, don't mess +* with accounting and we can retry later. Later when? It seems a little vague. :)
Quoted 8 lines
Yeah, this comment is actually incorrect, we're not "retrying later" anymore (we used to do that in a previous version), now the params are applied via:
ext.c:handle_hotplug() ->
dl_server_on()->dl_server_apply_params()Or via
scx_enable()when an scx scheduler is loaded. So, I'm wondering if this condition is still needed. Will do some tests.
Looks like I can't reproduce the error with the hotplug kselftest anymore (and it was happening pretty quickly).
Then I guess we can drop this patch or maybe add a WARN_ON_ONCE(!cpus) just to safe?
Thanks,
-Andrea
Juri Lelli
maintainerRe: [PATCH 04/14] sched/deadline: Return EBUSY if dl_bw_cpus is zero
Quoted 39 lines · Andrea Righi
Quoted 38 lines · Andrea Righi
Quoted 23 lines · Juri Lelli
Hi!
Quoted 20 lines · Andrea Righi
From: Joel Fernandes <[email protected]>
Hotplugged CPUs coming online do an enqueue but are not a part of any root domain containing
cpu_active()CPUs. So in this case, don't mess with accounting and we can retry later. Without this patch, we see crashes withsched_extselftest's hotplug test due to divide by zero.Signed-off-by Joel Fernandes <[email protected]>--- kernel/sched/deadline.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)kernel/sched/deadline.c +0
-0@@ -1665,7 +1665,12 @@ int dl_server_apply_params(struct sched_dl_entity *dl_se, u64 runtime, u64 perio cpus =dl_bw_cpus (cpu); cap =dl_bw_capacity (cpu);Quoted 7 lines
kernel/sched/deadline.c int dl_server_apply_params(struct sched_dl_entity *dl_se, u64 runtime, u64 perio · line 1671
Quoted 5 lines
kernel/sched/deadline.c +4
-1-if (__dl_overflow (dl_b, cap, old_bw, new_bw)) +/* +* Hotplugged CPUs coming online do an enqueue but are not a part of any +* root domain containing cpu_active() CPUs. So in this case, don't mess +* with accounting and we can retry later. Later when? It seems a little vague. :)
Yeah, this comment is actually incorrect, we're not "retrying later" anymore (we used to do that in a previous version), now the params are applied via:
ext.c:handle_hotplug() ->
dl_server_on()->dl_server_apply_params()
Quoted 8 lines
Quoted 2 lines
Or via
scx_enable()when an scx scheduler is loaded. So, I'm wondering if this condition is still needed. Will do some tests.Looks like I can't reproduce the error with the hotplug kselftest anymore (and it was happening pretty quickly).
Then I guess we can drop this patch or maybe add a
WARN_ON_ONCE(!cpus)just to safe?
WARN_ON_ONCE() works for me.
Thanks!
Andrea Righi
reviewerPATCH 5/14 sched: Add a server arg to dl_server_update_idle_time()
In reply to Andrea RighiFrom: Joel Fernandes <[email protected]>
Since we are adding more servers, make dl_server_update_idle_time() accept a server argument than a specific server.
--- kernel/sched/deadline.c | 15 ++++++++------- kernel/sched/fair.c | 2 +- kernel/sched/idle.c | 2 +- kernel/sched/sched.h | 3 ++- 4 files changed, 12 insertions(+), 10 deletions(-)
kernel/sched/deadline.c +8 -7
@@ -1543,26 +1543,27 @@ static void update_curr_dl_se(struct rq *rq, struct sched_dl_entity *dl_se, s64* as time available for the fair server, avoiding a penalty for the * rt scheduler that did not consumed that time. */ -void dl_server_update_idle_time (struct rq *rq,struct task_struct *p) +void dl_server_update_idle_time (struct rq *rq,struct task_struct *p, +struct sched_dl_entity *rq_dl_server) {s64 delta_exec; -if (!rq->fair_server.dl_defer) +if (!rq_dl_server->dl_defer)return ;/* no need to discount more */ -if (rq->fair_server.runtime <0 ) +if (rq_dl_server->runtime <0 )return ; delta_exec =rq_clock_task (rq) - p->se.exec_start;if (delta_exec <0 )return ; - rq->fair_server.runtime -= delta_exec; + rq_dl_server->runtime -= delta_exec; -if (rq->fair_server.runtime <0 ) { - rq->fair_server.dl_defer_running =0 ; - rq->fair_server.runtime =0 ; +if (rq_dl_server->runtime <0 ) { + rq_dl_server->dl_defer_running =0 ; + rq_dl_server->runtime =0 ; } p->se.exec_start =rq_clock_task (rq);
kernel/sched/fair.c +1 -1
@@ -6999,7 +6999,7 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)if (!rq_h_nr_queued && rq->cfs.h_nr_queued) {/* Account for idle runtime */ if (!rq->nr_running) -dl_server_update_idle_time (rq, rq->curr); +dl_server_update_idle_time (rq, rq->curr, &rq->fair_server);dl_server_start (&rq->fair_server); }
kernel/sched/idle.c +1 -1
@@ -454,7 +454,7 @@ 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,struct task_struct *next) { -dl_server_update_idle_time (rq, prev); +dl_server_update_idle_time (rq, prev, &rq->fair_server);scx_update_idle (rq, false, true); }
kernel/sched/sched.h +2 -1
@@ -412,7 +412,8 @@ extern void dl_server_init(struct sched_dl_entity *dl_se, struct rq *rq,extern void sched_init_dl_servers (void );extern void dl_server_update_idle_time (struct rq *rq, -struct task_struct *p); +struct task_struct *p, +struct sched_dl_entity *rq_dl_server);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,
Signature · 1 line
2.51.0
Re: [PATCH 05/14] sched: Add a server arg to dl_server_update_idle_time()
Hi!
Quoted 4 lines · Andrea Righi
From: Joel Fernandes <[email protected]>
Since we are adding more servers, make
dl_server_update_idle_time()accept a server argument than a specific server.
Nit, ^ rather?
Quoted 2 lines
Reviewed-by Andrea Righi <[email protected]>Signed-off-by Joel Fernandes <[email protected]>
Thanks,
Juri
Peter Zijlstra
maintainerRe: [PATCH 05/14] sched: Add a server arg to dl_server_update_idle_time()
In reply to Andrea RighiQuoted 7 lines · Andrea Righi
From: Joel Fernandes <[email protected]>
Since we are adding more servers, make
dl_server_update_idle_time()accept a server argument than a specific server.Reviewed-by Andrea Righi <[email protected]>Signed-off-by Joel Fernandes <[email protected]>
Can you run s/rq_dl_server/dl_server/g on the thing please?
Andrea Righi
reviewerPATCH 6/14 sched_ext: Add a DL server for sched_ext tasks
In reply to Andrea RighiFrom: Joel Fernandes <[email protected]>
sched_ext currently suffers starvation due to RT. The same workload when converted to EXT can get zero runtime if RT is 100% running, causing EXT processes to stall. Fix it by adding a DL server for EXT.
A kselftest is also provided later to verify:
./runner -t rt_stall
===== START =====
TEST: rt_stall
DESCRIPTION: Verify that RT tasks cannot stall SCHED_EXT tasks OUTPUT:
TAP version 13
1..1
ok 1 PASS: CFS task got more than 4.00% of runtime
[ arighi: drop ->balance() now that pick_task() has an rf argument ]
--- kernel/sched/core.c | 3 +++ kernel/sched/deadline.c | 2 +- kernel/sched/ext.c | 51 +++++++++++++++++++++++++++++++++++++++-- kernel/sched/sched.h | 2 ++ 4 files changed, 55 insertions(+), 3 deletions(-)
kernel/sched/core.c +3 -0
@@ -8679,6 +8679,9 @@ void __init sched_init(void)hrtick_rq_init (rq);atomic_set (&rq->nr_iowait,0 );fair_server_init (rq); +#ifdef CONFIG_SCHED_CLASS_EXT +ext_server_init (rq); +#endif #ifdef CONFIG_SCHED_CORE rq->core = rq;
kernel/sched/deadline.c +1 -1
@@ -1504,7 +1504,7 @@ static void update_curr_dl_se(struct rq *rq, struct sched_dl_entity *dl_se, s64* 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) +if (dl_se->dl_server)return ;#ifdef CONFIG_RT_GROUP_SCHED
kernel/sched/ext.c +49 -2
87 lines not on this page · raw message
kernel/sched/sched.h +2 -0
@@ -415,6 +415,7 @@ extern void dl_server_update_idle_time(struct rq *rq,struct task_struct *p, struct sched_dl_entity *rq_dl_server);extern void fair_server_init (struct rq *rq); +extern void ext_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);
@@ -1153,6 +1154,7 @@ struct rq {#endif struct sched_dl_entity fair_server; +struct sched_dl_entity ext_server;#ifdef CONFIG_FAIR_GROUP_SCHED /* list of leaf cfs_rq on this CPU: */
Signature · 1 line
2.51.0
Tejun Heo
reviewerRe: [PATCH 06/14] sched_ext: Add a DL server for sched_ext tasks
Quoted 17 lines · Andrea Righi
From: Joel Fernandes <[email protected]>
sched_extcurrently suffers starvation due to RT. The same workload when converted to EXT can get zero runtime if RT is 100% running, causing EXT processes to stall. Fix it by adding a DL server for EXT.A kselftest is also provided later to verify:
./runner -t
rt_stall
===== START =====
TEST:rt_stall
DESCRIPTION: Verify that RT tasks cannot stallSCHED_EXTtasks OUTPUT:
TAP version 13
1..1
ok 1 PASS: CFS task got more than 4.00% of runtime
Quoted 6 lines
[ arighi: drop ->balance() now that
pick_task()has an rf argument ]Cc Luigi De Matteis <[email protected]>Co-developed-by Andrea Righi <[email protected]>Signed-off-by Andrea Righi <[email protected]>Signed-off-by Joel Fernandes <[email protected]>
I don't see anything preventing this to come after patch 11 so that all sched_ext changes are at the end. Am I correct? That'd make applying the patches easier. All the debug and deadline changes can be applied to sched/core and I can pull that and apply sched_ext changes on top.
Thanks.
Signature · 1 line
tejun
Andrea Righi
reviewerRe: [PATCH 06/14] sched_ext: Add a DL server for sched_ext tasks
Quoted 25 lines · Tejun Heo
Quoted 17 lines · Andrea Righi
From: Joel Fernandes <[email protected]>
sched_extcurrently suffers starvation due to RT. The same workload when converted to EXT can get zero runtime if RT is 100% running, causing EXT processes to stall. Fix it by adding a DL server for EXT.A kselftest is also provided later to verify:
./runner -t
rt_stall
===== START =====
TEST:rt_stall
DESCRIPTION: Verify that RT tasks cannot stallSCHED_EXTtasks OUTPUT:
TAP version 13
1..1
ok 1 PASS: CFS task got more than 4.00% of runtimeQuoted 6 lines
[ arighi: drop ->balance() now that
pick_task()has an rf argument ]Cc Luigi De Matteis <[email protected]>Co-developed-by Andrea Righi <[email protected]>Signed-off-by Andrea Righi <[email protected]>Signed-off-by Joel Fernandes <[email protected]>
Quoted 4 lines
I don't see anything preventing this to come after patch 11 so that all
sched_extchanges are at the end. Am I correct? That'd make applying the patches easier. All the debug and deadline changes can be applied to sched/core and I can pull that and applysched_extchanges on top.
Oh yes, we can definitely move this one. I'll reorder the patches.
Thanks,
-Andrea
Tejun Heo
reviewerRe: [PATCH 06/14] sched_ext: Add a DL server for sched_ext tasks
In reply to Andrea Righikernel/sched/ext.c BTF_ID_FLAGS(func, scx_bpf_now) · line 6910
Quoted 5 lines · Andrea Righi
kernel/sched/ext.c +5
-0+static struct task_struct * +ext_server_pick_task (struct sched_dl_entity *dl_se,struct rq_flags *rf) +{ +return pick_task_scx (dl_se->rq, rf); +}
I wonder whether we should tell pick_task_scx() to suppress the rq_modified_above() test in this case as a fair or RT task being enqueued has no reason to restart the picking process. While it will behave fine on retry, it's probably useful to be explicit here.
Thanks.
Signature · 1 line
tejun
Andrea Righi
reviewerRe: [PATCH 06/14] sched_ext: Add a DL server for sched_ext tasks
Quoted 11 lines · Tejun Heo
kernel/sched/ext.c BTF_ID_FLAGS(func, scx_bpf_now) · line 6910
Quoted 5 lines · Andrea Righi
kernel/sched/ext.c +5
-0+static struct task_struct * +ext_server_pick_task (struct sched_dl_entity *dl_se,struct rq_flags *rf) +{ +return pick_task_scx (dl_se->rq, rf); +}I wonder whether we should tell
pick_task_scx()to suppress therq_modified_above()test in this case as a fair or RT task being enqueued has no reason to restart the picking process. While it will behave fine on retry, it's probably useful to be explicit here.
Yeah, that's a valid point. Maybe we can add a new flag to rq->scx.flags? Something like SCX_RQ_DL_SERVER_PICK?
Thanks,
-Andrea
Tejun Heo
reviewerRe: [PATCH 06/14] sched_ext: Add a DL server for sched_ext tasks
Quoted 13 lines · Andrea Righi
Quoted 11 lines · Tejun Heo
kernel/sched/ext.c BTF_ID_FLAGS(func, scx_bpf_now) · line 6910
Quoted 5 lines · Andrea Righi
kernel/sched/ext.c +5
-0+static struct task_struct * +ext_server_pick_task (struct sched_dl_entity *dl_se,struct rq_flags *rf) +{ +return pick_task_scx (dl_se->rq, rf); +}I wonder whether we should tell
pick_task_scx()to suppress therq_modified_above()test in this case as a fair or RT task being enqueued has no reason to restart the picking process. While it will behave fine on retry, it's probably useful to be explicit here.
Quoted 2 lines
Yeah, that's a valid point. Maybe we can add a new flag to
rq->scx.flags? Something likeSCX_RQ_DL_SERVER_PICK?
We can factor out the internals of pick_task_scx() into a separate function and add a flag there?
Thanks.
Signature · 1 line
tejun
Andrea Righi
reviewerRe: [PATCH 06/14] sched_ext: Add a DL server for sched_ext tasks
Quoted 14 lines · Tejun Heo
Quoted 13 lines · Andrea Righi
Quoted 11 lines · Tejun Heo
kernel/sched/ext.c BTF_ID_FLAGS(func, scx_bpf_now) · line 6910
Quoted 5 lines · Andrea Righi
kernel/sched/ext.c +5
-0+static struct task_struct * +ext_server_pick_task (struct sched_dl_entity *dl_se,struct rq_flags *rf) +{ +return pick_task_scx (dl_se->rq, rf); +}I wonder whether we should tell
pick_task_scx()to suppress therq_modified_above()test in this case as a fair or RT task being enqueued has no reason to restart the picking process. While it will behave fine on retry, it's probably useful to be explicit here.
Quoted 5 lines
Quoted 2 lines
Yeah, that's a valid point. Maybe we can add a new flag to
rq->scx.flags? Something likeSCX_RQ_DL_SERVER_PICK?We can factor out the internals of
pick_task_scx()into a separate function and add a flag there?
Much better, I like that. Ok, I'll incorporate this change and send a new version.
Thanks,
-Andrea
Juri Lelli
maintainerRe: [PATCH 06/14] sched_ext: Add a DL server for sched_ext tasks
In reply to Andrea RighiHi!
Quoted 49 lines · Andrea Righi
From: Joel Fernandes <[email protected]>
sched_extcurrently suffers starvation due to RT. The same workload when converted to EXT can get zero runtime if RT is 100% running, causing EXT processes to stall. Fix it by adding a DL server for EXT.A kselftest is also provided later to verify:
./runner -t
rt_stall
===== START =====
TEST:rt_stall
DESCRIPTION: Verify that RT tasks cannot stallSCHED_EXTtasks OUTPUT:
TAP version 13
1..1
ok 1 PASS: CFS task got more than 4.00% of runtime[ arighi: drop ->balance() now that
pick_task()has an rf argument ]Cc Luigi De Matteis <[email protected]>Co-developed-by Andrea Righi <[email protected]>Signed-off-by Andrea Righi <[email protected]>Signed-off-by Joel Fernandes <[email protected]>--- kernel/sched/core.c | 3 +++ kernel/sched/deadline.c | 2 +- kernel/sched/ext.c | 51 +++++++++++++++++++++++++++++++++++++++-- kernel/sched/sched.h | 2 ++ 4 files changed, 55 insertions(+), 3 deletions(-)kernel/sched/core.c +3
-0@@ -8679,6 +8679,9 @@ void __init sched_init(void)hrtick_rq_init (rq);atomic_set (&rq->nr_iowait,0 );fair_server_init (rq); +#ifdef CONFIG_SCHED_CLASS_EXT +ext_server_init (rq); +#endif #ifdef CONFIG_SCHED_CORE rq->core = rq;kernel/sched/deadline.c +0
-0
kernel/sched/deadline.c static void update_curr_dl_se(struct rq *rq, struct sched_dl_entity *dl_se, s64 · line 1504
Quoted 1 line
kernel/sched/deadline.c +0
-0@@ -1504,7 +1504,7 @@ static void update_curr_dl_se(struct rq *rq, struct sched_dl_entity *dl_se, s64* The fair server (sole dl_server) does not account for real-time
Fair server is not alone anymore. :))
Please update the comment as well.
Quoted 7 lines
kernel/sched/deadline.c +1
-1* workload because it is running fair work. */ -if (dl_se == &rq->fair_server) +if (dl_se->dl_server)return ;#ifdef CONFIG_RT_GROUP_SCHED
...
kernel/sched/deadline.c static bool dequeue_task_scx(struct rq *rq, struct task_struct *p, int deq_flags · line 1506
Quoted 9 lines
kernel/sched/deadline.c +5
-0@@ -1487,6 +1499,11 @@ static bool dequeue_task_scx(struct rq *rq, struct task_struct *p, int deq_flagssub_nr_running (rq,1 );dispatch_dequeue (rq, p); + +/* Stop the server if this was the last task */ +if (rq->scx.nr_running ==0 ) +dl_server_stop (&rq->ext_server); +
Do we want to use the delayed stop behavior for scx-server as we have for fair-server? Wonder if it's a matter of removing this explicit stop and wait for a full period to elapse as we do for fair. It should reduce timer reprogramming overhead for scx as well.
Thanks,
Juri
Andrea Righi
reviewerRe: [PATCH 06/14] sched_ext: Add a DL server for sched_ext tasks
Hi Juri,
Quoted 71 lines · Juri Lelli
Hi!
Quoted 49 lines · Andrea Righi
From: Joel Fernandes <[email protected]>
sched_extcurrently suffers starvation due to RT. The same workload when converted to EXT can get zero runtime if RT is 100% running, causing EXT processes to stall. Fix it by adding a DL server for EXT.A kselftest is also provided later to verify:
./runner -t
rt_stall
===== START =====
TEST:rt_stall
DESCRIPTION: Verify that RT tasks cannot stallSCHED_EXTtasks OUTPUT:
TAP version 13
1..1
ok 1 PASS: CFS task got more than 4.00% of runtime[ arighi: drop ->balance() now that
pick_task()has an rf argument ]Cc Luigi De Matteis <[email protected]>Co-developed-by Andrea Righi <[email protected]>Signed-off-by Andrea Righi <[email protected]>Signed-off-by Joel Fernandes <[email protected]>--- kernel/sched/core.c | 3 +++ kernel/sched/deadline.c | 2 +- kernel/sched/ext.c | 51 +++++++++++++++++++++++++++++++++++++++-- kernel/sched/sched.h | 2 ++ 4 files changed, 55 insertions(+), 3 deletions(-)kernel/sched/core.c +3
-0@@ -8679,6 +8679,9 @@ void __init sched_init(void)hrtick_rq_init (rq);atomic_set (&rq->nr_iowait,0 );fair_server_init (rq); +#ifdef CONFIG_SCHED_CLASS_EXT +ext_server_init (rq); +#endif #ifdef CONFIG_SCHED_CORE rq->core = rq;kernel/sched/deadline.c +0
-0kernel/sched/deadline.c static void update_curr_dl_se(struct rq *rq, struct sched_dl_entity *dl_se, s64 · line 1504
Quoted 1 line
kernel/sched/deadline.c +0
-0@@ -1504,7 +1504,7 @@ static void update_curr_dl_se(struct rq *rq, struct sched_dl_entity *dl_se, s64* The fair server (sole dl_server) does not account for real-time Fair server is not alone anymore. :))
Please update the comment as well.
Quoted 7 lines
kernel/sched/deadline.c +1
-1* workload because it is running fair work. */ -if (dl_se == &rq->fair_server) +if (dl_se->dl_server)return ;#ifdef CONFIG_RT_GROUP_SCHED ...
Quoted 3 lines
kernel/sched/deadline.c +0
-0@@ -1487,6 +1499,11 @@ static bool dequeue_task_scx(struct rq *rq, struct task_struct *p, int deq_flagssub_nr_running (rq,1 );
Quoted 11 lines
kernel/sched/deadline.c static bool dequeue_task_scx(struct rq *rq, struct task_struct *p, int deq_flags · line 1506
Quoted 6 lines
kernel/sched/deadline.c +5
-0dispatch_dequeue (rq, p); + +/* Stop the server if this was the last task */ +if (rq->scx.nr_running ==0 ) +dl_server_stop (&rq->ext_server); +Do we want to use the delayed stop behavior for scx-server as we have for fair-server? Wonder if it's a matter of removing this explicit stop and wait for a full period to elapse as we do for fair. It should reduce timer reprogramming overhead for scx as well.
So, IIUC we could just remove this explicit dl_server_stop() and the server would naturally stop at the end of its current deadline period, if there are still no runnable tasks, right?
In that case it's worth a try.
Thanks,
-Andrea
Juri Lelli
maintainerRe: [PATCH 06/14] sched_ext: Add a DL server for sched_ext tasks
Quoted 6 lines · Andrea Righi
Hi Juri,
Quoted 3 lines · Juri Lelli
Hi!
On 17/10/25 11:25, Andrea Righi wrote:
...
Quoted 15 lines
Quoted 14 lines
kernel/sched/ext.c static bool dequeue_task_scx(struct rq *rq, struct task_struct *p, int deq_flags · line 1506
Quoted 9 lines
kernel/sched/ext.c +5
-0@@ -1487,6 +1499,11 @@ static bool dequeue_task_scx(struct rq *rq, struct task_struct *p, int deq_flagssub_nr_running (rq,1 );dispatch_dequeue (rq, p); + +/* Stop the server if this was the last task */ +if (rq->scx.nr_running ==0 ) +dl_server_stop (&rq->ext_server); +Do we want to use the delayed stop behavior for scx-server as we have for fair-server? Wonder if it's a matter of removing this explicit stop and wait for a full period to elapse as we do for fair. It should reduce timer reprogramming overhead for scx as well.
Quoted 3 lines
So, IIUC we could just remove this explicit
dl_server_stop()and the server would naturally stop at the end of its current deadline period, if there are still no runnable tasks, right?
Right, that is what I'd expect. But this part tricked me several times already, so I am not 100% certain (Peter please keep me honest :).
Quoted 1 line
In that case it's worth a try.
Andrea Righi
reviewerPATCH 7/14 sched/debug: Add support to change sched_ext server params
In reply to Andrea RighiFrom: Joel Fernandes <[email protected]>
When a sched_ext server is loaded, tasks in CFS are converted to run in sched_ext class. Add support to modify the ext server parameters similar to how the fair server parameters are modified.
Re-use common code between ext and fair servers as needed.
[ arighi: Use dl_se->dl_server to determine if dl_se is a DL server, as
suggested by PeterZ. ]
--- kernel/sched/debug.c | 149 ++++++++++++++++++++++++++++++++++++------- 1 file changed, 125 insertions(+), 24 deletions(-)
kernel/sched/debug.c +125 -24
@@ -336,14 +336,16 @@ enum dl_param { DL_PERIOD, }; -static unsigned long fair_server_period_max = (1UL <<22 ) *NSEC_PER_USEC ;/* ~4 seconds */ -static unsigned long fair_server_period_min = (100 ) *NSEC_PER_USEC ;/* 100 us */ +static unsigned long dl_server_period_max = (1UL <<22 ) *NSEC_PER_USEC ;/* ~4 seconds */ +static unsigned long dl_server_period_min = (100 ) *NSEC_PER_USEC ;/* 100 us */ -static ssize_t sched_fair_server_write (struct file *filp,const char __user *ubuf, -size_t cnt,loff_t *ppos,enum dl_param param) +static ssize_t sched_server_write_common (struct file *filp,const char __user *ubuf, +size_t cnt,loff_t *ppos,enum dl_param param, +void *server) {long cpu = (long ) ((struct seq_file *) filp->private_data)->private;struct rq *rq =cpu_rq (cpu); +struct sched_dl_entity *dl_se = (struct sched_dl_entity *)server;u64 runtime, period;int retval =0 ;size_t err;
@@ -356,8 +358,8 @@ static ssize_t sched_fair_server_write(struct file *filp, const char __user *ubu scoped_guard (rq_lock_irqsave, rq) {bool is_active; - runtime = rq->fair_server.dl_runtime; - period = rq->fair_server.dl_period; + runtime = dl_se->dl_runtime; + period = dl_se->dl_period;switch (param) {case DL_RUNTIME :
@@ -373,25 +375,25 @@ static ssize_t sched_fair_server_write(struct file *filp, const char __user *ubu }if (runtime > period || - period > fair_server_period_max || - period < fair_server_period_min) { + period > dl_server_period_max || + period < dl_server_period_min) {return -EINVAL ; } - is_active =dl_server_active (&rq->fair_server); + is_active =dl_server_active (dl_se);if (is_active) {update_rq_clock (rq); -dl_server_stop (&rq->fair_server); +dl_server_stop (dl_se); } - retval =dl_server_apply_params (&rq->fair_server, runtime, period,0 ); + retval =dl_server_apply_params (dl_se, runtime, period,0 );if (!runtime) -printk_deferred ("Fair server disabled in CPU %d, system may crash due to starvation.\n" , -cpu_of (rq)); +printk_deferred ("%s server disabled on CPU %d, system may crash due to starvation.\n" , + server == &rq->fair_server ?"Fair" :"Ext" ,cpu_of (rq));if (is_active) -dl_server_start (&rq->fair_server); +dl_server_start (dl_se);if (retval <0 )return retval;
@@ -401,36 +403,42 @@ static ssize_t sched_fair_server_write(struct file *filp, const char __user *ubureturn cnt; } -static size_t sched_fair_server_show (struct seq_file *m,void *v,enum dl_param param) +static size_t sched_server_show_common (struct seq_file *m,void *v,enum dl_param param, +void *server) { -unsigned long cpu = (unsigned long ) m->private; -struct rq *rq =cpu_rq (cpu); +struct sched_dl_entity *dl_se = (struct sched_dl_entity *)server;u64 value;switch (param) {case DL_RUNTIME : - value = rq->fair_server.dl_runtime; + value = dl_se->dl_runtime;break ;case DL_PERIOD : - value = rq->fair_server.dl_period; + value = dl_se->dl_period;break ; }seq_printf (m,"%llu\n" , value);return 0 ; - }static ssize_t sched_fair_server_runtime_write (struct file *filp,const char __user *ubuf,size_t cnt,loff_t *ppos) { -return sched_fair_server_write (filp, ubuf, cnt, ppos,DL_RUNTIME ); +long cpu = (long ) ((struct seq_file *) filp->private_data)->private; +struct rq *rq =cpu_rq (cpu); + +return sched_server_write_common (filp, ubuf, cnt, ppos,DL_RUNTIME , + &rq->fair_server); }static int sched_fair_server_runtime_show (struct seq_file *m,void *v) { -return sched_fair_server_show (m, v,DL_RUNTIME ); +unsigned long cpu = (unsigned long ) m->private; +struct rq *rq =cpu_rq (cpu); + +return sched_server_show_common (m, v,DL_RUNTIME , &rq->fair_server); }static int sched_fair_server_runtime_open (struct inode *inode,struct file *filp)
@@ -446,16 +454,55 @@ static const struct file_operations fair_server_runtime_fops = { .release = single_release, }; +static ssize_t +sched_ext_server_runtime_write (struct file *filp,const char __user *ubuf, +size_t cnt,loff_t *ppos) +{ +long cpu = (long ) ((struct seq_file *) filp->private_data)->private; +struct rq *rq =cpu_rq (cpu); + +return sched_server_write_common (filp, ubuf, cnt, ppos,DL_RUNTIME , + &rq->ext_server); +} + +static int sched_ext_server_runtime_show (struct seq_file *m,void *v) +{ +unsigned long cpu = (unsigned long ) m->private; +struct rq *rq =cpu_rq (cpu); + +return sched_server_show_common (m, v,DL_RUNTIME , &rq->ext_server); +} + +static int sched_ext_server_runtime_open (struct inode *inode,struct file *filp) +{ +return single_open (filp, sched_ext_server_runtime_show, inode->i_private); +} + +static const struct file_operations ext_server_runtime_fops = { + .open = sched_ext_server_runtime_open, + .write = sched_ext_server_runtime_write, + .read = seq_read, + .llseek = seq_lseek, + .release = single_release, +}; +static ssize_t sched_fair_server_period_write (struct file *filp,const char __user *ubuf,size_t cnt,loff_t *ppos) { -return sched_fair_server_write (filp, ubuf, cnt, ppos,DL_PERIOD ); +long cpu = (long ) ((struct seq_file *) filp->private_data)->private; +struct rq *rq =cpu_rq (cpu); + +return sched_server_write_common (filp, ubuf, cnt, ppos,DL_PERIOD , + &rq->fair_server); }static int sched_fair_server_period_show (struct seq_file *m,void *v) { -return sched_fair_server_show (m, v,DL_PERIOD ); +unsigned long cpu = (unsigned long ) m->private; +struct rq *rq =cpu_rq (cpu); + +return sched_server_show_common (m, v,DL_PERIOD , &rq->fair_server); }static int sched_fair_server_period_open (struct inode *inode,struct file *filp)
@@ -471,6 +518,38 @@ static const struct file_operations fair_server_period_fops = { .release = single_release, }; +static ssize_t +sched_ext_server_period_write (struct file *filp,const char __user *ubuf, +size_t cnt,loff_t *ppos) +{ +long cpu = (long ) ((struct seq_file *) filp->private_data)->private; +struct rq *rq =cpu_rq (cpu); + +return sched_server_write_common (filp, ubuf, cnt, ppos,DL_PERIOD , + &rq->ext_server); +} + +static int sched_ext_server_period_show (struct seq_file *m,void *v) +{ +unsigned long cpu = (unsigned long ) m->private; +struct rq *rq =cpu_rq (cpu); + +return sched_server_show_common (m, v,DL_PERIOD , &rq->ext_server); +} + +static int sched_ext_server_period_open (struct inode *inode,struct file *filp) +{ +return single_open (filp, sched_ext_server_period_show, inode->i_private); +} + +static const struct file_operations ext_server_period_fops = { + .open = sched_ext_server_period_open, + .write = sched_ext_server_period_write, + .read = seq_read, + .llseek = seq_lseek, + .release = single_release, +}; +static struct dentry *debugfs_sched;static void debugfs_fair_server_init (void )
@@ -494,6 +573,27 @@ static void debugfs_fair_server_init(void) } } +static void debugfs_ext_server_init (void ) +{ +struct dentry *d_ext; +unsigned long cpu; + + d_ext =debugfs_create_dir ("ext_server" , debugfs_sched); +if (!d_ext) +return ; + +for_each_possible_cpu (cpu) { +struct dentry *d_cpu; +char buf[32 ]; + +snprintf (buf,sizeof (buf),"cpu%lu" , cpu); + d_cpu =debugfs_create_dir (buf, d_ext); + +debugfs_create_file ("runtime" ,0644 , d_cpu, (void *) cpu, &ext_server_runtime_fops); +debugfs_create_file ("period" ,0644 , d_cpu, (void *) cpu, &ext_server_period_fops); + } +} +static __init int sched_init_debug (void ) {struct dentry __maybe_unused *numa;
@@ -532,6 +632,7 @@ static __init int sched_init_debug(void)debugfs_create_file ("debug" ,0444 , debugfs_sched,NULL , &sched_debug_fops);debugfs_fair_server_init (); +debugfs_ext_server_init ();return 0 ; }
Signature · 1 line
2.51.0
Re: [PATCH 07/14] sched/debug: Add support to change sched_ext server params
Hi!
Quoted 15 lines · Andrea Righi
From: Joel Fernandes <[email protected]>
When a
sched_extserver is loaded, tasks in CFS are converted to run insched_extclass. Add support to modify the ext server parameters similar to how the fair server parameters are modified.Re-use common code between ext and fair servers as needed.
[ arighi: Use
dl_se->dl_serverto determine ifdl_seis a DL server, as
suggested by PeterZ. ]Co-developed-by Andrea Righi <[email protected]>Signed-off-by Andrea Righi <[email protected]>Signed-off-by Joel Fernandes <[email protected]>---
...
Quoted 17 lines
kernel/sched/debug.c +4
-4@@ -373,25 +375,25 @@ static ssize_t sched_fair_server_write(struct file *filp, const char __user *ubu }if (runtime > period || - period > fair_server_period_max || - period < fair_server_period_min) { + period > dl_server_period_max || + period < dl_server_period_min) {return -EINVAL ; } - is_active =dl_server_active (&rq->fair_server); + is_active =dl_server_active (dl_se);if (is_active) {update_rq_clock (rq); -dl_server_stop (&rq->fair_server); +dl_server_stop (dl_se);
kernel/sched/debug.c static ssize_t sched_fair_server_write(struct file *filp, const char __user *ubu · line 393
Quoted 10 lines
kernel/sched/debug.c +3
-3} - retval =dl_server_apply_params (&rq->fair_server, runtime, period,0 ); + retval =dl_server_apply_params (dl_se, runtime, period,0 );if (!runtime) -printk_deferred ("Fair server disabled in CPU %d, system may crash due to starvation.\n" , -cpu_of (rq)); +printk_deferred ("%s server disabled on CPU %d, system may crash due to starvation.\n" , + server == &rq->fair_server ?"Fair" :"Ext" ,cpu_of (rq));
Guess this might get convoluted if are ever going to add an additional dl-server, but I fail to see that happening atm (to service what?).
Thanks,
Juri
Andrea Righi
reviewerRe: [PATCH 07/14] sched/debug: Add support to change sched_ext server params
Quoted 43 lines · Juri Lelli
Hi!
Quoted 15 lines · Andrea Righi
From: Joel Fernandes <[email protected]>
When a
sched_extserver is loaded, tasks in CFS are converted to run insched_extclass. Add support to modify the ext server parameters similar to how the fair server parameters are modified.Re-use common code between ext and fair servers as needed.
[ arighi: Use
dl_se->dl_serverto determine ifdl_seis a DL server, as
suggested by PeterZ. ]Co-developed-by Andrea Righi <[email protected]>Signed-off-by Andrea Righi <[email protected]>Signed-off-by Joel Fernandes <[email protected]>---
...
Quoted 22 lines
kernel/sched/debug.c +5
-5@@ -373,25 +375,25 @@ static ssize_t sched_fair_server_write(struct file *filp, const char __user *ubu }if (runtime > period || - period > fair_server_period_max || - period < fair_server_period_min) { + period > dl_server_period_max || + period < dl_server_period_min) {return -EINVAL ; } - is_active =dl_server_active (&rq->fair_server); + is_active =dl_server_active (dl_se);if (is_active) {update_rq_clock (rq); -dl_server_stop (&rq->fair_server); +dl_server_stop (dl_se); } - retval =dl_server_apply_params (&rq->fair_server, runtime, period,0 ); + retval =dl_server_apply_params (dl_se, runtime, period,0 );
Quoted 8 lines
kernel/sched/debug.c static ssize_t sched_fair_server_write(struct file *filp, const char __user *ubu · line 393
Quoted 5 lines
kernel/sched/debug.c +2
-2if (!runtime) -printk_deferred ("Fair server disabled in CPU %d, system may crash due to starvation.\n" , -cpu_of (rq)); +printk_deferred ("%s server disabled on CPU %d, system may crash due to starvation.\n" , + server == &rq->fair_server ?"Fair" :"Ext" ,cpu_of (rq));Guess this might get convoluted if are ever going to add an additional dl-server, but I fail to see that happening atm (to service what?).
We could add a ->server_class() method that returns the name or something similar, but it's probably a bit overkill, since we have just two dl servers at the moment (and I don't see any use case to have more...).
Thanks,
-Andrea
Quoted 6 lines
Reviewed-by Juri Lelli <[email protected]>Thanks,
Juri
Andrea Righi
reviewerPATCH 8/14 sched/deadline: Add support to remove DL server's bandwidth contribution
In reply to Andrea RighiDuring switching from sched_ext to FAIR tasks and vice-versa, we need support for removing the bandwidth contribution of either DL server. Add support for the same.
--- kernel/sched/deadline.c | 31 +++++++++++++++++++++++++++++++ kernel/sched/sched.h | 1 + 2 files changed, 32 insertions(+)
kernel/sched/deadline.c +31 -0
@@ -1684,6 +1684,12 @@ int dl_server_apply_params(struct sched_dl_entity *dl_se, u64 runtime, u64 periodl_rq_change_utilization (rq, dl_se, new_bw); } +/* Clear these so that the dl_server is reinitialized */ +if (new_bw ==0 ) { + dl_se->dl_defer =0 ; + dl_se->dl_server =0 ; + } + dl_se->dl_runtime = runtime; dl_se->dl_deadline = period; dl_se->dl_period = period;
@@ -1697,6 +1703,31 @@ int dl_server_apply_params(struct sched_dl_entity *dl_se, u64 runtime, u64 perioreturn retval; } +/** +* dl_server_remove_params - Remove bandwidth reservation for a DL server +* @dl_se: The DL server entity to remove bandwidth for +* +* This function removes the bandwidth reservation for a DL server entity, +* cleaning up all bandwidth accounting and server state. +* +* Returns: 0 on success, negative error code on failure +*/ +int dl_server_remove_params (struct sched_dl_entity *dl_se) +{ +if (!dl_se->dl_server) +return 0 ;/* Already disabled */ + +/* +* First dequeue if still queued. It should not be queued since +* we call this only after the last dl_server_stop(). +*/ +if (WARN_ON_ONCE (on_dl_rq (dl_se))) +dequeue_dl_entity (dl_se,DEQUEUE_SLEEP ); + +/* Remove bandwidth reservation */ +return dl_server_apply_params (dl_se,0 , dl_se->dl_period, false); +} +/* * 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/sched.h +1 -0
@@ -419,6 +419,7 @@ extern void ext_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); +extern int dl_server_remove_params (struct sched_dl_entity *dl_se);static inline bool dl_server_active (struct sched_dl_entity *dl_se) {
Signature · 1 line
2.51.0
Juri Lelli
maintainerRe: [PATCH 08/14] sched/deadline: Add support to remove DL server's bandwidth contribution
Hi!
Quoted 47 lines · Andrea Righi
During switching from
sched_extto FAIR tasks and vice-versa, we need support for removing the bandwidth contribution of either DL server. Add support for the same.Co-developed-by Joel Fernandes <[email protected]>Signed-off-by Joel Fernandes <[email protected]>Signed-off-by Andrea Righi <[email protected]>--- kernel/sched/deadline.c | 31 +++++++++++++++++++++++++++++++ kernel/sched/sched.h | 1 + 2 files changed, 32 insertions(+)kernel/sched/deadline.c +20
-0@@ -1684,6 +1684,12 @@ int dl_server_apply_params(struct sched_dl_entity *dl_se, u64 runtime, u64 periodl_rq_change_utilization (rq, dl_se, new_bw); } +/* Clear these so that the dl_server is reinitialized */ +if (new_bw ==0 ) { + dl_se->dl_defer =0 ; + dl_se->dl_server =0 ; + } + dl_se->dl_runtime = runtime; dl_se->dl_deadline = period; dl_se->dl_period = period;@@ -1697,6 +1703,31 @@ int dl_server_apply_params(struct sched_dl_entity *dl_se, u64 runtime, u64 perioreturn retval; } +/** +* dl_server_remove_params - Remove bandwidth reservation for a DL server +* @dl_se: The DL server entity to remove bandwidth for +* +* This function removes the bandwidth reservation for a DL server entity, +* cleaning up all bandwidth accounting and server state. +* +* Returns: 0 on success, negative error code on failure +*/ +int dl_server_remove_params (struct sched_dl_entity *dl_se) +{ +if (!dl_se->dl_server) +return 0 ;/* Already disabled */ +
kernel/sched/deadline.c int dl_server_apply_params(struct sched_dl_entity *dl_se, u64 runtime, u64 perio · line 1729
Quoted 10 lines
kernel/sched/deadline.c +10
-0+/* +* First dequeue if still queued. It should not be queued since +* we call this only after the last dl_server_stop(). +*/ +if (WARN_ON_ONCE (on_dl_rq (dl_se))) +dequeue_dl_entity (dl_se,DEQUEUE_SLEEP ); + +/* Remove bandwidth reservation */ +return dl_server_apply_params (dl_se,0 , dl_se->dl_period, false); +}
I am not sure this is correct wrt inactive_task_timer and task_non_contending. I fear that removing bw immediately might break other deadline entities guarantees (especially if one then maliciously add/removes dl-servers quickly). I kind of think (but again not sure, please Peter and other keep me honest :) we should be waiting for inactive_task_timer to fire (if stopping before 0-lag) and let it clean things up at that point (like we do for simple tasks).
You seem to have additional fixes later on in the series that might be caused by what I describe above.
Thinking more about this I actually wonder if we need this (well it's coming up with later patches) mechanism for automatically removing servers based on fair vs. scx state (full/partial). If we are going to manage dl-servers bw explicitly and separately [1], maybe we can just leave the burden to the user (or middleware) of doing that via the configuration interface?
Thanks,
Juri
Andrea Righi
reviewerPATCH 9/14 sched/deadline: Account ext server bandwidth
In reply to Andrea RighiAlways account for both the ext_server and fair_server bandwidths, especially during CPU hotplug operations. Ignoring either can lead to imbalances in total_bw when sched_ext schedulers are active and CPUs are brought online / offline.
--- kernel/sched/deadline.c | 29 +++++++++++++++++++++-------- kernel/sched/topology.c | 5 +++++ 2 files changed, 26 insertions(+), 8 deletions(-)
kernel/sched/deadline.c +21 -8
@@ -2984,9 +2984,17 @@ void dl_clear_root_domain(struct root_domain *rd)* them, we need to account for them here explicitly. */ for_each_cpu (i, rd->span) { -struct sched_dl_entity *dl_se = &cpu_rq (i)->fair_server; +struct sched_dl_entity *dl_se; -if (dl_server (dl_se) &&cpu_active (i)) +if (!cpu_active (i)) +continue ; + + dl_se = &cpu_rq (i)->fair_server; +if (dl_server (dl_se)) +__dl_add (&rd->dl_bw, dl_se->dl_bw,dl_bw_cpus (i)); + + dl_se = &cpu_rq (i)->ext_server; +if (dl_server (dl_se))__dl_add (&rd->dl_bw, dl_se->dl_bw,dl_bw_cpus (i)); } }
@@ -3485,6 +3493,7 @@ static int dl_bw_manage(enum dl_bw_request req, int cpu, u64 dl_bw)struct dl_bw *dl_b;bool overflow =0 ;u64 fair_server_bw =0 ; +u64 ext_server_bw =0 ;rcu_read_lock_sched (); dl_b =dl_bw_of (cpu);
@@ -3517,27 +3526,31 @@ static int dl_bw_manage(enum dl_bw_request req, int cpu, u64 dl_bw) cap -=arch_scale_cpu_capacity (cpu);/* -* cpu is going offline and NORMAL tasks will be moved away -* from it. We can thus discount dl_server bandwidth -* contribution as it won't need to be servicing tasks after -* the cpu is off. +* cpu is going offline and NORMAL and EXT tasks will be +* moved away from it. We can thus discount dl_server +* bandwidth contribution as it won't need to be servicing +* tasks after the cpu is off. */ if (cpu_rq (cpu)->fair_server.dl_server) fair_server_bw =cpu_rq (cpu)->fair_server.dl_bw; +if (cpu_rq (cpu)->ext_server.dl_server) + ext_server_bw =cpu_rq (cpu)->ext_server.dl_bw; +/* * Not much to check if no DEADLINE bandwidth is present. * dl_servers we can discount, as tasks will be moved out the * offlined CPUs anyway. */ -if (dl_b->total_bw - fair_server_bw >0 ) { +if (dl_b->total_bw - fair_server_bw - ext_server_bw >0 ) {/* * Leaving at least one CPU for DEADLINE tasks seems a * wise thing to do. As said above, cpu is not offline * yet, so account for that. */ if (dl_bw_cpus (cpu) -1 ) - overflow =__dl_overflow (dl_b, cap, fair_server_bw,0 ); + overflow =__dl_overflow (dl_b, cap, + fair_server_bw + ext_server_bw,0 );else overflow =1 ; }
kernel/sched/topology.c +5 -0
@@ -508,6 +508,11 @@ void rq_attach_root(struct rq *rq, struct root_domain *rd)if (rq->fair_server.dl_server)__dl_server_attach_root (&rq->fair_server, rq); +#ifdef CONFIG_SCHED_CLASS_EXT +if (rq->ext_server.dl_server) +__dl_server_attach_root (&rq->ext_server, rq); +#endif +rq_unlock_irqrestore (rq, &rf);if (old_rd)
Signature · 1 line
2.51.0
Andrea Righi
reviewerPATCH 10/14 sched/deadline: Allow to initialize DL server when needed
In reply to Andrea RighiWhen switching between fair and sched_ext, we need to initialize the bandwidth contribution of the DL server independently for each class.
Add support for on-demand initialization to handle such transitions.
--- kernel/sched/deadline.c | 36 +++++++++++++++++++++++++++++------- kernel/sched/sched.h | 1 + 2 files changed, 30 insertions(+), 7 deletions(-)
kernel/sched/deadline.c +29 -7
@@ -1576,6 +1576,32 @@ void dl_server_update(struct sched_dl_entity *dl_se, s64 delta_exec)update_curr_dl_se (dl_se->rq, dl_se, delta_exec); } +/** +* dl_server_init_params - Initialize bandwidth reservation for a DL server +* @dl_se: The DL server entity to remove bandwidth for +* +* This function initializes the bandwidth reservation for a DL server +* entity, its bandwidth accounting and server state. +* +* Returns: 0 on success, negative error code on failure +*/ +int dl_server_init_params (struct sched_dl_entity *dl_se) +{ +u64 runtime =50 *NSEC_PER_MSEC ; +u64 period =1000 *NSEC_PER_MSEC ; +int err; + + err =dl_server_apply_params (dl_se, runtime, period,1 ); +if (err) +return err; + + dl_se->dl_server =1 ; + dl_se->dl_defer =1 ; +setup_new_dl_entity (dl_se); + +return err; +} +void dl_server_start (struct sched_dl_entity *dl_se) {struct rq *rq = dl_se->rq;
@@ -1615,8 +1641,7 @@ void sched_init_dl_servers(void)struct sched_dl_entity *dl_se;for_each_online_cpu (cpu) { -u64 runtime =50 *NSEC_PER_MSEC ; -u64 period =1000 *NSEC_PER_MSEC ; +int err; rq =cpu_rq (cpu);
@@ -1626,11 +1651,8 @@ void sched_init_dl_servers(void)WARN_ON (dl_server (dl_se)); -dl_server_apply_params (dl_se, runtime, period,1 ); - - dl_se->dl_server =1 ; - dl_se->dl_defer =1 ; -setup_new_dl_entity (dl_se); + err =dl_server_init_params (dl_se); +WARN_ON_ONCE (err); } }
kernel/sched/sched.h +1 -0
@@ -419,6 +419,7 @@ extern void ext_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); +extern int dl_server_init_params (struct sched_dl_entity *dl_se);extern int dl_server_remove_params (struct sched_dl_entity *dl_se);static inline bool dl_server_active (struct sched_dl_entity *dl_se)
Signature · 1 line
2.51.0
Andrea Righi
reviewerPATCH 11/14 sched/deadline: Fix DL server crash in inactive_timer callback
In reply to Andrea RighiFrom: Joel Fernandes <[email protected]>
When sched_ext is rapidly disabled/enabled (the reload_loop selftest), the following crash is observed. This happens because the timer handler could not be cancelled and still fires even though the dl_server bandwidth may have been removed via dl_server_remove_params(). hrtimer_try_to_cancel() does not guarantee timer cancellation. This results in a NULL pointer dereference as 'p' is bogus for a dl_se.
This happens because the timer may be about to run, but its softirq has not executed yet. Because of that hrtimer_try_to_cancel() cannot prevent the timer from being canceled, however dl_server is still set to NULL by dl_server_apply_params(). When the timer handler eventually runs, it crashes.
[ 24.771835] BUG: kernel NULL pointer dereference, address: 000000000000006c [ 24.772097] #PF: supervisor read access in kernel mode [ 24.772248] #PF: error_code(0x0000) - not-present page [ 24.772404] PGD 0 P4D 0 [ 24.772499] Oops: Oops: 0000 [#1] SMP PTI [ 24.772614] CPU: 9 UID: 0 PID: 0 Comm: swapper/9 [..] #74 PREEMPT(voluntary) [ 24.772932] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), [...] [ 24.773149] Sched_ext: maximal (disabling) [ 24.773944] RSP: 0018:ffffb162c0348ee0 EFLAGS: 00010046 [ 24.774100] RAX: 0000000000000000 RBX: 0000000000000000 RCX: ffff88d4412f1800 [ 24.774302] RDX: 0000000000000001 RSI: 0000000000000010 RDI: ffffffffac939240 [ 24.774498] RBP: ffff88d47e65b940 R08: 0000000000000010 R09: 00000008bad3370a [ 24.774742] R10: 0000000000000000 R11: ffffffffa9f159d0 R12: ffff88d47e65b900 [ 24.774962] R13: ffff88d47e65b960 R14: ffff88d47e66a340 R15: ffff88d47e66aed0 [ 24.775182] FS: 0000000000000000(0000) GS:ffff88d4d1d56000(0000) knlGS:[...] [ 24.775392] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 24.775579] CR2: 000000000000006c CR3: 0000000002bb0003 CR4: 0000000000770ef0 [ 24.775810] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 24.776023] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 [ 24.776225] PKRU: 55555554 [ 24.776292] Call Trace: [ 24.776373] <IRQ> [ 24.776453] ? __pfx_inactive_task_timer+0x10/0x10 [ 24.776591] __hrtimer_run_queues+0xf1/0x270 [ 24.776744] hrtimer_interrupt+0xfa/0x220 [ 24.776847] __sysvec_apic_timer_interrupt+0x4d/0x190 [ 24.776988] sysvec_apic_timer_interrupt+0x69/0x80 [ 24.777132] </IRQ> [ 24.777194] <TASK> [ 24.777256] asm_sysvec_apic_timer_interrupt+0x1a/0x20
Fix by also checking the DL server's pick_task pointer which only exists for server tasks. This avoids dereferencing invalid task pointers when the timer fires after the DL server has been disabled.
[ arighi: replace ->server_has_tasks with ->server_pick_task ]
--- kernel/sched/deadline.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)
kernel/sched/deadline.c +12 -3
@@ -1784,7 +1784,16 @@ static enum hrtimer_restart inactive_task_timer(struct hrtimer *timer)struct rq_flags rf;struct rq *rq; -if (!dl_server (dl_se)) { +/* +* It is possible that after dl_server_apply_params(), the +* dl_se->dl_server == NULL, but the inactive timer is still queued +* and could not get canceled. +* +* Double check by looking at ->server_pick_tasks to make sure +* we're dealing with a non-server entity. Otherwise p may be bogus +* and we'll crash. +*/ +if (!dl_server (dl_se) && !dl_se->server_pick_task) { p =dl_task_of (dl_se); rq =task_rq_lock (p, &rf); }else {
@@ -1795,7 +1804,7 @@ static enum hrtimer_restart inactive_task_timer(struct hrtimer *timer)sched_clock_tick ();update_rq_clock (rq); -if (dl_server (dl_se)) +if (dl_server (dl_se) || dl_se->server_pick_task) goto no_task;if (!dl_task (p) ||READ_ONCE (p->__state) ==TASK_DEAD ) {
@@ -1823,7 +1832,7 @@ static enum hrtimer_restart inactive_task_timer(struct hrtimer *timer) dl_se->dl_non_contending =0 ; unlock: -if (!dl_server (dl_se)) { +if (!dl_server (dl_se) && !dl_se->server_pick_task) {task_rq_unlock (rq, p, &rf);put_task_struct (p); }else {
Signature · 1 line
2.51.0
Andrea Righi
reviewerPATCH 12/14 sched_ext: Selectively enable ext and fair DL servers
In reply to Andrea RighiEnable or disable the appropriate DL servers (ext and fair) depending on whether an scx scheduler is started in full or partial mode:
- in full mode, disable the fair DL server and enable the ext DL server on all online CPUs,
- in partial mode (%SCX_OPS_SWITCH_PARTIAL), keep both fair and ext DL servers active to support tasks in both scheduling classes.
Additionally, handle CPU hotplug events by selectively enabling or disabling the relevant DL servers on the CPU that is going offline/online. This ensures correct bandwidth reservation also when CPUs are brought online or offline.
--- kernel/sched/ext.c | 97 +++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 87 insertions(+), 10 deletions(-)
kernel/sched/ext.c +87 -10
@@ -2545,6 +2545,57 @@ static void set_cpus_allowed_scx(struct task_struct *p, p, (struct cpumask *)p->cpus_ptr); } +static void dl_server_on (struct rq *rq,bool switch_all) +{ +struct rq_flags rf; +int err; + +rq_lock_irqsave (rq, &rf); +update_rq_clock (rq); + +if (switch_all) { +/* +* If all fair tasks are moved to the scx scheduler, we +* don't need the fair DL servers anymore, so remove it. +* +* When the current scx scheduler is unloaded, the fair DL +* server will be re-initialized. +*/ +if (dl_server_active (&rq->fair_server)) +dl_server_stop (&rq->fair_server); +dl_server_remove_params (&rq->fair_server); + } + + err =dl_server_init_params (&rq->ext_server); +WARN_ON_ONCE (err); + +rq_unlock_irqrestore (rq, &rf); +} + +static void dl_server_off (struct rq *rq,bool switch_all) +{ +struct rq_flags rf; +int err; + +rq_lock_irqsave (rq, &rf); +update_rq_clock (rq); + +if (dl_server_active (&rq->ext_server)) +dl_server_stop (&rq->ext_server); +dl_server_remove_params (&rq->ext_server); + +if (switch_all) { +/* +* Re-initialize the fair DL server if it was previously disabled +* because all fair tasks had been moved to the ext class. +*/ + err =dl_server_init_params (&rq->fair_server); +WARN_ON_ONCE (err); + } + +rq_unlock_irqrestore (rq, &rf); +} +static void handle_hotplug (struct rq *rq,bool online) {struct scx_sched *sch = scx_root;
@@ -2560,9 +2611,20 @@ static void handle_hotplug(struct rq *rq, bool online)if (unlikely (!sch))return ; -if (scx_enabled ()) +if (scx_enabled ()) { +bool is_switching_all =READ_ONCE (scx_switching_all); +scx_idle_update_selcpu_topology (&sch->ops); +/* +* Update ext and fair DL servers on hotplug events. +*/ +if (online) +dl_server_on (rq, is_switching_all); +else +dl_server_off (rq, is_switching_all); + } +if (online &&SCX_HAS_OP (sch, cpu_online))SCX_CALL_OP (sch,SCX_KF_UNLOCKED , cpu_online,NULL , cpu);else if (!online &&SCX_HAS_OP (sch, cpu_offline))
@@ -3921,6 +3983,7 @@ static void scx_disable_workfn(struct kthread_work *work)struct scx_exit_info *ei = sch->exit_info;struct scx_task_iter sti;struct task_struct *p; +bool is_switching_all =READ_ONCE (scx_switching_all);int kind, cpu; kind =atomic_read (&sch->exit_kind);
@@ -3976,6 +4039,22 @@ static void scx_disable_workfn(struct kthread_work *work) scx_init_task_enabled = false; +for_each_online_cpu (cpu) { +struct rq *rq =cpu_rq (cpu); + +/* +* Invalidate all the rq clocks to prevent getting outdated +* rq clocks from a previous scx scheduler. +*/ +scx_rq_clock_invalidate (rq); + +/* +* We are unloading the sched_ext scheduler, we do not need its +* DL server bandwidth anymore, remove it for all CPUs. +*/ +dl_server_off (rq, is_switching_all); + } +scx_task_iter_start (&sti);while ((p =scx_task_iter_next_locked (&sti))) {unsigned int queue_flags =DEQUEUE_SAVE |DEQUEUE_MOVE |DEQUEUE_NOCLOCK ;
@@ -3997,15 +4076,6 @@ static void scx_disable_workfn(struct kthread_work *work)scx_task_iter_stop (&sti);percpu_up_write (&scx_fork_rwsem); -/* -* Invalidate all the rq clocks to prevent getting outdated -* rq clocks from a previous scx scheduler. -*/ -for_each_possible_cpu (cpu) { -struct rq *rq =cpu_rq (cpu); -scx_rq_clock_invalidate (rq); - } -/* no task is on scx, turn off all the switches and flush in-progress calls */ static_branch_disable (&__scx_enabled);bitmap_zero (sch->has_op,SCX_OPI_END );
@@ -4778,6 +4848,13 @@ static int scx_enable(struct sched_ext_ops *ops, struct bpf_link *link)put_task_struct (p); }scx_task_iter_stop (&sti); + +/* +* Enable the ext DL server on all online CPUs. +*/ +for_each_online_cpu (cpu) +dl_server_on (cpu_rq (cpu), !(ops->flags &SCX_OPS_SWITCH_PARTIAL )); +percpu_up_write (&scx_fork_rwsem);scx_bypass (false);
Signature · 1 line
2.51.0
Andrea Righi
reviewerPATCH 13/14 selftests/sched_ext: Add test for sched_ext dl_server
In reply to Andrea RighiAdd a selftest to validate the correct behavior of the deadline server for the ext_sched_class.
[ Joel: Replaced occurences of CFS in the test with EXT. ]
--- tools/testing/selftests/sched_ext/Makefile | 1 + .../selftests/sched_ext/rt_stall.bpf.c | 23 ++ tools/testing/selftests/sched_ext/rt_stall.c | 214 ++++++++++++++++++ 3 files changed, 238 insertions(+) create mode 100644 tools/testing/selftests/sched_ext/rt_stall.bpf.c create mode 100644 tools/testing/selftests/sched_ext/rt_stall.c
tools/testing/selftests/sched_ext/Makefile +1 -0
@@ -183,6 +183,7 @@ auto-test-targets := \ select_cpu_dispatch_bad_dsq \ select_cpu_dispatch_dbl_dsp \ select_cpu_vtime \ + rt_stall \ test_example \ testcase-targets := $(addsuffix .o,$(addprefix $(SCXOBJ_DIR)/,$(auto-test-targets)))
tools/testing/selftests/sched_ext/rt_stall.bpf.c new file +23 -0
@@ -0,0 +1,23 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* A scheduler that verified if RT tasks can stall SCHED_EXT tasks. +* +* Copyright (c) 2025 NVIDIA Corporation. +*/ + +#include <scx/common.bpf.h> + +char _license[] SEC("license") = "GPL"; + +UEI_DEFINE (uei); + +void BPF_STRUCT_OPS (rt_stall_exit ,struct scx_exit_info *ei) +{ +UEI_RECORD (uei, ei); +} + +SEC(".struct_ops.link") +struct sched_ext_ops rt_stall_ops = { + .exit = (void *)rt_stall_exit, + .name ="rt_stall" , +};
tools/testing/selftests/sched_ext/rt_stall.c new file +214 -0
214 lines not on this page · raw message
Signature · 1 line
2.51.0
Re: [PATCH 13/14] selftests/sched_ext: Add test for sched_ext dl_server
Quoted 10 lines · Andrea Righi
Add a selftest to validate the correct behavior of the deadline server for the
ext_sched_class.[ Joel: Replaced occurences of CFS in the test with EXT. ]
Co-developed-by Joel Fernandes <[email protected]>Signed-off-by Joel Fernandes <[email protected]>Signed-off-by Andrea Righi <[email protected]>---
Nits listed below, but otherwise:
Code review aside, on my VM the test alternates between 4.81% and 5.20% for me so it's working as expected.
Quoted 157 lines
tools/testing/selftests/sched_ext/Makefile | 1 + .../selftests/sched_ext/rt_stall.bpf.c | 23 ++ tools/testing/selftests/sched_ext/rt_stall.c | 214 ++++++++++++++++++ 3 files changed, 238 insertions(+) create mode 100644 tools/testing/selftests/sched_ext/rt_stall.bpf.c create mode 100644 tools/testing/selftests/sched_ext/rt_stall.ctools/testing/selftests/sched_ext/Makefile +1
-0@@ -183,6 +183,7 @@ auto-test-targets := \ select_cpu_dispatch_bad_dsq \ select_cpu_dispatch_dbl_dsp \ select_cpu_vtime \ + rt_stall \ test_example \ testcase-targets := $(addsuffix .o,$(addprefix $(SCXOBJ_DIR)/,$(auto-test-targets)))tools/testing/selftests/sched_ext/rt_stall.bpf.c new file +23
-0@@ -0,0 +1,23 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* A scheduler that verified if RT tasks can stall SCHED_EXT tasks. +* +* Copyright (c) 2025 NVIDIA Corporation. +*/ + +#include <scx/common.bpf.h> + +char _license[] SEC("license") = "GPL"; + +UEI_DEFINE (uei); + +void BPF_STRUCT_OPS (rt_stall_exit ,struct scx_exit_info *ei) +{ +UEI_RECORD (uei, ei); +} + +SEC(".struct_ops.link") +struct sched_ext_ops rt_stall_ops = { + .exit = (void *)rt_stall_exit, + .name ="rt_stall" , +};tools/testing/selftests/sched_ext/rt_stall.c new file +103
-0@@ -0,0 +1,214 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* Copyright (c) 2025 NVIDIA Corporation. +*/ +#define _GNU_SOURCE +#include <stdio.h> +#include <stdlib.h> +#include <unistd.h> +#include <sched.h> +#include <sys/prctl.h> +#include <sys/types.h> +#include <sys/wait.h> +#include <time.h> +#include <linux/sched.h> +#include <signal.h> +#include <bpf/bpf.h> +#include <scx/common.h> +#include <sys/wait.h> +#include <unistd.h> +#include "rt_stall.bpf.skel.h" +#include "scx_test.h" +#include "../kselftest.h" + +#define CORE_ID 0/* CPU to pin tasks to */ +#define RUN_TIME 5/* How long to run the test in seconds */ + +/* Simple busy-wait function for test tasks */ +static void process_func (void ) +{ +while (1 ) { +/* Busy wait */ +for (volatile unsigned long i =0 ; i <10000000UL ; i++) + ; + } +} + +/* Set CPU affinity to a specific core */ +static void set_affinity (int cpu) +{ +cpu_set_t mask; + +CPU_ZERO (&mask); +CPU_SET (cpu, &mask); +if (sched_setaffinity (0 ,sizeof (mask), &mask) !=0 ) { +perror ("sched_setaffinity" ); +exit (EXIT_FAILURE ); + } +} + +/* Set task scheduling policy and priority */ +static void set_sched (int policy,int priority) +{ +struct sched_param param; + + param.sched_priority = priority; +if (sched_setscheduler (0 , policy, ¶m) !=0 ) { +perror ("sched_setscheduler" ); +exit (EXIT_FAILURE ); + } +} + +/* Get process runtime from /proc/<pid>/stat */ +static float get_process_runtime (int pid) +{ +char path[256 ]; +FILE *file; +long utime, stime; +int fields; + +snprintf (path,sizeof (path),"/proc/%d/stat" , pid); + file =fopen (path,"r" ); +if (file ==NULL ) { +perror ("Failed to open stat file" ); +return -1 ; + } + +/* Skip the first 13 fields and read the 14th and 15th */ + fields =fscanf (file, +"%*d %*s %*c %*d %*d %*d %*d %*d %*u %*u %*u %*u %*u %lu %lu" , + &utime, &stime); +fclose (file); + +if (fields !=2 ) { +fprintf (stderr,"Failed to read stat file\n" ); +return -1 ; + } + +/* Calculate the total time spent in the process */ +long total_time = utime + stime; +long ticks_per_second =sysconf (_SC_CLK_TCK); +float runtime_seconds = total_time *1.0 / ticks_per_second; + +return runtime_seconds; +} + +static enum scx_test_status setup (void **ctx) +{ +struct rt_stall *skel; + + skel =rt_stall__open (); +SCX_FAIL_IF (!skel,"Failed to open" ); +SCX_ENUM_INIT (skel); +SCX_FAIL_IF (rt_stall__load (skel),"Failed to load skel" );
tools/testing/selftests/sched_ext/rt_stall.c line 113
Quoted 10 lines
tools/testing/selftests/sched_ext/rt_stall.c new file +10
-0+ + *ctx = skel; + +return SCX_TEST_PASS ; +} + +static bool sched_stress_test (void ) +{ +float cfs_runtime, rt_runtime, actual_ratio; +int cfs_pid, rt_pid;
I think it should be cfs_pid -> ext_pid, cfs_runtime -> ext_runtime
tools/testing/selftests/sched_ext/rt_stall.c line 114
Quoted 1 line
tools/testing/selftests/sched_ext/rt_stall.c new file +1
-0+float expected_min_ratio =0.04 ;/* 4% */
Maybe add a comment that explains the 4% value? As in, we're expecting it to be around 5% so 0.04 accounts for values close enough but below < 5%.
Quoted 35 lines
tools/testing/selftests/sched_ext/rt_stall.c new file +35
-0+ +ksft_print_header (); +ksft_set_plan (1 ); + +/* Create and set up a EXT task */ + cfs_pid =fork (); +if (cfs_pid ==0 ) { +set_affinity (CORE_ID ); +process_func (); +exit (0 ); + }else if (cfs_pid <0 ) { +perror ("fork for EXT task" ); +ksft_exit_fail (); + } + +/* Create an RT task */ + rt_pid =fork (); +if (rt_pid ==0 ) { +set_affinity (CORE_ID ); +set_sched (SCHED_FIFO ,50 ); +process_func (); +exit (0 ); + }else if (rt_pid <0 ) { +perror ("fork for RT task" ); +ksft_exit_fail (); + } + +/* Let the processes run for the specified time */ +sleep (RUN_TIME ); + +/* Get runtime for the EXT task */ + cfs_runtime =get_process_runtime (cfs_pid); +if (cfs_runtime !=-1 ) +ksft_print_msg ("Runtime of EXT task (PID %d) is %f seconds\n" , + cfs_pid, cfs_runtime);
tools/testing/selftests/sched_ext/rt_stall.c line 159
Quoted 10 lines
tools/testing/selftests/sched_ext/rt_stall.c new file +10
-0+else +ksft_exit_fail_msg ("Error getting runtime for EXT task (PID %d)\n" , cfs_pid); + +/* Get runtime for the RT task */ + rt_runtime =get_process_runtime (rt_pid); +if (rt_runtime !=-1 ) +ksft_print_msg ("Runtime of RT task (PID %d) is %f seconds\n" , rt_pid, rt_runtime); +else +ksft_exit_fail_msg ("Error getting runtime for RT task (PID %d)\n" , rt_pid); +
Minor, but why not
if (rt_runtime == -1)
ksft_exit_fail_msg("Error getting runtime for RT task (PID
%d)\n", rt_pid);ksft_print_msg("Runtime of RT task (PID %d) is %f seconds\n", rt_pid, rt_runtime);
since ksft_exit_fail_msg never returns?
Quoted 59 lines
tools/testing/selftests/sched_ext/rt_stall.c new file +55
-1+/* Kill the processes */ +kill (cfs_pid,SIGKILL ); +kill (rt_pid,SIGKILL ); +waitpid (cfs_pid,NULL ,0 ); +waitpid (rt_pid,NULL ,0 ); + +/* Verify that the scx task got enough runtime */ + actual_ratio = cfs_runtime / (cfs_runtime + rt_runtime); +ksft_print_msg ("EXT task got %.2f%% of total runtime\n" , actual_ratio *100 ); + +if (actual_ratio >= expected_min_ratio) { +ksft_test_result_pass ("PASS: EXT task got more than %.2f%% of runtime\n" , + expected_min_ratio *100 ); +return true; + } +ksft_test_result_fail ("FAIL: EXT task got less than %.2f%% of runtime\n" , + expected_min_ratio *100 ); +return false; +} + +static enum scx_test_status run (void *ctx) +{ +struct rt_stall *skel = ctx; +struct bpf_link *link; +bool res; + + link =bpf_map__attach_struct_ops (skel->maps.rt_stall_ops); +SCX_FAIL_IF (!link,"Failed to attach scheduler" ); + + res =sched_stress_test (); + +SCX_EQ (skel->data->uei.kind,EXIT_KIND (SCX_EXIT_NONE )); +bpf_link__destroy (link); + +if (!res) +ksft_exit_fail (); + +return SCX_TEST_PASS ; +} + +static void cleanup (void *ctx) +{ +struct rt_stall *skel = ctx; + +rt_stall__destroy (skel); +} + +struct scx_test rt_stall = { + .name ="rt_stall" , + .description ="Verify that RT tasks cannot stall SCHED_EXT tasks" , + .setup = setup, + .run = run, + .cleanup = cleanup, +}; +REGISTER_SCX_TEST (&rt_stall) --2.51.0
Andrea Righi
reviewerRe: [PATCH 13/14] selftests/sched_ext: Add test for sched_ext dl_server
Hi Emil,
Quoted 12 lines · Emil Tsalapatis
Quoted 10 lines · Andrea Righi
Add a selftest to validate the correct behavior of the deadline server for the
ext_sched_class.[ Joel: Replaced occurences of CFS in the test with EXT. ]
Co-developed-by Joel Fernandes <[email protected]>Signed-off-by Joel Fernandes <[email protected]>Signed-off-by Andrea Righi <[email protected]>---
Quoted 5 lines
Nits listed below, but otherwise:
Reviewed-by Emil Tsalapatis <[email protected]>Code review aside, on my VM the test alternates between 4.81% and 5.20% for me so it's working as expected.
Yeah, that sounds right, a bit of fluctuation like that is expected.
Quoted 171 lines
Quoted 158 lines
tools/testing/selftests/sched_ext/Makefile | 1 + .../selftests/sched_ext/rt_stall.bpf.c | 23 ++ tools/testing/selftests/sched_ext/rt_stall.c | 214 ++++++++++++++++++ 3 files changed, 238 insertions(+) create mode 100644 tools/testing/selftests/sched_ext/rt_stall.bpf.c create mode 100644 tools/testing/selftests/sched_ext/rt_stall.ctools/testing/selftests/sched_ext/Makefile +1
-0@@ -183,6 +183,7 @@ auto-test-targets := \ select_cpu_dispatch_bad_dsq \ select_cpu_dispatch_dbl_dsp \ select_cpu_vtime \ + rt_stall \ test_example \ testcase-targets := $(addsuffix .o,$(addprefix $(SCXOBJ_DIR)/,$(auto-test-targets)))tools/testing/selftests/sched_ext/rt_stall.bpf.c new file +23
-0@@ -0,0 +1,23 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* A scheduler that verified if RT tasks can stall SCHED_EXT tasks. +* +* Copyright (c) 2025 NVIDIA Corporation. +*/ + +#include <scx/common.bpf.h> + +char _license[] SEC("license") = "GPL"; + +UEI_DEFINE (uei); + +void BPF_STRUCT_OPS (rt_stall_exit ,struct scx_exit_info *ei) +{ +UEI_RECORD (uei, ei); +} + +SEC(".struct_ops.link") +struct sched_ext_ops rt_stall_ops = { + .exit = (void *)rt_stall_exit, + .name ="rt_stall" , +};tools/testing/selftests/sched_ext/rt_stall.c new file +103
-0@@ -0,0 +1,214 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* Copyright (c) 2025 NVIDIA Corporation. +*/ +#define _GNU_SOURCE +#include <stdio.h> +#include <stdlib.h> +#include <unistd.h> +#include <sched.h> +#include <sys/prctl.h> +#include <sys/types.h> +#include <sys/wait.h> +#include <time.h> +#include <linux/sched.h> +#include <signal.h> +#include <bpf/bpf.h> +#include <scx/common.h> +#include <sys/wait.h> +#include <unistd.h> +#include "rt_stall.bpf.skel.h" +#include "scx_test.h" +#include "../kselftest.h" + +#define CORE_ID 0/* CPU to pin tasks to */ +#define RUN_TIME 5/* How long to run the test in seconds */ + +/* Simple busy-wait function for test tasks */ +static void process_func (void ) +{ +while (1 ) { +/* Busy wait */ +for (volatile unsigned long i =0 ; i <10000000UL ; i++) + ; + } +} + +/* Set CPU affinity to a specific core */ +static void set_affinity (int cpu) +{ +cpu_set_t mask; + +CPU_ZERO (&mask); +CPU_SET (cpu, &mask); +if (sched_setaffinity (0 ,sizeof (mask), &mask) !=0 ) { +perror ("sched_setaffinity" ); +exit (EXIT_FAILURE ); + } +} + +/* Set task scheduling policy and priority */ +static void set_sched (int policy,int priority) +{ +struct sched_param param; + + param.sched_priority = priority; +if (sched_setscheduler (0 , policy, ¶m) !=0 ) { +perror ("sched_setscheduler" ); +exit (EXIT_FAILURE ); + } +} + +/* Get process runtime from /proc/<pid>/stat */ +static float get_process_runtime (int pid) +{ +char path[256 ]; +FILE *file; +long utime, stime; +int fields; + +snprintf (path,sizeof (path),"/proc/%d/stat" , pid); + file =fopen (path,"r" ); +if (file ==NULL ) { +perror ("Failed to open stat file" ); +return -1 ; + } + +/* Skip the first 13 fields and read the 14th and 15th */ + fields =fscanf (file, +"%*d %*s %*c %*d %*d %*d %*d %*d %*u %*u %*u %*u %*u %lu %lu" , + &utime, &stime); +fclose (file); + +if (fields !=2 ) { +fprintf (stderr,"Failed to read stat file\n" ); +return -1 ; + } + +/* Calculate the total time spent in the process */ +long total_time = utime + stime; +long ticks_per_second =sysconf (_SC_CLK_TCK); +float runtime_seconds = total_time *1.0 / ticks_per_second; + +return runtime_seconds; +} + +static enum scx_test_status setup (void **ctx) +{ +struct rt_stall *skel; + + skel =rt_stall__open (); +SCX_FAIL_IF (!skel,"Failed to open" ); +SCX_ENUM_INIT (skel); +SCX_FAIL_IF (rt_stall__load (skel),"Failed to load skel" );tools/testing/selftests/sched_ext/rt_stall.c line 113
Quoted 10 lines
tools/testing/selftests/sched_ext/rt_stall.c new file +10
-0+ + *ctx = skel; + +return SCX_TEST_PASS ; +} + +static bool sched_stress_test (void ) +{ +float cfs_runtime, rt_runtime, actual_ratio; +int cfs_pid, rt_pid;I think it should be
cfs_pid->ext_pid,cfs_runtime->ext_runtime
Quoted 5 lines
tools/testing/selftests/sched_ext/rt_stall.c line 114
Quoted 1 line
tools/testing/selftests/sched_ext/rt_stall.c new file +1
-0+float expected_min_ratio =0.04 ;/* 4% */ Maybe add a comment that explains the 4% value? As in, we're expecting it to be around 5% so 0.04 accounts for values close enough but below < 5%.
Makes sense, I’ll add this comment (or something along those lines).
Quoted 49 lines
Quoted 36 lines
tools/testing/selftests/sched_ext/rt_stall.c new file +35
-0+ +ksft_print_header (); +ksft_set_plan (1 ); + +/* Create and set up a EXT task */ + cfs_pid =fork (); +if (cfs_pid ==0 ) { +set_affinity (CORE_ID ); +process_func (); +exit (0 ); + }else if (cfs_pid <0 ) { +perror ("fork for EXT task" ); +ksft_exit_fail (); + } + +/* Create an RT task */ + rt_pid =fork (); +if (rt_pid ==0 ) { +set_affinity (CORE_ID ); +set_sched (SCHED_FIFO ,50 ); +process_func (); +exit (0 ); + }else if (rt_pid <0 ) { +perror ("fork for RT task" ); +ksft_exit_fail (); + } + +/* Let the processes run for the specified time */ +sleep (RUN_TIME ); + +/* Get runtime for the EXT task */ + cfs_runtime =get_process_runtime (cfs_pid); +if (cfs_runtime !=-1 ) +ksft_print_msg ("Runtime of EXT task (PID %d) is %f seconds\n" , + cfs_pid, cfs_runtime);tools/testing/selftests/sched_ext/rt_stall.c line 159
Quoted 10 lines
tools/testing/selftests/sched_ext/rt_stall.c new file +10
-0+else +ksft_exit_fail_msg ("Error getting runtime for EXT task (PID %d)\n" , cfs_pid); + +/* Get runtime for the RT task */ + rt_runtime =get_process_runtime (rt_pid); +if (rt_runtime !=-1 ) +ksft_print_msg ("Runtime of RT task (PID %d) is %f seconds\n" , rt_pid, rt_runtime); +else +ksft_exit_fail_msg ("Error getting runtime for RT task (PID %d)\n" , rt_pid); +Minor, but why not
Quoted 7 lines
if (
rt_runtime== -1)
ksft_exit_fail_msg("Error getting runtime for RT task (PID
%d)\n",rt_pid);ksft_print_msg("Runtime of RT task (PID %d) is %f seconds\n", rt_pid, rt_runtime);since
ksft_exit_fail_msgnever returns?
Ack.
Quoted 52 lines
Quoted 52 lines
tools/testing/selftests/sched_ext/rt_stall.c new file +51
-0+/* Kill the processes */ +kill (cfs_pid,SIGKILL ); +kill (rt_pid,SIGKILL ); +waitpid (cfs_pid,NULL ,0 ); +waitpid (rt_pid,NULL ,0 ); + +/* Verify that the scx task got enough runtime */ + actual_ratio = cfs_runtime / (cfs_runtime + rt_runtime); +ksft_print_msg ("EXT task got %.2f%% of total runtime\n" , actual_ratio *100 ); + +if (actual_ratio >= expected_min_ratio) { +ksft_test_result_pass ("PASS: EXT task got more than %.2f%% of runtime\n" , + expected_min_ratio *100 ); +return true; + } +ksft_test_result_fail ("FAIL: EXT task got less than %.2f%% of runtime\n" , + expected_min_ratio *100 ); +return false; +} + +static enum scx_test_status run (void *ctx) +{ +struct rt_stall *skel = ctx; +struct bpf_link *link; +bool res; + + link =bpf_map__attach_struct_ops (skel->maps.rt_stall_ops); +SCX_FAIL_IF (!link,"Failed to attach scheduler" ); + + res =sched_stress_test (); + +SCX_EQ (skel->data->uei.kind,EXIT_KIND (SCX_EXIT_NONE )); +bpf_link__destroy (link); + +if (!res) +ksft_exit_fail (); + +return SCX_TEST_PASS ; +} + +static void cleanup (void *ctx) +{ +struct rt_stall *skel = ctx; + +rt_stall__destroy (skel); +} + +struct scx_test rt_stall = { + .name ="rt_stall" , + .description ="Verify that RT tasks cannot stall SCHED_EXT tasks" , + .setup = setup,
Quoted 8 lines
Quoted 8 lines
tools/testing/selftests/sched_ext/rt_stall.c new file +4
-1+ .run = run, + .cleanup = cleanup, +}; +REGISTER_SCX_TEST (&rt_stall) --2.51.0
Thanks,
-Andrea
Andrea Righi
reviewerRe: [PATCH 13/14] selftests/sched_ext: Add test for sched_ext dl_server
Quoted 1 line · Andrea Righi
Hi Emil,
...
Quoted 6 lines
Quoted 6 lines
tools/testing/selftests/sched_ext/rt_stall.c line 113
Quoted 4 lines
tools/testing/selftests/sched_ext/rt_stall.c new file +4
-0+static bool sched_stress_test (void ) +{ +float cfs_runtime, rt_runtime, actual_ratio; +int cfs_pid, rt_pid;I think it should be
cfs_pid->ext_pid,cfs_runtime->ext_runtime
Oh and ack to this as well, using CFS in general is a bit confusing at this point.
-Andrea
Re: [PATCH 13/14] selftests/sched_ext: Add test for sched_ext dl_server
In reply to Andrea RighiQuoted 8 lines · Andrea Righi
Add a selftest to validate the correct behavior of the deadline server for the
ext_sched_class.[ Joel: Replaced occurences of CFS in the test with EXT. ]
Co-developed-by Joel Fernandes <[email protected]>Signed-off-by Joel Fernandes <[email protected]>Signed-off-by Andrea Righi <[email protected]>
Quoted 7 lines
--- tools/testing/selftests/sched_ext/Makefile | 1 + .../selftests/sched_ext/rt_stall.bpf.c | 23 ++ tools/testing/selftests/sched_ext/rt_stall.c | 214 ++++++++++++++++++ 3 files changed, 238 insertions(+) create mode 100644 tools/testing/selftests/sched_ext/rt_stall.bpf.c create mode 100644 tools/testing/selftests/sched_ext/rt_stall.c
Does this pass consistently for you?
For a loop of 1000 runs I'm getting total runtime numbers for the EXT task of:
0.000 - 0.261 | (7) 0.261 - 0.522 | ###### (86) 0.522 - 4.437 | (0) 4.437 - 4.698 | (1) 4.698 - 4.959 | ################### (257) 4.959 - 5.220 | ################################################## (649)
I'll try to see what's going wrong here...
Andrea Righi
reviewerRe: [PATCH 13/14] selftests/sched_ext: Add test for sched_ext dl_server
Hi Christian,
Quoted 21 lines · Christian Loehle
Quoted 8 lines · Andrea Righi
Add a selftest to validate the correct behavior of the deadline server for the
ext_sched_class.[ Joel: Replaced occurences of CFS in the test with EXT. ]
Co-developed-by Joel Fernandes <[email protected]>Signed-off-by Joel Fernandes <[email protected]>Signed-off-by Andrea Righi <[email protected]>Quoted 7 lines
--- tools/testing/selftests/sched_ext/Makefile | 1 + .../selftests/sched_ext/rt_stall.bpf.c | 23 ++ tools/testing/selftests/sched_ext/rt_stall.c | 214 ++++++++++++++++++ 3 files changed, 238 insertions(+) create mode 100644 tools/testing/selftests/sched_ext/rt_stall.bpf.c create mode 100644 tools/testing/selftests/sched_ext/rt_stall.cDoes this pass consistently for you?
For a loop of 1000 runs I'm getting total runtime numbers for the EXT task of:
Quoted 8 lines
0.000 - 0.261 | (7) 0.261 - 0.522 | ###### (86) 0.522 - 4.437 | (0) 4.437 - 4.698 | (1) 4.698 - 4.959 | ################### (257) 4.959 - 5.220 | ################################################## (649)I'll try to see what's going wrong here...
Is that 1000 runs of total_bw? Yeah, the small ones don't look right at all, unless they're caused by some errors in the measurement (or something wrong in the test itself). Still better than without the dl_server, but it'd be nice to understand what's going on. :)
I'll try to reproduce that on my side as well.
Thanks,
-Andrea
Andrea Righi
reviewerRe: [PATCH 13/14] selftests/sched_ext: Add test for sched_ext dl_server
Quoted 31 lines · Andrea Righi
Hi Christian,
Quoted 28 lines · Christian Loehle
Quoted 8 lines · Andrea Righi
Add a selftest to validate the correct behavior of the deadline server for the
ext_sched_class.[ Joel: Replaced occurences of CFS in the test with EXT. ]
Co-developed-by Joel Fernandes <[email protected]>Signed-off-by Joel Fernandes <[email protected]>Signed-off-by Andrea Righi <[email protected]>Quoted 7 lines
--- tools/testing/selftests/sched_ext/Makefile | 1 + .../selftests/sched_ext/rt_stall.bpf.c | 23 ++ tools/testing/selftests/sched_ext/rt_stall.c | 214 ++++++++++++++++++ 3 files changed, 238 insertions(+) create mode 100644 tools/testing/selftests/sched_ext/rt_stall.bpf.c create mode 100644 tools/testing/selftests/sched_ext/rt_stall.cDoes this pass consistently for you?
For a loop of 1000 runs I'm getting total runtime numbers for the EXT task of:0.000 - 0.261 | (7) 0.261 - 0.522 | ###### (86) 0.522 - 4.437 | (0) 4.437 - 4.698 | (1) 4.698 - 4.959 | ################### (257) 4.959 - 5.220 | ################################################## (649)
Quoted 3 lines
Quoted 1 line
I'll try to see what's going wrong here...
Is that 1000 runs of
total_bw? Yeah, the small ones don't look right at
s/total_bw/rt_stall/
-Andrea
Quoted 8 lines
all, unless they're caused by some errors in the measurement (or something wrong in the test itself). Still better than without the
dl_server, but it'd be nice to understand what's going on. :)I'll try to reproduce that on my side as well.
Thanks,
-Andrea
Re: [PATCH 13/14] selftests/sched_ext: Add test for sched_ext dl_server
In reply to Andrea RighiQuoted 31 lines · Andrea Righi
Hi Christian,
Quoted 28 lines · Christian Loehle
Quoted 8 lines · Andrea Righi
Add a selftest to validate the correct behavior of the deadline server for the
ext_sched_class.[ Joel: Replaced occurences of CFS in the test with EXT. ]
Co-developed-by Joel Fernandes <[email protected]>Signed-off-by Joel Fernandes <[email protected]>Signed-off-by Andrea Righi <[email protected]>Quoted 7 lines
--- tools/testing/selftests/sched_ext/Makefile | 1 + .../selftests/sched_ext/rt_stall.bpf.c | 23 ++ tools/testing/selftests/sched_ext/rt_stall.c | 214 ++++++++++++++++++ 3 files changed, 238 insertions(+) create mode 100644 tools/testing/selftests/sched_ext/rt_stall.bpf.c create mode 100644 tools/testing/selftests/sched_ext/rt_stall.cDoes this pass consistently for you?
For a loop of 1000 runs I'm getting total runtime numbers for the EXT task of:0.000 - 0.261 | (7) 0.261 - 0.522 | ###### (86) 0.522 - 4.437 | (0) 4.437 - 4.698 | (1) 4.698 - 4.959 | ################### (257) 4.959 - 5.220 | ################################################## (649)
Quoted 9 lines
Quoted 1 line
I'll try to see what's going wrong here...
Is that 1000 runs of
total_bw? Yeah, the small ones don't look right at all, unless they're caused by some errors in the measurement (or something wrong in the test itself). Still better than without thedl_server, but it'd be nice to understand what's going on. :)I'll try to reproduce that on my side as well.
Yes it's pretty much
for i in $(seq 0 999); do ./runner -t rt_stall ; sleep 10; done
I also tried to increase the runtime of the test, but results look the same so I assume the DL server isn't running in the fail cases.
Re: [PATCH 13/14] selftests/sched_ext: Add test for sched_ext dl_server
Quoted 39 lines · Christian Loehle
Quoted 38 lines · Andrea Righi
Hi Christian,
Quoted 21 lines · Christian Loehle
Quoted 8 lines · Andrea Righi
Add a selftest to validate the correct behavior of the deadline server for the
ext_sched_class.[ Joel: Replaced occurences of CFS in the test with EXT. ]
Co-developed-by Joel Fernandes <[email protected]>Signed-off-by Joel Fernandes <[email protected]>Signed-off-by Andrea Righi <[email protected]>Quoted 7 lines
--- tools/testing/selftests/sched_ext/Makefile | 1 + .../selftests/sched_ext/rt_stall.bpf.c | 23 ++ tools/testing/selftests/sched_ext/rt_stall.c | 214 ++++++++++++++++++ 3 files changed, 238 insertions(+) create mode 100644 tools/testing/selftests/sched_ext/rt_stall.bpf.c create mode 100644 tools/testing/selftests/sched_ext/rt_stall.cDoes this pass consistently for you?
For a loop of 1000 runs I'm getting total runtime numbers for the EXT task of:Quoted 8 lines
0.000 - 0.261 | (7) 0.261 - 0.522 | ###### (86) 0.522 - 4.437 | (0) 4.437 - 4.698 | (1) 4.698 - 4.959 | ################### (257) 4.959 - 5.220 | ################################################## (649)I'll try to see what's going wrong here...
Is that 1000 runs of
total_bw? Yeah, the small ones don't look right at all, unless they're caused by some errors in the measurement (or something wrong in the test itself). Still better than without thedl_server, but it'd be nice to understand what's going on. :)
Quoted 9 lines
Quoted 2 lines
I'll try to reproduce that on my side as well.
Yes it's pretty much
for i in $(seq 0 999); do ./runner -trt_stall; sleep 10; doneI also tried to increase the runtime of the test, but results look the same so I assume the DL server isn't running in the fail cases.
FWIW the below fixes the issue and also explains why runtime of the test was irrelevant. I wonder if we should let the test do FAIR->EXT->FAIR->EXT or something like that, the change would be minimal and coverage improved significantly IMO.
-----8<-----
kernel/sched/ext.c +2 -0
@@ -2568,6 +2568,8 @@ static void dl_server_on(struct rq *rq, bool switch_all) err =dl_server_init_params (&rq->ext_server);WARN_ON_ONCE (err); +if (rq->scx.nr_running) +dl_server_start (&rq->ext_server);rq_unlock_irqrestore (rq, &rf); }
Andrea Righi
reviewerRe: [PATCH 13/14] selftests/sched_ext: Add test for sched_ext dl_server
Quoted 43 lines · Christian Loehle
Quoted 42 lines · Christian Loehle
Quoted 40 lines · Andrea Righi
Hi Christian,
Quoted 21 lines · Christian Loehle
Quoted 8 lines · Andrea Righi
Add a selftest to validate the correct behavior of the deadline server for the
ext_sched_class.[ Joel: Replaced occurences of CFS in the test with EXT. ]
Co-developed-by Joel Fernandes <[email protected]>Signed-off-by Joel Fernandes <[email protected]>Signed-off-by Andrea Righi <[email protected]>Quoted 7 lines
--- tools/testing/selftests/sched_ext/Makefile | 1 + .../selftests/sched_ext/rt_stall.bpf.c | 23 ++ tools/testing/selftests/sched_ext/rt_stall.c | 214 ++++++++++++++++++ 3 files changed, 238 insertions(+) create mode 100644 tools/testing/selftests/sched_ext/rt_stall.bpf.c create mode 100644 tools/testing/selftests/sched_ext/rt_stall.cDoes this pass consistently for you?
For a loop of 1000 runs I'm getting total runtime numbers for the EXT task of:Quoted 8 lines
0.000 - 0.261 | (7) 0.261 - 0.522 | ###### (86) 0.522 - 4.437 | (0) 4.437 - 4.698 | (1) 4.698 - 4.959 | ################### (257) 4.959 - 5.220 | ################################################## (649)I'll try to see what's going wrong here...
Is that 1000 runs of
total_bw? Yeah, the small ones don't look right at all, unless they're caused by some errors in the measurement (or something wrong in the test itself). Still better than without thedl_server, but it'd be nice to understand what's going on. :)I'll try to reproduce that on my side as well.
Quoted 8 lines
Quoted 6 lines
Yes it's pretty much
for i in $(seq 0 999); do ./runner -trt_stall; sleep 10; doneI also tried to increase the runtime of the test, but results look the same so I assume the DL server isn't running in the fail cases.
FWIW the below fixes the issue and also explains why runtime of the test was irrelevant.
Ah, good catch Christian! this makes sense to me, I'll also run some tests on my side with this applied and I'll include it in the next patch series.
Quoted 2 lines
I wonder if we should let the test do
FAIR->EXT->FAIR->EXTor something like that, the change would be minimal and coverage improved significantly IMO.
I agree, running a couple of rounds of fair->ext seems reasonable to me and it can potentially trigger more issues in advance.
Thanks,
-Andrea
Quoted 17 lines
-----8<-----
kernel/sched/ext.c +2
-0@@ -2568,6 +2568,8 @@ static void dl_server_on(struct rq *rq, bool switch_all) err =dl_server_init_params (&rq->ext_server);WARN_ON_ONCE (err); +if (rq->scx.nr_running) +dl_server_start (&rq->ext_server);rq_unlock_irqrestore (rq, &rf); }
Andrea Righi
reviewerPATCH 14/14 selftests/sched_ext: Add test for DL server total_bw consistency
In reply to Andrea RighiFrom: Joel Fernandes <[email protected]>
Add a new kselftest to verify that the total_bw value in /sys/kernel/debug/sched/debug remains consistent across all CPUs under different sched_ext BPF program states:
1. Before a BPF scheduler is loaded
2. While a BPF scheduler is loaded and active
3. After a BPF scheduler is unloaded
The test runs CPU stress threads to ensure DL server bandwidth values stabilize before checking consistency. This helps catch potential issues with DL server bandwidth accounting during sched_ext transitions.
[ arighi: small coding style fixes ]
--- tools/testing/selftests/sched_ext/Makefile | 1 + tools/testing/selftests/sched_ext/total_bw.c | 281 +++++++++++++++++++ 2 files changed, 282 insertions(+) create mode 100644 tools/testing/selftests/sched_ext/total_bw.c
tools/testing/selftests/sched_ext/Makefile +1 -0
@@ -185,6 +185,7 @@ auto-test-targets := \ select_cpu_vtime \ rt_stall \ test_example \ + total_bw \ testcase-targets := $(addsuffix .o,$(addprefix $(SCXOBJ_DIR)/,$(auto-test-targets)))
tools/testing/selftests/sched_ext/total_bw.c new file +281 -0
281 lines not on this page · raw message
Signature · 1 line
2.51.0