linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/3] sched_ext: Harden scx_bpf_cpu_rq()
@ 2025-09-01 13:26 Christian Loehle
  2025-09-01 13:26 ` [PATCH v5 " Christian Loehle
                   ` (4 more replies)
  0 siblings, 5 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_task_acquire_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_locked() that mentions the
two alternatives.

This also simplifies scx code from:

rq = scx_bpf_cpu_rq(cpu);
if (!rq)
	return;
p = rq->curr
if (!p)
	return;
/* ... Do something with p */

into:

p = scx_bpf_task_acquire_remote_curr(cpu);
if (!p)
	return;
/* ... Do something with p */
bpf_task_release(p);


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: Provide scx_bpf_task_acquire_remote_curr()
  sched_ext: deprecation warn for scx_bpf_cpu_rq()

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

-- 
2.34.1


^ 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
@ 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

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

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

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

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

* 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

* 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 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

* 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

* 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

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

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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:38   ` Christian Loehle
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
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
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
2025-09-01 13:26 ` [PATCH v5 0/3] sched_ext: Harden scx_bpf_cpu_rq() Christian Loehle

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).