[PATCH] sched_ext: Fix timer pinning and return value in scx_central

[PATCH] sched_ext: Fix timer pinning and return value in scx_central

[Wanwu Li]

central_timerfn() re-arms the timer with a hardcoded
BPF_F_TIMER_CPU_PIN flag and ignores the return value, defeating
central_init()'s -EINVAL fallback for kernels without the flag
(<6.7): on such kernels the first tick kills the timer permanently
with no diagnostic.  Honor timer_pinned and check the return like
the init path does.

Fixes: 22a920209ab6 ("sched_ext: Implement tickless support")
Signed-off-by: Wanwu Li <liwanwu@kylinos.cn>
---
diff --git a/tools/sched_ext/scx_central.bpf.c b/tools/sched_ext/scx_central.bpf.c
index 64dd60b3e922..65dae9e45400 100644
--- a/tools/sched_ext/scx_central.bpf.c
+++ b/tools/sched_ext/scx_central.bpf.c
@@ -299,6 +299,7 @@ static int central_timerfn(void *map, int *key, struct bpf_timer *timer)
 	u64 now = scx_bpf_now();
 	u64 nr_to_kick = nr_queued;
 	s32 i, curr_cpu;
+	int ret;
 
 	curr_cpu = bpf_get_smp_processor_id();
 	if (timer_pinned && (curr_cpu != central_cpu)) {
@@ -332,7 +333,10 @@ static int central_timerfn(void *map, int *key, struct bpf_timer *timer)
 		scx_bpf_kick_cpu(cpu, SCX_KICK_PREEMPT);
 	}
 
-	bpf_timer_start(timer, TIMER_INTERVAL_NS, BPF_F_TIMER_CPU_PIN);
+	ret = bpf_timer_start(timer, TIMER_INTERVAL_NS,
+			      timer_pinned ? BPF_F_TIMER_CPU_PIN : 0);
+	if (ret)
+		scx_bpf_error("bpf_timer_start failed (%d)", ret);
 	__sync_fetch_and_add(&nr_timers, 1);
 	return 0;
 }

[PATCH] sched_ext: Fix vtime delta loss in scx_flatcg cgroup migration

[Wanwu Li]

fcg_cgroup_move() lost the signed vtime offset across cgroup
migration in the mechanical conversion to time helpers:
time_delta() clamps negative deltas to 0, so a queued task (whose
dsq_vtime is normally behind the source frontier) loses its
accumulated vtime credit and lands exactly at the destination
frontier instead of keeping its relative position.  Restore the
wrapping signed subtraction.

Fixes: 62addc6dbf36 ("sched_ext: Use time helpers in BPF schedulers")
Signed-off-by: Wanwu Li <liwanwu@kylinos.cn>
---
diff --git a/tools/sched_ext/scx_flatcg.bpf.c b/tools/sched_ext/scx_flatcg.bpf.c
index 64cf4dd964d6..d3186a6038c3 100644
--- a/tools/sched_ext/scx_flatcg.bpf.c
+++ b/tools/sched_ext/scx_flatcg.bpf.c
@@ -931,14 +931,14 @@ void BPF_STRUCT_OPS(fcg_cgroup_move, struct task_struct *p,
 		    struct cgroup *from, struct cgroup *to)
 {
 	struct fcg_cgrp_ctx *from_cgc, *to_cgc;
 	s64 delta;
 
 	/* find_cgrp_ctx() triggers scx_bpf_error() on lookup failures */
 	if (!(from_cgc = find_cgrp_ctx(from)) || !(to_cgc = find_cgrp_ctx(to)))
 		return;
 
-	delta = time_delta(p->scx.dsq_vtime, from_cgc->tvtime_now);
+	delta = (s64)(p->scx.dsq_vtime - from_cgc->tvtime_now);
 	scx_bpf_task_set_dsq_vtime(p, to_cgc->tvtime_now + delta);
 }
 
 s32 BPF_STRUCT_OPS_SLEEPABLE(fcg_init)

[PATCH] sched_ext: Check bpf_timer_start return values in scx_qmap

[Wanwu Li]

monitor_timerfn(), lowpri_timerfn() and round_robin_timerfn() ignore
bpf_timer_start()'s return value: a failed re-arm silently stops the
periodic heartbeat, starving every task parked in LOWPRI_DSQ (lowpri)
or freezing cid rotation (round-robin).  Check the returns and raise
scx_bpf_error(), matching the init paths.

Signed-off-by: Wanwu Li <liwanwu@kylinos.cn>
---
 tools/sched_ext/scx_qmap.bpf.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/tools/sched_ext/scx_qmap.bpf.c b/tools/sched_ext/scx_qmap.bpf.c
