public inbox for sched-ext@lists.linux.dev
 help / color / mirror / Atom feed
* [PATCH] sched_ext: Add scx_bpf_task_add_dsq_vtime()
@ 2026-03-11 15:39 Cheng-Yang Chou
  2026-03-11 15:58 ` Andrea Righi
  0 siblings, 1 reply; 3+ messages in thread
From: Cheng-Yang Chou @ 2026-03-11 15:39 UTC (permalink / raw)
  To: sched-ext; +Cc: tj, void, arighi, changwoo, jserv, yphbchou0911

Writing directly to p->scx.slice and p->scx.dsq_vtime from a BPF
scheduler is deprecated. Currently, if schedulers want to add a
virtual time delta, they are forced to do it directly via
p->scx.dsq_vtime += vtime_delta.

Unlike the existing scx_bpf_task_set_* helpers, this direct
modification bypasses the scx_task_on_sched() authority check,
potentially modifying tasks the calling scheduler does not control.

Introduce scx_bpf_task_add_dsq_vtime() to provide a safe and
approved API for schedulers to easily apply a delta to p->scx.dsq_vtime
while ensuring task authority is properly verified.

Additionally, update the deprecation warning in
bpf_scx_btf_struct_access() to suggest using the new helper.

Signed-off-by: Cheng-Yang Chou <yphbchou0911@gmail.com>
---
 kernel/sched/ext.c                       | 27 +++++++++++++++++++++++-
 tools/sched_ext/include/scx/compat.bpf.h |  9 ++++++++
 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/kernel/sched/ext.c b/kernel/sched/ext.c
index e7ab3647e35f..26a85ec58d31 100644
--- a/kernel/sched/ext.c
+++ b/kernel/sched/ext.c
@@ -6963,7 +6963,7 @@ static int bpf_scx_btf_struct_access(struct bpf_verifier_log *log,
 		     off + size <= offsetofend(struct task_struct, scx.slice)) ||
 		    (off >= offsetof(struct task_struct, scx.dsq_vtime) &&
 		     off + size <= offsetofend(struct task_struct, scx.dsq_vtime))) {
-			pr_warn("sched_ext: Writing directly to p->scx.slice/dsq_vtime is deprecated, use scx_bpf_task_set_slice/dsq_vtime()");
+			pr_warn("sched_ext: Writing directly to p->scx.slice/dsq_vtime is deprecated, use scx_bpf_task_set_slice/dsq_vtime() or scx_bpf_task_add_dsq_vtime()");
 			return SCALAR_VALUE;
 		}
 
@@ -8281,6 +8281,30 @@ __bpf_kfunc bool scx_bpf_task_set_dsq_vtime(struct task_struct *p, u64 vtime,
 	return true;
 }
 
+/**
+ * scx_bpf_task_add_dsq_vtime - Add to task's virtual time for DSQ ordering
+ * @p: task of interest
+ * @vtime_delta: virtual time delta to add
+ * @aux: implicit BPF argument to access bpf_prog_aux hidden from BPF progs
+ *
+ * Add @vtime_delta to @p's virtual time. Returns %true on success, %false if the
+ * calling scheduler doesn't have authority over @p.
+ */
+
+__bpf_kfunc bool scx_bpf_task_add_dsq_vtime(struct task_struct *p, s64 vtime_delta,
+					    const struct bpf_prog_aux *aux)
+{
+	struct scx_sched *sch;
+
+	guard(rcu)();
+	sch = scx_prog_sched(aux);
+	if (unlikely(!scx_task_on_sched(sch, p)))
+		return false;
+
+	p->scx.dsq_vtime += vtime_delta;
+	return true;
+}
+
 static void scx_kick_cpu(struct scx_sched *sch, s32 cpu, u64 flags)
 {
 	struct rq *this_rq;
@@ -9167,6 +9191,7 @@ __bpf_kfunc_end_defs();
 BTF_KFUNCS_START(scx_kfunc_ids_any)
 BTF_ID_FLAGS(func, scx_bpf_task_set_slice, KF_IMPLICIT_ARGS | KF_RCU);
 BTF_ID_FLAGS(func, scx_bpf_task_set_dsq_vtime, KF_IMPLICIT_ARGS | KF_RCU);
+BTF_ID_FLAGS(func, scx_bpf_task_add_dsq_vtime, KF_IMPLICIT_ARGS | KF_RCU);
 BTF_ID_FLAGS(func, scx_bpf_kick_cpu, KF_IMPLICIT_ARGS)
 BTF_ID_FLAGS(func, scx_bpf_dsq_nr_queued)
 BTF_ID_FLAGS(func, scx_bpf_destroy_dsq)
diff --git a/tools/sched_ext/include/scx/compat.bpf.h b/tools/sched_ext/include/scx/compat.bpf.h
index 704728864d83..06f298f6909f 100644
--- a/tools/sched_ext/include/scx/compat.bpf.h
+++ b/tools/sched_ext/include/scx/compat.bpf.h
@@ -343,6 +343,7 @@ scx_bpf_dsq_insert(struct task_struct *p, u64 dsq_id, u64 slice, u64 enq_flags)
  */
 bool scx_bpf_task_set_slice___new(struct task_struct *p, u64 slice) __ksym __weak;
 bool scx_bpf_task_set_dsq_vtime___new(struct task_struct *p, u64 vtime) __ksym __weak;
+bool scx_bpf_task_add_dsq_vtime___new(struct task_struct *p, u64 vtime_delta) __ksym __weak;
 
 static inline void scx_bpf_task_set_slice(struct task_struct *p, u64 slice)
 {
@@ -360,6 +361,14 @@ static inline void scx_bpf_task_set_dsq_vtime(struct task_struct *p, u64 vtime)
 		p->scx.dsq_vtime = vtime;
 }
 
+static inline void scx_bpf_task_add_dsq_vtime(struct task_struct *p, u64 vtime_delta)
+{
+	if (bpf_ksym_exists(scx_bpf_task_add_dsq_vtime___new))
+		scx_bpf_task_add_dsq_vtime___new(p, vtime_delta);
+	else
+		p->scx.dsq_vtime += vtime_delta;
+}
+
 /*
  * v6.19: The new void variant can be called from anywhere while the older v1
  * variant can only be called from ops.cpu_release(). The double ___ prefixes on
-- 
2.48.1


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

* Re: [PATCH] sched_ext: Add scx_bpf_task_add_dsq_vtime()
  2026-03-11 15:39 [PATCH] sched_ext: Add scx_bpf_task_add_dsq_vtime() Cheng-Yang Chou
@ 2026-03-11 15:58 ` Andrea Righi
  2026-03-11 16:12   ` Cheng-Yang Chou
  0 siblings, 1 reply; 3+ messages in thread
From: Andrea Righi @ 2026-03-11 15:58 UTC (permalink / raw)
  To: Cheng-Yang Chou; +Cc: sched-ext, tj, void, changwoo, jserv

On Wed, Mar 11, 2026 at 11:39:57PM +0800, Cheng-Yang Chou wrote:
> Writing directly to p->scx.slice and p->scx.dsq_vtime from a BPF
> scheduler is deprecated. Currently, if schedulers want to add a
> virtual time delta, they are forced to do it directly via
> p->scx.dsq_vtime += vtime_delta.
> 
> Unlike the existing scx_bpf_task_set_* helpers, this direct
> modification bypasses the scx_task_on_sched() authority check,
> potentially modifying tasks the calling scheduler does not control.
> 
> Introduce scx_bpf_task_add_dsq_vtime() to provide a safe and
> approved API for schedulers to easily apply a delta to p->scx.dsq_vtime
> while ensuring task authority is properly verified.
> 
> Additionally, update the deprecation warning in
> bpf_scx_btf_struct_access() to suggest using the new helper.
> 
> Signed-off-by: Cheng-Yang Chou <yphbchou0911@gmail.com>

Hm, I'm not sure we need a dedicated kfunc for this. Sub-schedulers can
read p->scx.slice and p->scx.dsq_vtime from any task, they just can't
modify those values for the tasks they don't own.

So they could read the current value, apply the delta, and then use
scx_bpf_task_set_*() to update it.

Am I missing something here?

Thanks,
-Andrea

> ---
>  kernel/sched/ext.c                       | 27 +++++++++++++++++++++++-
>  tools/sched_ext/include/scx/compat.bpf.h |  9 ++++++++
>  2 files changed, 35 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/sched/ext.c b/kernel/sched/ext.c
> index e7ab3647e35f..26a85ec58d31 100644
> --- a/kernel/sched/ext.c
> +++ b/kernel/sched/ext.c
> @@ -6963,7 +6963,7 @@ static int bpf_scx_btf_struct_access(struct bpf_verifier_log *log,
>  		     off + size <= offsetofend(struct task_struct, scx.slice)) ||
>  		    (off >= offsetof(struct task_struct, scx.dsq_vtime) &&
>  		     off + size <= offsetofend(struct task_struct, scx.dsq_vtime))) {
> -			pr_warn("sched_ext: Writing directly to p->scx.slice/dsq_vtime is deprecated, use scx_bpf_task_set_slice/dsq_vtime()");
> +			pr_warn("sched_ext: Writing directly to p->scx.slice/dsq_vtime is deprecated, use scx_bpf_task_set_slice/dsq_vtime() or scx_bpf_task_add_dsq_vtime()");
>  			return SCALAR_VALUE;
>  		}
>  
> @@ -8281,6 +8281,30 @@ __bpf_kfunc bool scx_bpf_task_set_dsq_vtime(struct task_struct *p, u64 vtime,
>  	return true;
>  }
>  
> +/**
> + * scx_bpf_task_add_dsq_vtime - Add to task's virtual time for DSQ ordering
> + * @p: task of interest
> + * @vtime_delta: virtual time delta to add
> + * @aux: implicit BPF argument to access bpf_prog_aux hidden from BPF progs
> + *
> + * Add @vtime_delta to @p's virtual time. Returns %true on success, %false if the
> + * calling scheduler doesn't have authority over @p.
> + */
> +
> +__bpf_kfunc bool scx_bpf_task_add_dsq_vtime(struct task_struct *p, s64 vtime_delta,
> +					    const struct bpf_prog_aux *aux)
> +{
> +	struct scx_sched *sch;
> +
> +	guard(rcu)();
> +	sch = scx_prog_sched(aux);
> +	if (unlikely(!scx_task_on_sched(sch, p)))
> +		return false;
> +
> +	p->scx.dsq_vtime += vtime_delta;
> +	return true;
> +}
> +
>  static void scx_kick_cpu(struct scx_sched *sch, s32 cpu, u64 flags)
>  {
>  	struct rq *this_rq;
> @@ -9167,6 +9191,7 @@ __bpf_kfunc_end_defs();
>  BTF_KFUNCS_START(scx_kfunc_ids_any)
>  BTF_ID_FLAGS(func, scx_bpf_task_set_slice, KF_IMPLICIT_ARGS | KF_RCU);
>  BTF_ID_FLAGS(func, scx_bpf_task_set_dsq_vtime, KF_IMPLICIT_ARGS | KF_RCU);
> +BTF_ID_FLAGS(func, scx_bpf_task_add_dsq_vtime, KF_IMPLICIT_ARGS | KF_RCU);
>  BTF_ID_FLAGS(func, scx_bpf_kick_cpu, KF_IMPLICIT_ARGS)
>  BTF_ID_FLAGS(func, scx_bpf_dsq_nr_queued)
>  BTF_ID_FLAGS(func, scx_bpf_destroy_dsq)
> diff --git a/tools/sched_ext/include/scx/compat.bpf.h b/tools/sched_ext/include/scx/compat.bpf.h
> index 704728864d83..06f298f6909f 100644
> --- a/tools/sched_ext/include/scx/compat.bpf.h
> +++ b/tools/sched_ext/include/scx/compat.bpf.h
> @@ -343,6 +343,7 @@ scx_bpf_dsq_insert(struct task_struct *p, u64 dsq_id, u64 slice, u64 enq_flags)
>   */
>  bool scx_bpf_task_set_slice___new(struct task_struct *p, u64 slice) __ksym __weak;
>  bool scx_bpf_task_set_dsq_vtime___new(struct task_struct *p, u64 vtime) __ksym __weak;
> +bool scx_bpf_task_add_dsq_vtime___new(struct task_struct *p, u64 vtime_delta) __ksym __weak;
>  
>  static inline void scx_bpf_task_set_slice(struct task_struct *p, u64 slice)
>  {
> @@ -360,6 +361,14 @@ static inline void scx_bpf_task_set_dsq_vtime(struct task_struct *p, u64 vtime)
>  		p->scx.dsq_vtime = vtime;
>  }
>  
> +static inline void scx_bpf_task_add_dsq_vtime(struct task_struct *p, u64 vtime_delta)
> +{
> +	if (bpf_ksym_exists(scx_bpf_task_add_dsq_vtime___new))
> +		scx_bpf_task_add_dsq_vtime___new(p, vtime_delta);
> +	else
> +		p->scx.dsq_vtime += vtime_delta;
> +}
> +
>  /*
>   * v6.19: The new void variant can be called from anywhere while the older v1
>   * variant can only be called from ops.cpu_release(). The double ___ prefixes on
> -- 
> 2.48.1
> 

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

* Re: [PATCH] sched_ext: Add scx_bpf_task_add_dsq_vtime()
  2026-03-11 15:58 ` Andrea Righi
