Re: [PATCH] sched_ext: Add scx_bpf_task_add_dsq_vtime()

[Andrea Righi <arighi@nvidia.com>]

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
>