index 5bb8b90a275a..9f6e61d7ca07 100644
--- a/tools/sched_ext/scx_qmap.bpf.c
+++ b/tools/sched_ext/scx_qmap.bpf.c
@@ -1246,7 +1246,8 @@ static int monitor_timerfn(void *map, int *key, struct bpf_timer *timer)
 			   scx_read_event(&events, SCX_EV_BYPASS_ACTIVATE));
 	}
 
-	bpf_timer_start(timer, ONE_SEC_IN_NS, 0);
+	if (bpf_timer_start(timer, ONE_SEC_IN_NS, 0))
+		scx_bpf_error("failed to re-arm stats timer");
 	return 0;
 }
 
@@ -1268,7 +1269,8 @@ struct {
 static int lowpri_timerfn(void *map, int *key, struct bpf_timer *timer)
 {
 	scx_bpf_dsq_reenq(LOWPRI_DSQ, 0);
-	bpf_timer_start(timer, LOWPRI_INTV_NS, 0);
+	if (bpf_timer_start(timer, LOWPRI_INTV_NS, 0))
+		scx_bpf_error("failed to re-arm lowpri timer");
 	return 0;
 }
 
@@ -1747,7 +1749,8 @@ static void rr_advance(void)
 static int round_robin_timerfn(void *map, int *key, struct bpf_timer *timer)
 {
 	rr_advance();
-	bpf_timer_start(timer, round_robin_ns, 0);
+	if (bpf_timer_start(timer, round_robin_ns, 0))
+		scx_bpf_error("failed to re-arm round-robin timer");
 	return 0;
 }
 
-- 
2.25.1

Re: [PATCH] sched_ext: Check bpf_timer_start return values in scx_qmap

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Medium] central_timerfn() unconditionally re-arms the timer with BPF_F_TIMER_CPU_PIN and ignores the return value, causing silent failure on older kernels.
--

commit b5a91ea2da5adc3305be9b35666d557645b76fd2
Author: Wanwu Li <liwanwu@kylinos.cn>

sched_ext: Check bpf_timer_start return values in scx_qmap

This commit adds return value checks to bpf_timer_start() calls in several
timer functions within scx_qmap. If the timer fails to re-arm, it now raises
an error via scx_bpf_error() rather than silently failing.

> diff --git a/tools/sched_ext/scx_qmap.bpf.c b/tools/sched_ext/scx_qmap.bpf.c
> index 5bb8b90a275a3..9f6e61d7ca071 100644
> --- a/tools/sched_ext/scx_qmap.bpf.c
> +++ b/tools/sched_ext/scx_qmap.bpf.c
> @@ -1246,7 +1246,8 @@ static int monitor_timerfn(void *map, int *key, struct bpf_timer *timer)

[ ... ]

> -	bpf_timer_start(timer, ONE_SEC_IN_NS, 0);
> +	if (bpf_timer_start(timer, ONE_SEC_IN_NS, 0))
> +		scx_bpf_error("failed to re-arm stats timer");
>  	return 0;
>  }

[Severity: Medium]
This is a pre-existing issue, but while adding these return value checks here,
should we apply the same fix to central_timerfn() in scx_central.bpf.c?

It unconditionally re-arms the timer with BPF_F_TIMER_CPU_PIN and ignores
the return value:

tools/sched_ext/scx_central.bpf.c:central_timerfn() {
    ...
    bpf_timer_start(timer, TIMER_INTERVAL_NS, BPF_F_TIMER_CPU_PIN);
    __sync_fetch_and_add(&nr_timers, 1);
    return 0;
}

On older kernels that don't support BPF_F_TIMER_CPU_PIN, this fails with
-EINVAL. While start_central_timer() gracefully falls back to flags = 0
if unsupported, the timer callback unconditionally passes the flag. Because
the return value is ignored, the timer permanently stops, potentially
starving tasks.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260827080738.829103-3-liwanwu@kylinos.cn?part=1

Re: [PATCH] sched_ext: Fix timer pinning and return value in scx_central

[Tejun Heo]

Applied the following three patches to sched_ext/for-7.3-fixes:

  sched_ext: Fix timer pinning and return value in scx_central
  sched_ext: Fix vtime delta loss in scx_flatcg cgroup migration
  sched_ext: Check bpf_timer_start return values in scx_qmap

The scx_central description attributed the -EINVAL fallback to
central_init(). It has lived in start_central_timer() since d6edb15ad92c
("scx_central: Defer timer start to central dispatch to fix init error"), so
I updated the description accordingly.

Thanks.

--
tejun