@ 2026-03-11 16:12   ` Cheng-Yang Chou
  0 siblings, 0 replies; 3+ messages in thread
From: Cheng-Yang Chou @ 2026-03-11 16:12 UTC (permalink / raw)
  To: Andrea Righi; +Cc: sched-ext, tj, void, changwoo, jserv

Hi Andrea,

On Wed, Mar 11, 2026 at 04:58:27PM +0100, Andrea Righi wrote:
> Hm, I'm not sure we need a dedicated kfunc for this. Sub-schedulers can
> read p->scx.slice and p->scx.dsq_vtime from any task, they just can't
> modify those values for the tasks they don't own.
> 
> So they could read the current value, apply the delta, and then use
> scx_bpf_task_set_*() to update it.
> 
> Am I missing something here?

Oh I see.

My original thought was just to make the API a bit more convenient.
Instead of writing:

    scx_bpf_task_set_dsq_vtime(p, p->scx.dsq_vtime + delta);

Developers could simply write:

    scx_bpf_task_add_dsq_vtime(p, delta);

But as you pointed out, the set() helper is already a straightforward
one-liner and functionally identical. The authority check is also
properly handled.

I am totally fine with dropping this patch.

-- 
Thanks,
Cheng-Yang

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

end of thread, other threads:[~2026-03-11 16:12 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-11 15:39 [PATCH] sched_ext: Add scx_bpf_task_add_dsq_vtime() Cheng-Yang Chou
2026-03-11 15:58 ` Andrea Righi
2026-03-11 16:12   ` Cheng-Yang Chou

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