* [PATCH v5 0/3] sched_ext: Harden scx_bpf_cpu_rq()
2025-09-01 13:26 [PATCH v4 0/3] sched_ext: Harden scx_bpf_cpu_rq() Christian Loehle
@ 2025-09-01 13:26 ` Christian Loehle
2025-09-01 13:38 ` Christian Loehle
2025-09-01 13:26 ` [PATCH v5 1/3] sched_ext: Introduce scx_bpf_cpu_rq_locked() Christian Loehle
` (3 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Christian Loehle @ 2025-09-01 13:26 UTC (permalink / raw)
To: tj, arighi, void
Cc: linux-kernel, sched-ext, changwoo, hodgesd, mingo, peterz, jake,
Christian Loehle
scx_bpf_cpu_rq() currently allows accessing struct rq fields without
holding the associated rq.
It is being used by scx_cosmos, scx_flash, scx_lavd, scx_layered, and
scx_tickless. Fortunately it is only ever used to fetch rq->curr.
So provide an alternative scx_bpf_remote_curr() that doesn't expose struct rq
and provide a hardened scx_bpf_cpu_rq_locked() by ensuring we hold the rq lock.
Add a deprecation warning to scx_bpf_cpu_rq() that mentions the two alternatives.
This also simplifies scx code from:
rq = scx_bpf_cpu_rq(cpu);
if (!rq)
return;
p = rq->curr
/* ... Do something with p */
into:
p = scx_bpf_remote_curr(cpu);
/* ... Do something with p */
v4:
Remove cpu argument from scx_bpf_cpu_rq_locked() as SCX has a unique
locked_rq_state anyway. (Tejun)
Expose RCU pointer in scx_bpf_remote_curr() (Peter)
v3:
https://lore.kernel.org/lkml/20250805111036.130121-1-christian.loehle@arm.com/
Don't change scx_bpf_cpu_rq() do not break BPF schedulers without the
grace period. Just add the deprecation warning and do the hardening in
the new scx_bpf_cpu_rq_locked(). (Andrea, Tejun, Jake)
v2:
https://lore.kernel.org/lkml/20250804112743.711816-1-christian.loehle@arm.com/
- Open-code bpf_task_acquire() to avoid the forward declaration (Andrea)
- Rename scx_bpf_task_acquire_remote_curr() to make it more explicit it
behaves like bpf_task_acquire()
- Dis
v1:
https://lore.kernel.org/lkml/20250801141741.355059-1-christian.loehle@arm.com/
- scx_bpf_cpu_rq() now errors when a not locked rq is requested. (Andrea)
- scx_bpf_remote_curr() calls bpf_task_acquire() which BPF user needs to
release. (Andrea)
Christian Loehle (3):
sched_ext: Introduce scx_bpf_cpu_rq_locked()
sched_ext: Introduce scx_bpf_remote_curr()
sched_ext: deprecation warn for scx_bpf_cpu_rq()
kernel/sched/ext.c | 40 ++++++++++++++++++++++++
tools/sched_ext/include/scx/common.bpf.h | 2 ++
2 files changed, 42 insertions(+)
--
2.34.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v5 0/3] sched_ext: Harden scx_bpf_cpu_rq()
2025-09-01 13:26 ` [PATCH v5 " Christian Loehle
@ 2025-09-01 13:38 ` Christian Loehle
0 siblings, 0 replies; 12+ messages in thread
From: Christian Loehle @ 2025-09-01 13:38 UTC (permalink / raw)
To: tj, arighi, void
Cc: linux-kernel, sched-ext, changwoo, hodgesd, mingo, peterz, jake
On 9/1/25 14:26, Christian Loehle wrote:
> scx_bpf_cpu_rq() currently allows accessing struct rq fields without
> holding the associated rq.
> It is being used by scx_cosmos, scx_flash, scx_lavd, scx_layered, and
> scx_tickless. Fortunately it is only ever used to fetch rq->curr.
> So provide an alternative scx_bpf_remote_curr() that doesn't expose struct rq
> and provide a hardened scx_bpf_cpu_rq_locked() by ensuring we hold the rq lock.
> Add a deprecation warning to scx_bpf_cpu_rq() that mentions the two alternatives.
>
> This also simplifies scx code from:
>
> rq = scx_bpf_cpu_rq(cpu);
> if (!rq)
> return;
> p = rq->curr
> /* ... Do something with p */
>
> into:
>
> p = scx_bpf_remote_curr(cpu);
> /* ... Do something with p */
>
> v4:
> Remove cpu argument from scx_bpf_cpu_rq_locked() as SCX has a unique
> locked_rq_state anyway. (Tejun)
> Expose RCU pointer in scx_bpf_remote_curr() (Peter)
> v3:
> https://lore.kernel.org/lkml/20250805111036.130121-1-christian.loehle@arm.com/
> Don't change scx_bpf_cpu_rq() do not break BPF schedulers without the
> grace period. Just add the deprecation warning and do the hardening in
> the new scx_bpf_cpu_rq_locked(). (Andrea, Tejun, Jake)
> v2:
> https://lore.kernel.org/lkml/20250804112743.711816-1-christian.loehle@arm.com/
> - Open-code bpf_task_acquire() to avoid the forward declaration (Andrea)
> - Rename scx_bpf_task_acquire_remote_curr() to make it more explicit it
> behaves like bpf_task_acquire()
> - Dis
> v1:
> https://lore.kernel.org/lkml/20250801141741.355059-1-christian.loehle@arm.com/
> - scx_bpf_cpu_rq() now errors when a not locked rq is requested. (Andrea)
> - scx_bpf_remote_curr() calls bpf_task_acquire() which BPF user needs to
> release. (Andrea)
>
> Christian Loehle (3):
> sched_ext: Introduce scx_bpf_cpu_rq_locked()
> sched_ext: Introduce scx_bpf_remote_curr()
> sched_ext: deprecation warn for scx_bpf_cpu_rq()
>
> kernel/sched/ext.c | 40 ++++++++++++++++++++++++
> tools/sched_ext/include/scx/common.bpf.h | 2 ++
> 2 files changed, 42 insertions(+)
>
> --
> 2.34.1
>
Messed up my git-send-mail here :/
Anyway either one of those v5 cover letters is the correct one.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v5 1/3] sched_ext: Introduce scx_bpf_cpu_rq_locked()
2025-09-01 13:26 [PATCH v4 0/3] sched_ext: Harden scx_bpf_cpu_rq() Christian Loehle
2025-09-01 13:26 ` [PATCH v5 " Christian Loehle
@ 2025-09-01 13:26 ` Christian Loehle
2025-09-03 15:54 ` Tejun Heo
2025-09-01 13:26 ` [PATCH v5 2/3] sched_ext: Introduce scx_bpf_remote_curr() Christian Loehle
` (2 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Christian Loehle @ 2025-09-01 13:26 UTC (permalink / raw)
To: tj, arighi, void
Cc: linux-kernel, sched-ext, changwoo, hodgesd, mingo, peterz, jake,
Christian Loehle
Most fields in scx_bpf_cpu_rq() assume that its rq_lock is held.
Furthermore they become meaningless without rq lock, too.
Make a safer version of scx_bpf_cpu_rq() that only returns a rq
if we hold rq lock of that rq.
Also mark the new scx_bpf_cpu_rq_locked() as returning NULL.
Signed-off-by: Christian Loehle <christian.loehle@arm.com>
---
kernel/sched/ext.c | 23 +++++++++++++++++++++++
tools/sched_ext/include/scx/common.bpf.h | 1 +
2 files changed, 24 insertions(+)
diff --git a/kernel/sched/ext.c b/kernel/sched/ext.c
index 4ae32ef179dd..9fcc310d85d5 100644
--- a/kernel/sched/ext.c
+++ b/kernel/sched/ext.c
@@ -7430,6 +7430,28 @@ __bpf_kfunc struct rq *scx_bpf_cpu_rq(s32 cpu)
return cpu_rq(cpu);
}
+/**
+ * scx_bpf_cpu_rq_locked - Return the rq currently locked by SCX
+ *
+ * Returns the rq if a rq lock is currently held by SCX.
+ * Otherwise emits an error and returns NULL.
+ */
+__bpf_kfunc struct rq *scx_bpf_cpu_rq_locked(void)
+{
+ struct rq *rq;
+
+ preempt_disable();
+ rq = scx_locked_rq();
+ if (!rq) {
+ preempt_enable();
+ scx_kf_error("accessing rq without holding rq lock");
+ return NULL;
+ }
+ preempt_enable();
+
+ return rq;
+}
+
/**
* scx_bpf_task_cgroup - Return the sched cgroup of a task
* @p: task of interest
@@ -7594,6 +7616,7 @@ BTF_ID_FLAGS(func, scx_bpf_put_cpumask, KF_RELEASE)
BTF_ID_FLAGS(func, scx_bpf_task_running, KF_RCU)
BTF_ID_FLAGS(func, scx_bpf_task_cpu, KF_RCU)
BTF_ID_FLAGS(func, scx_bpf_cpu_rq)
+BTF_ID_FLAGS(func, scx_bpf_cpu_rq_locked, KF_RET_NULL)
#ifdef CONFIG_CGROUP_SCHED
BTF_ID_FLAGS(func, scx_bpf_task_cgroup, KF_RCU | KF_ACQUIRE)
#endif
diff --git a/tools/sched_ext/include/scx/common.bpf.h b/tools/sched_ext/include/scx/common.bpf.h
index d4e21558e982..f5be06c93359 100644
--- a/tools/sched_ext/include/scx/common.bpf.h
+++ b/tools/sched_ext/include/scx/common.bpf.h
@@ -91,6 +91,7 @@ s32 scx_bpf_pick_any_cpu(const cpumask_t *cpus_allowed, u64 flags) __ksym;
bool scx_bpf_task_running(const struct task_struct *p) __ksym;
s32 scx_bpf_task_cpu(const struct task_struct *p) __ksym;
struct rq *scx_bpf_cpu_rq(s32 cpu) __ksym;
+struct rq *scx_bpf_cpu_rq_locked(void) __ksym;
struct cgroup *scx_bpf_task_cgroup(struct task_struct *p) __ksym __weak;
u64 scx_bpf_now(void) __ksym __weak;
void scx_bpf_events(struct scx_event_stats *events, size_t events__sz) __ksym __weak;
--
2.34.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v5 1/3] sched_ext: Introduce scx_bpf_cpu_rq_locked()
2025-09-01 13:26 ` [PATCH v5 1/3] sched_ext: Introduce scx_bpf_cpu_rq_locked() Christian Loehle
@ 2025-09-03 15:54 ` Tejun Heo
0 siblings, 0 replies; 12+ messages in thread
From: Tejun Heo @ 2025-09-03 15:54 UTC (permalink / raw)
To: Christian Loehle
Cc: arighi, void, linux-kernel, sched-ext, changwoo, hodgesd, mingo,
peterz, jake
On Mon, Sep 01, 2025 at 02:26:02PM +0100, Christian Loehle wrote:
> Most fields in scx_bpf_cpu_rq() assume that its rq_lock is held.
> Furthermore they become meaningless without rq lock, too.
> Make a safer version of scx_bpf_cpu_rq() that only returns a rq
> if we hold rq lock of that rq.
>
> Also mark the new scx_bpf_cpu_rq_locked() as returning NULL.
>
> Signed-off-by: Christian Loehle <christian.loehle@arm.com>
> ---
> kernel/sched/ext.c | 23 +++++++++++++++++++++++
> tools/sched_ext/include/scx/common.bpf.h | 1 +
> 2 files changed, 24 insertions(+)
>
> diff --git a/kernel/sched/ext.c b/kernel/sched/ext.c
> index 4ae32ef179dd..9fcc310d85d5 100644
> --- a/kernel/sched/ext.c
> +++ b/kernel/sched/ext.c
> @@ -7430,6 +7430,28 @@ __bpf_kfunc struct rq *scx_bpf_cpu_rq(s32 cpu)
> return cpu_rq(cpu);
> }
>
> +/**
> + * scx_bpf_cpu_rq_locked - Return the rq currently locked by SCX
> + *
> + * Returns the rq if a rq lock is currently held by SCX.
> + * Otherwise emits an error and returns NULL.
> + */
> +__bpf_kfunc struct rq *scx_bpf_cpu_rq_locked(void)
How about naming it scx_bpf_locked_rq()? That reads a lot easier to me and
given that it doesn't take @cpu anymore, the _cpu_ part of the name isn't
necessary.
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v5 2/3] sched_ext: Introduce scx_bpf_remote_curr()
2025-09-01 13:26 [PATCH v4 0/3] sched_ext: Harden scx_bpf_cpu_rq() Christian Loehle
2025-09-01 13:26 ` [PATCH v5 " Christian Loehle
2025-09-01 13:26 ` [PATCH v5 1/3] sched_ext: Introduce scx_bpf_cpu_rq_locked() Christian Loehle
@ 2025-09-01 13:26 ` Christian Loehle
2025-09-02 5:43 ` Andrea Righi
2025-09-03 15:55 ` Tejun Heo
2025-09-01 13:26 ` [PATCH v5 3/3] sched_ext: deprecation warn for scx_bpf_cpu_rq() Christian Loehle
2025-09-01 13:26 ` [PATCH v5 0/3] sched_ext: Harden scx_bpf_cpu_rq() Christian Loehle
4 siblings, 2 replies; 12+ messages in thread
From: Christian Loehle @ 2025-09-01 13:26 UTC (permalink / raw)
To: tj, arighi, void
Cc: linux-kernel, sched-ext, changwoo, hodgesd, mingo, peterz, jake,
Christian Loehle
Provide scx_bpf_remote_curr() as a way for scx schedulers to check the curr
task of a remote rq without assuming its lock is held.
Many scx schedulers make use of scx_bpf_cpu_rq() to check a remote curr
(e.g. to see if it should be preempted). This is problematic because
scx_bpf_cpu_rq() provides access to all fields of struct rq, most of
which aren't safe to use without holding the associated rq lock.
Signed-off-by: Christian Loehle <christian.loehle@arm.com>
---
kernel/sched/ext.c | 14 ++++++++++++++
tools/sched_ext/include/scx/common.bpf.h | 1 +
2 files changed, 15 insertions(+)
diff --git a/kernel/sched/ext.c b/kernel/sched/ext.c
index 9fcc310d85d5..e242a2520f06 100644
--- a/kernel/sched/ext.c
+++ b/kernel/sched/ext.c
@@ -7452,6 +7452,19 @@ __bpf_kfunc struct rq *scx_bpf_cpu_rq_locked(void)
return rq;
}
+/**
+ * scx_bpf_remote_curr - Return remote CPU's curr task
+ * @cpu: CPU of interest
+ *
+ * Callers must hold RCU read lock (KF_RCU).
+ */
+__bpf_kfunc struct task_struct *scx_bpf_remote_curr(s32 cpu)
+{
+ if (!kf_cpu_valid(cpu, NULL))
+ return NULL;
+ return READ_ONCE(cpu_rq(cpu)->curr);
+}
+
/**
* scx_bpf_task_cgroup - Return the sched cgroup of a task
* @p: task of interest
@@ -7617,6 +7630,7 @@ BTF_ID_FLAGS(func, scx_bpf_task_running, KF_RCU)
BTF_ID_FLAGS(func, scx_bpf_task_cpu, KF_RCU)
BTF_ID_FLAGS(func, scx_bpf_cpu_rq)
BTF_ID_FLAGS(func, scx_bpf_cpu_rq_locked, KF_RET_NULL)
+BTF_ID_FLAGS(func, scx_bpf_remote_curr, KF_RET_NULL | KF_RCU)
#ifdef CONFIG_CGROUP_SCHED
BTF_ID_FLAGS(func, scx_bpf_task_cgroup, KF_RCU | KF_ACQUIRE)
#endif
diff --git a/tools/sched_ext/include/scx/common.bpf.h b/tools/sched_ext/include/scx/common.bpf.h
index f5be06c93359..dd3d94256c10 100644
--- a/tools/sched_ext/include/scx/common.bpf.h
+++ b/tools/sched_ext/include/scx/common.bpf.h
@@ -92,6 +92,7 @@ bool scx_bpf_task_running(const struct task_struct *p) __ksym;
s32 scx_bpf_task_cpu(const struct task_struct *p) __ksym;
struct rq *scx_bpf_cpu_rq(s32 cpu) __ksym;
struct rq *scx_bpf_cpu_rq_locked(void) __ksym;
+struct task_struct *scx_bpf_remote_curr(s32 cpu) __ksym;
struct cgroup *scx_bpf_task_cgroup(struct task_struct *p) __ksym __weak;
u64 scx_bpf_now(void) __ksym __weak;
void scx_bpf_events(struct scx_event_stats *events, size_t events__sz) __ksym __weak;
--
2.34.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v5 2/3] sched_ext: Introduce scx_bpf_remote_curr()
2025-09-01 13:26 ` [PATCH v5 2/3] sched_ext: Introduce scx_bpf_remote_curr() Christian Loehle
@ 2025-09-02 5:43 ` Andrea Righi
2025-09-03 15:55 ` Tejun Heo
1 sibling, 0 replies; 12+ messages in thread
From: Andrea Righi @ 2025-09-02 5:43 UTC (permalink / raw)
To: Christian Loehle
Cc: tj, void, linux-kernel, sched-ext, changwoo, hodgesd, mingo,
peterz, jake
Hi Christian,
On Mon, Sep 01, 2025 at 02:26:03PM +0100, Christian Loehle wrote:
> Provide scx_bpf_remote_curr() as a way for scx schedulers to check the curr
> task of a remote rq without assuming its lock is held.
>
> Many scx schedulers make use of scx_bpf_cpu_rq() to check a remote curr
> (e.g. to see if it should be preempted). This is problematic because
> scx_bpf_cpu_rq() provides access to all fields of struct rq, most of
> which aren't safe to use without holding the associated rq lock.
>
> Signed-off-by: Christian Loehle <christian.loehle@arm.com>
> ---
> kernel/sched/ext.c | 14 ++++++++++++++
> tools/sched_ext/include/scx/common.bpf.h | 1 +
> 2 files changed, 15 insertions(+)
>
> diff --git a/kernel/sched/ext.c b/kernel/sched/ext.c
> index 9fcc310d85d5..e242a2520f06 100644
> --- a/kernel/sched/ext.c
> +++ b/kernel/sched/ext.c
> @@ -7452,6 +7452,19 @@ __bpf_kfunc struct rq *scx_bpf_cpu_rq_locked(void)
> return rq;
> }
>
> +/**
> + * scx_bpf_remote_curr - Return remote CPU's curr task
> + * @cpu: CPU of interest
> + *
> + * Callers must hold RCU read lock (KF_RCU).
> + */
> +__bpf_kfunc struct task_struct *scx_bpf_remote_curr(s32 cpu)
> +{
> + if (!kf_cpu_valid(cpu, NULL))
> + return NULL;
> + return READ_ONCE(cpu_rq(cpu)->curr);
It shouldn't be rcu_dereference(cpu_rq(cpu)->curr)?
Thanks,
-Andrea
> +}
> +
> /**
> * scx_bpf_task_cgroup - Return the sched cgroup of a task
> * @p: task of interest
> @@ -7617,6 +7630,7 @@ BTF_ID_FLAGS(func, scx_bpf_task_running, KF_RCU)
> BTF_ID_FLAGS(func, scx_bpf_task_cpu, KF_RCU)
> BTF_ID_FLAGS(func, scx_bpf_cpu_rq)
> BTF_ID_FLAGS(func, scx_bpf_cpu_rq_locked, KF_RET_NULL)
> +BTF_ID_FLAGS(func, scx_bpf_remote_curr, KF_RET_NULL | KF_RCU)
> #ifdef CONFIG_CGROUP_SCHED
> BTF_ID_FLAGS(func, scx_bpf_task_cgroup, KF_RCU | KF_ACQUIRE)
> #endif
> diff --git a/tools/sched_ext/include/scx/common.bpf.h b/tools/sched_ext/include/scx/common.bpf.h
> index f5be06c93359..dd3d94256c10 100644
> --- a/tools/sched_ext/include/scx/common.bpf.h
> +++ b/tools/sched_ext/include/scx/common.bpf.h
> @@ -92,6 +92,7 @@ bool scx_bpf_task_running(const struct task_struct *p) __ksym;
> s32 scx_bpf_task_cpu(const struct task_struct *p) __ksym;
> struct rq *scx_bpf_cpu_rq(s32 cpu) __ksym;
> struct rq *scx_bpf_cpu_rq_locked(void) __ksym;
> +struct task_struct *scx_bpf_remote_curr(s32 cpu) __ksym;
> struct cgroup *scx_bpf_task_cgroup(struct task_struct *p) __ksym __weak;
> u64 scx_bpf_now(void) __ksym __weak;
> void scx_bpf_events(struct scx_event_stats *events, size_t events__sz) __ksym __weak;
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v5 2/3] sched_ext: Introduce scx_bpf_remote_curr()
2025-09-01 13:26 ` [PATCH v5 2/3] sched_ext: Introduce scx_bpf_remote_curr() Christian Loehle
2025-09-02 5:43 ` Andrea Righi
@ 2025-09-03 15:55 ` Tejun Heo
1 sibling, 0 replies; 12+ messages in thread
From: Tejun Heo @ 2025-09-03 15:55 UTC (permalink / raw)
To: Christian Loehle
Cc: arighi, void, linux-kernel, sched-ext, changwoo, hodgesd, mingo,
peterz, jake
Hello,
On Mon, Sep 01, 2025 at 02:26:03PM +0100, Christian Loehle wrote:
> +/**
> + * scx_bpf_remote_curr - Return remote CPU's curr task
> + * @cpu: CPU of interest
> + *
> + * Callers must hold RCU read lock (KF_RCU).
> + */
> +__bpf_kfunc struct task_struct *scx_bpf_remote_curr(s32 cpu)
And name this scx_bpf_cpu_curr().
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v5 3/3] sched_ext: deprecation warn for scx_bpf_cpu_rq()
2025-09-01 13:26 [PATCH v4 0/3] sched_ext: Harden scx_bpf_cpu_rq() Christian Loehle
` (2 preceding siblings ...)
2025-09-01 13:26 ` [PATCH v5 2/3] sched_ext: Introduce scx_bpf_remote_curr() Christian Loehle
@ 2025-09-01 13:26 ` Christian Loehle
2025-09-03 15:56 ` Tejun Heo
2025-09-01 13:26 ` [PATCH v5 0/3] sched_ext: Harden scx_bpf_cpu_rq() Christian Loehle
4 siblings, 1 reply; 12+ messages in thread
From: Christian Loehle @ 2025-09-01 13:26 UTC (permalink / raw)
To: tj, arighi, void
Cc: linux-kernel, sched-ext, changwoo, hodgesd, mingo, peterz, jake,
Christian Loehle
scx_bpf_cpu_rq() works on an unlocked rq which generally isn't safe.
For the common use-cases scx_bpf_cpu_rq_locked() and
scx_bpf_remote_curr() work, so add a deprecation warning
to scx_bpf_cpu_rq() so it can eventually be removed.
Signed-off-by: Christian Loehle <christian.loehle@arm.com>
---
kernel/sched/ext.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/kernel/sched/ext.c b/kernel/sched/ext.c
index e242a2520f06..a524d404ad09 100644
--- a/kernel/sched/ext.c
+++ b/kernel/sched/ext.c
@@ -7427,6 +7427,9 @@ __bpf_kfunc struct rq *scx_bpf_cpu_rq(s32 cpu)
if (!kf_cpu_valid(cpu, NULL))
return NULL;
+ pr_warn_once("%s() is deprecated; use scx_bpf_cpu_rq_locked() when holding rq lock "
+ "or scx_bpf_remote_curr() to read remote curr safely.\n", __func__);
+
return cpu_rq(cpu);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v5 3/3] sched_ext: deprecation warn for scx_bpf_cpu_rq()
2025-09-01 13:26 ` [PATCH v5 3/3] sched_ext: deprecation warn for scx_bpf_cpu_rq() Christian Loehle
@ 2025-09-03 15:56 ` Tejun Heo
2025-09-03 20:37 ` Christian Loehle
0 siblings, 1 reply; 12+ messages in thread
From: Tejun Heo @ 2025-09-03 15:56 UTC (permalink / raw)
To: Christian Loehle
Cc: arighi, void, linux-kernel, sched-ext, changwoo, hodgesd, mingo,
peterz, jake
On Mon, Sep 01, 2025 at 02:26:04PM +0100, Christian Loehle wrote:
> scx_bpf_cpu_rq() works on an unlocked rq which generally isn't safe.
> For the common use-cases scx_bpf_cpu_rq_locked() and
> scx_bpf_remote_curr() work, so add a deprecation warning
> to scx_bpf_cpu_rq() so it can eventually be removed.
>
> Signed-off-by: Christian Loehle <christian.loehle@arm.com>
> ---
> kernel/sched/ext.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/kernel/sched/ext.c b/kernel/sched/ext.c
> index e242a2520f06..a524d404ad09 100644
> --- a/kernel/sched/ext.c
> +++ b/kernel/sched/ext.c
> @@ -7427,6 +7427,9 @@ __bpf_kfunc struct rq *scx_bpf_cpu_rq(s32 cpu)
> if (!kf_cpu_valid(cpu, NULL))
> return NULL;
>
> + pr_warn_once("%s() is deprecated; use scx_bpf_cpu_rq_locked() when holding rq lock "
> + "or scx_bpf_remote_curr() to read remote curr safely.\n", __func__);
> +
I wonder whether it'd make more sense to tie the once testing to the
scheduler instance (see warned_zero_slice).
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v5 3/3] sched_ext: deprecation warn for scx_bpf_cpu_rq()
2025-09-03 15:56 ` Tejun Heo
@ 2025-09-03 20:37 ` Christian Loehle
0 siblings, 0 replies; 12+ messages in thread
From: Christian Loehle @ 2025-09-03 20:37 UTC (permalink / raw)
To: Tejun Heo
Cc: arighi, void, linux-kernel, sched-ext, changwoo, hodgesd, mingo,
peterz, jake
On 9/3/25 16:56, Tejun Heo wrote:
> On Mon, Sep 01, 2025 at 02:26:04PM +0100, Christian Loehle wrote:
>> scx_bpf_cpu_rq() works on an unlocked rq which generally isn't safe.
>> For the common use-cases scx_bpf_cpu_rq_locked() and
>> scx_bpf_remote_curr() work, so add a deprecation warning
>> to scx_bpf_cpu_rq() so it can eventually be removed.
>>
>> Signed-off-by: Christian Loehle <christian.loehle@arm.com>
>> ---
>> kernel/sched/ext.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/kernel/sched/ext.c b/kernel/sched/ext.c
>> index e242a2520f06..a524d404ad09 100644
>> --- a/kernel/sched/ext.c
>> +++ b/kernel/sched/ext.c
>> @@ -7427,6 +7427,9 @@ __bpf_kfunc struct rq *scx_bpf_cpu_rq(s32 cpu)
>> if (!kf_cpu_valid(cpu, NULL))
>> return NULL;
>>
>> + pr_warn_once("%s() is deprecated; use scx_bpf_cpu_rq_locked() when holding rq lock "
>> + "or scx_bpf_remote_curr() to read remote curr safely.\n", __func__);
>> +
>
> I wonder whether it'd make more sense to tie the once testing to the
> scheduler instance (see warned_zero_slice).
>
Yes that makes sense to me, I'll resend with this and the renaming suggestions.
Thanks!
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v5 0/3] sched_ext: Harden scx_bpf_cpu_rq()
2025-09-01 13:26 [PATCH v4 0/3] sched_ext: Harden scx_bpf_cpu_rq() Christian Loehle
` (3 preceding siblings ...)
2025-09-01 13:26 ` [PATCH v5 3/3] sched_ext: deprecation warn for scx_bpf_cpu_rq() Christian Loehle
@ 2025-09-01 13:26 ` Christian Loehle
4 siblings, 0 replies; 12+ messages in thread
From: Christian Loehle @ 2025-09-01 13:26 UTC (permalink / raw)
To: tj, arighi, void
Cc: linux-kernel, sched-ext, changwoo, hodgesd, mingo, peterz, jake,
Christian Loehle
scx_bpf_cpu_rq() currently allows accessing struct rq fields without
holding the associated rq.
It is being used by scx_cosmos, scx_flash, scx_lavd, scx_layered, and
scx_tickless. Fortunately it is only ever used to fetch rq->curr.
So provide an alternative scx_bpf_remote_curr() that doesn't expose struct rq
and provide a hardened scx_bpf_cpu_rq_locked() by ensuring we hold the rq lock.
Add a deprecation warning to scx_bpf_cpu_rq() that mentions the two alternatives.
This also simplifies scx code from:
rq = scx_bpf_cpu_rq(cpu);
if (!rq)
return;
p = rq->curr
/* ... Do something with p */
into:
p = scx_bpf_remote_curr(cpu);
/* ... Do something with p */
v4:
Remove cpu argument from scx_bpf_cpu_rq_locked() as SCX has a unique
locked_rq_state anyway. (Tejun)
Expose RCU pointer in scx_bpf_remote_curr() (Peter)
v3:
https://lore.kernel.org/lkml/20250805111036.130121-1-christian.loehle@arm.com/
Don't change scx_bpf_cpu_rq() do not break BPF schedulers without the
grace period. Just add the deprecation warning and do the hardening in
the new scx_bpf_cpu_rq_locked(). (Andrea, Tejun, Jake)
v2:
https://lore.kernel.org/lkml/20250804112743.711816-1-christian.loehle@arm.com/
- Open-code bpf_task_acquire() to avoid the forward declaration (Andrea)
- Rename scx_bpf_task_acquire_remote_curr() to make it more explicit it
behaves like bpf_task_acquire()
- Dis
v1:
https://lore.kernel.org/lkml/20250801141741.355059-1-christian.loehle@arm.com/
- scx_bpf_cpu_rq() now errors when a not locked rq is requested. (Andrea)
- scx_bpf_remote_curr() calls bpf_task_acquire() which BPF user needs to
release. (Andrea)
Christian Loehle (3):
sched_ext: Introduce scx_bpf_cpu_rq_locked()
sched_ext: Introduce scx_bpf_remote_curr()
sched_ext: deprecation warn for scx_bpf_cpu_rq()
kernel/sched/ext.c | 40 ++++++++++++++++++++++++
tools/sched_ext/include/scx/common.bpf.h | 2 ++
2 files changed, 42 insertions(+)
--
2.34.1
^ permalink raw reply [flat|nested] 12+ messages in thread