[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: 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