public inbox for sched-ext@lists.linux.dev
 help / color / mirror / Atom feed
From: Andrea Righi <arighi@nvidia.com>
To: Cheng-Yang Chou <yphbchou0911@gmail.com>
Cc: sched-ext@lists.linux.dev, tj@kernel.org, void@manifault.com,
	changwoo@igalia.com, jserv@ccns.ncku.edu.tw
Subject: Re: [PATCH 1/2] sched_ext: Update demo schedulers and selftests to use scx_bpf_task_set_dsq_vtime()
Date: Thu, 12 Mar 2026 07:41:47 +0100	[thread overview]
Message-ID: <abJgK3m8Z0ELfGrF@gpd4> (raw)
In-Reply-To: <20260312042001.955675-2-yphbchou0911@gmail.com>

Hi Cheng-Yang,

On Thu, Mar 12, 2026 at 12:20:00PM +0800, Cheng-Yang Chou wrote:
> Direct writes to p->scx.dsq_vtime are deprecated in favor of
> scx_bpf_task_set_dsq_vtime(). Update scx_simple, scx_flatcg, and
> select_cpu_vtime selftest to use the new kfunc.
> 
> Use bpf_ksym_exists() to fall back to direct assignment on older kernels
> that don't have the new kfunc, preserving backwards compatibility.
> 
> Signed-off-by: Cheng-Yang Chou <yphbchou0911@gmail.com>
> ---
>  tools/sched_ext/scx_flatcg.bpf.c              | 21 ++++++++++++++-----
>  tools/sched_ext/scx_simple.bpf.c              | 12 +++++++++--
>  .../sched_ext/select_cpu_vtime.bpf.c          | 13 ++++++++++--
>  3 files changed, 37 insertions(+), 9 deletions(-)
> 
> diff --git a/tools/sched_ext/scx_flatcg.bpf.c b/tools/sched_ext/scx_flatcg.bpf.c
> index a8a9234bb41e..27d99bb1e60f 100644
> --- a/tools/sched_ext/scx_flatcg.bpf.c
> +++ b/tools/sched_ext/scx_flatcg.bpf.c
> @@ -551,9 +551,14 @@ void BPF_STRUCT_OPS(fcg_stopping, struct task_struct *p, bool runnable)
>  	 * too much, determine the execution time by taking explicit timestamps
>  	 * instead of depending on @p->scx.slice.
>  	 */
> -	if (!fifo_sched)
> -		p->scx.dsq_vtime +=
> -			(SCX_SLICE_DFL - p->scx.slice) * 100 / p->scx.weight;
> +	u64 delta = (SCX_SLICE_DFL - p->scx.slice) * 100 / p->scx.weight;

While at it, can we use:

 u64 delta = scale_by_task_weight(p, SCX_SLICE_DFL - p->scx.slice);

> +
> +	if (!fifo_sched) {
> +		if (bpf_ksym_exists(scx_bpf_task_set_dsq_vtime___new))
> +			scx_bpf_task_set_dsq_vtime___new(p, p->scx.dsq_vtime + delta);
> +		else
> +			p->scx.dsq_vtime += delta;
> +	}

No, let's just use:

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

It's already doing the bpf_ksym_exists() logic internally, see
tools/sched_ext/include/scx/compat.bpf.h.

>  
>  	taskc = bpf_task_storage_get(&task_ctx, p, 0, 0);
>  	if (!taskc) {
> @@ -822,7 +827,10 @@ s32 BPF_STRUCT_OPS(fcg_init_task, struct task_struct *p,
>  	if (!(cgc = find_cgrp_ctx(args->cgroup)))
>  		return -ENOENT;
>  
> -	p->scx.dsq_vtime = cgc->tvtime_now;
> +	if (bpf_ksym_exists(scx_bpf_task_set_dsq_vtime___new))
> +		scx_bpf_task_set_dsq_vtime___new(p, cgc->tvtime_now);
> +	else
> +		p->scx.dsq_vtime = cgc->tvtime_now;

Ditto.

>  
>  	return 0;
>  }
> @@ -924,7 +932,10 @@ void BPF_STRUCT_OPS(fcg_cgroup_move, struct task_struct *p,
>  		return;
>  
>  	delta = time_delta(p->scx.dsq_vtime, from_cgc->tvtime_now);
> -	p->scx.dsq_vtime = to_cgc->tvtime_now + delta;
> +	if (bpf_ksym_exists(scx_bpf_task_set_dsq_vtime___new))
> +		scx_bpf_task_set_dsq_vtime___new(p, to_cgc->tvtime_now + delta);
> +	else
> +		p->scx.dsq_vtime = to_cgc->tvtime_now + delta;

Ditto.

>  }
>  
>  s32 BPF_STRUCT_OPS_SLEEPABLE(fcg_init)
> diff --git a/tools/sched_ext/scx_simple.bpf.c b/tools/sched_ext/scx_simple.bpf.c
> index b456bd7cae77..61d3bcf54ce7 100644
> --- a/tools/sched_ext/scx_simple.bpf.c
> +++ b/tools/sched_ext/scx_simple.bpf.c
> @@ -121,12 +121,20 @@ void BPF_STRUCT_OPS(simple_stopping, struct task_struct *p, bool runnable)
>  	 * too much, determine the execution time by taking explicit timestamps
>  	 * instead of depending on @p->scx.slice.
>  	 */
> -	p->scx.dsq_vtime += (SCX_SLICE_DFL - p->scx.slice) * 100 / p->scx.weight;
> +	u64 delta = (SCX_SLICE_DFL - p->scx.slice) * 100 / p->scx.weight;
> +
> +	if (bpf_ksym_exists(scx_bpf_task_set_dsq_vtime___new))
> +		scx_bpf_task_set_dsq_vtime___new(p, p->scx.dsq_vtime + delta);
> +	else
> +		p->scx.dsq_vtime += delta;

Ditto.

>  }
>  
>  void BPF_STRUCT_OPS(simple_enable, struct task_struct *p)
>  {
> -	p->scx.dsq_vtime = vtime_now;
> +	if (bpf_ksym_exists(scx_bpf_task_set_dsq_vtime___new))
> +		scx_bpf_task_set_dsq_vtime___new(p, vtime_now);
> +	else
> +		p->scx.dsq_vtime = vtime_now;

Ditto.

>  }
>  
>  s32 BPF_STRUCT_OPS_SLEEPABLE(simple_init)
> diff --git a/tools/testing/selftests/sched_ext/select_cpu_vtime.bpf.c b/tools/testing/selftests/sched_ext/select_cpu_vtime.bpf.c
> index bfcb96cd4954..17ed515c3e25 100644
> --- a/tools/testing/selftests/sched_ext/select_cpu_vtime.bpf.c
> +++ b/tools/testing/selftests/sched_ext/select_cpu_vtime.bpf.c
> @@ -66,12 +66,21 @@ void BPF_STRUCT_OPS(select_cpu_vtime_running, struct task_struct *p)
>  void BPF_STRUCT_OPS(select_cpu_vtime_stopping, struct task_struct *p,
>  		    bool runnable)
>  {
> -	p->scx.dsq_vtime += (SCX_SLICE_DFL - p->scx.slice) * 100 / p->scx.weight;
> +	u64 delta = (SCX_SLICE_DFL - p->scx.slice) * 100 / p->scx.weight;

Ditto (scale_by_task_weight).

> +
> +	if (bpf_ksym_exists(scx_bpf_task_set_dsq_vtime___new))
> +		scx_bpf_task_set_dsq_vtime___new(p, p->scx.dsq_vtime + delta);
> +	else
> +		p->scx.dsq_vtime += delta;
> +

Ditto.

>  }
>  
>  void BPF_STRUCT_OPS(select_cpu_vtime_enable, struct task_struct *p)
>  {
> -	p->scx.dsq_vtime = vtime_now;
> +	if (bpf_ksym_exists(scx_bpf_task_set_dsq_vtime___new))
> +		scx_bpf_task_set_dsq_vtime___new(p, vtime_now);
> +	else
> +		p->scx.dsq_vtime = vtime_now;

Ditto.

>  }
>  
>  s32 BPF_STRUCT_OPS_SLEEPABLE(select_cpu_vtime_init)
> -- 
> 2.48.1
> 

Thanks,
-Andrea

  reply	other threads:[~2026-03-12  6:42 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-12  4:19 [PATCH 0/2] sched_ext: Update demo schedulers and selftests for deprecated APIs Cheng-Yang Chou
2026-03-12  4:20 ` [PATCH 1/2] sched_ext: Update demo schedulers and selftests to use scx_bpf_task_set_dsq_vtime() Cheng-Yang Chou
2026-03-12  6:41   ` Andrea Righi [this message]
2026-03-12  4:20 ` [PATCH 2/2] sched_ext: Update demo schedulers and selftests to drop ops.cpu_acquire/release() Cheng-Yang Chou
2026-03-12  7:40   ` Andrea Righi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=abJgK3m8Z0ELfGrF@gpd4 \
    --to=arighi@nvidia.com \
    --cc=changwoo@igalia.com \
    --cc=jserv@ccns.ncku.edu.tw \
    --cc=sched-ext@lists.linux.dev \
    --cc=tj@kernel.org \
    --cc=void@manifault.com \
    --cc=yphbchou0911@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox