The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH v7 0/3] sched_ext: Harden scx_bpf_cpu_rq()
@ 2025-09-03 21:23 Christian Loehle
  2025-09-03 21:23 ` [PATCH v7 1/3] sched_ext: Introduce scx_bpf_locked_rq() Christian Loehle
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Christian Loehle @ 2025-09-03 21:23 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_cpu_curr() that doesn't expose struct rq
and provide a hardened scx_bpf_locked_rq() 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_cpu_curr(cpu);
/* ... Do something with p */

Changes since:
v6:
https://lore.kernel.org/lkml/20250902111143.2667154-1-christian.loehle@arm.com/
- Rename: scx_bpf_cpu_rq_locked() -> scx_bpf_locked_rq() and
scx_bpf_remote_curr() -> scx_bpf_cpu_curr() (Tejun)
- Print the deprecation warning of scx_bpf_cpu_rq() once per scheduler. (Tejun)
- Picked up Andrea's ACKs (except for 3/3 because of the logic change).
v5:
https://lore.kernel.org/lkml/20250901132605.2282650-2-christian.loehle@arm.com/
- Actually expose the RCU pointer in scx_bpf_remote_curr() as such (Andrea)
v4:
https://lore.kernel.org/lkml/20250811212150.85759-1-christian.loehle@arm.com/
- 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()
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_locked_rq()
  sched_ext: Introduce scx_bpf_cpu_curr()
  sched_ext: deprecation warn for scx_bpf_cpu_rq()

 kernel/sched/ext.c                       | 47 ++++++++++++++++++++++++
 tools/sched_ext/include/scx/common.bpf.h |  2 +
 2 files changed, 49 insertions(+)

--
2.34.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v7 1/3] sched_ext: Introduce scx_bpf_locked_rq()
  2025-09-03 21:23 [PATCH v7 0/3] sched_ext: Harden scx_bpf_cpu_rq() Christian Loehle
@ 2025-09-03 21:23 ` Christian Loehle
  2025-09-03 21:23 ` [PATCH v7 2/3] sched_ext: Introduce scx_bpf_cpu_curr() Christian Loehle
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Christian Loehle @ 2025-09-03 21:23 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_locked_rq() as returning NULL as
scx_bpf_cpu_rq() should've been too.

Signed-off-by: Christian Loehle <christian.loehle@arm.com>
Acked-by: Andrea Righi <arighi@nvidia.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..e15f1b0501a1 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_locked_rq - 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_locked_rq(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_locked_rq, 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..e9b006226cf6 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_locked_rq(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] 5+ messages in thread

* [PATCH v7 2/3] sched_ext: Introduce scx_bpf_cpu_curr()
  2025-09-03 21:23 [PATCH v7 0/3] sched_ext: Harden scx_bpf_cpu_rq() Christian Loehle
  2025-09-03 21:23 ` [PATCH v7 1/3] sched_ext: Introduce scx_bpf_locked_rq() Christian Loehle
@ 2025-09-03 21:23 ` Christian Loehle
  2025-09-03 21:23 ` [PATCH v7 3/3] sched_ext: deprecation warn for scx_bpf_cpu_rq() Christian Loehle
  2025-09-03 21:52 ` [PATCH v7 0/3] sched_ext: Harden scx_bpf_cpu_rq() Tejun Heo
  3 siblings, 0 replies; 5+ messages in thread
From: Christian Loehle @ 2025-09-03 21:23 UTC (permalink / raw)
  To: tj, arighi, void
  Cc: linux-kernel, sched-ext, changwoo, hodgesd, mingo, peterz, jake,
	Christian Loehle

Provide scx_bpf_cpu_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>
Acked-by: Andrea Righi <arighi@nvidia.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 e15f1b0501a1..6ea81b6a6b2d 100644
--- a/kernel/sched/ext.c
+++ b/kernel/sched/ext.c
@@ -7452,6 +7452,19 @@ __bpf_kfunc struct rq *scx_bpf_locked_rq(void)
 	return rq;
 }

+/**
+ * scx_bpf_cpu_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_cpu_curr(s32 cpu)
+{
+	if (!kf_cpu_valid(cpu, NULL))
+		return NULL;
+	return rcu_dereference(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_locked_rq, KF_RET_NULL)
+BTF_ID_FLAGS(func, scx_bpf_cpu_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 e9b006226cf6..03e614506fa3 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_locked_rq(void) __ksym;
+struct task_struct *scx_bpf_cpu_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] 5+ messages in thread

* [PATCH v7 3/3] sched_ext: deprecation warn for scx_bpf_cpu_rq()
  2025-09-03 21:23 [PATCH v7 0/3] sched_ext: Harden scx_bpf_cpu_rq() Christian Loehle
  2025-09-03 21:23 ` [PATCH v7 1/3] sched_ext: Introduce scx_bpf_locked_rq() Christian Loehle
  2025-09-03 21:23 ` [PATCH v7 2/3] sched_ext: Introduce scx_bpf_cpu_curr() Christian Loehle
@ 2025-09-03 21:23 ` Christian Loehle
  2025-09-03 21:52 ` [PATCH v7 0/3] sched_ext: Harden scx_bpf_cpu_rq() Tejun Heo
  3 siblings, 0 replies; 5+ messages in thread
From: Christian Loehle @ 2025-09-03 21:23 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_locked_rq() and
scx_bpf_cpu_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 | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/kernel/sched/ext.c b/kernel/sched/ext.c
index 6ea81b6a6b2d..1a0afdf5fcf5 100644
--- a/kernel/sched/ext.c
+++ b/kernel/sched/ext.c
@@ -873,6 +873,7 @@ struct scx_sched {
 	struct scx_event_stats __percpu *event_stats_cpu;
 
 	bool			warned_zero_slice;
+	bool			warned_deprecated_rq;
 
 	atomic_t		exit_kind;
 	struct scx_exit_info	*exit_info;
@@ -7424,9 +7425,18 @@ __bpf_kfunc s32 scx_bpf_task_cpu(const struct task_struct *p)
  */
 __bpf_kfunc struct rq *scx_bpf_cpu_rq(s32 cpu)
 {
+	struct scx_sched *sch = scx_root;
+
 	if (!kf_cpu_valid(cpu, NULL))
 		return NULL;
 
+	if (!sch->warned_deprecated_rq) {
+		printk_deferred(KERN_WARNING "sched_ext: %s() is deprecated; "
+				"use scx_bpf_locked_rq() when holding rq lock "
+				"or scx_bpf_cpu_curr() to read remote curr safely.\n", __func__);
+		sch->warned_deprecated_rq = true;
+	}
+
 	return cpu_rq(cpu);
 }
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v7 0/3] sched_ext: Harden scx_bpf_cpu_rq()
  2025-09-03 21:23 [PATCH v7 0/3] sched_ext: Harden scx_bpf_cpu_rq() Christian Loehle
                   ` (2 preceding siblings ...)
  2025-09-03 21:23 ` [PATCH v7 3/3] sched_ext: deprecation warn for scx_bpf_cpu_rq() Christian Loehle
@ 2025-09-03 21:52 ` Tejun Heo
  3 siblings, 0 replies; 5+ messages in thread
From: Tejun Heo @ 2025-09-03 21:52 UTC (permalink / raw)
  To: Christian Loehle
  Cc: arighi, void, linux-kernel, sched-ext, changwoo, hodgesd, mingo,
	peterz, jake

On Wed, Sep 03, 2025 at 10:23:08PM +0100, 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_cpu_curr() that doesn't expose struct rq
> and provide a hardened scx_bpf_locked_rq() 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_cpu_curr(cpu);
> /* ... Do something with p */

Applied 1-3 to sched_ext/for-6.18 (the last patch needed a bit of update to
account for struct scx_sched defintion being moved to ext_internal.h).

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-09-03 21:52 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-03 21:23 [PATCH v7 0/3] sched_ext: Harden scx_bpf_cpu_rq() Christian Loehle
2025-09-03 21:23 ` [PATCH v7 1/3] sched_ext: Introduce scx_bpf_locked_rq() Christian Loehle
2025-09-03 21:23 ` [PATCH v7 2/3] sched_ext: Introduce scx_bpf_cpu_curr() Christian Loehle
2025-09-03 21:23 ` [PATCH v7 3/3] sched_ext: deprecation warn for scx_bpf_cpu_rq() Christian Loehle
2025-09-03 21:52 ` [PATCH v7 0/3] sched_ext: Harden scx_bpf_cpu_rq() Tejun Heo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox