All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrea Righi <arighi@nvidia.com>
To: Tejun Heo <tj@kernel.org>
Cc: sched-ext@lists.linux.dev, David Vernet <void@manifault.com>,
	Changwoo Min <changwoo@igalia.com>,
	Cheng-Yang Chou <yphbchou0911@gmail.com>,
	Juntong Deng <juntong.deng@outlook.com>,
	Ching-Chun Huang <jserv@ccns.ncku.edu.tw>,
	Chia-Ping Tsai <chia7712@gmail.com>,
	Emil Tsalapatis <emil@etsalapatis.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 02/10] sched_ext: Add select_cpu kfuncs to scx_kfunc_ids_unlocked
Date: Fri, 10 Apr 2026 18:07:16 +0200	[thread overview]
Message-ID: <adkgNHHwGL2uM46o@gpd4> (raw)
In-Reply-To: <20260410063046.3556100-3-tj@kernel.org>

On Thu, Apr 09, 2026 at 08:30:38PM -1000, Tejun Heo wrote:
> select_cpu_from_kfunc() has an extra scx_kf_allowed_if_unlocked() branch
> that accepts calls from unlocked contexts and takes task_rq_lock() itself
> - a "callable from unlocked" property encoded in the kfunc body rather
> than in set membership. That's fine while the runtime check is the
> authoritative gate, but the upcoming verifier-time filter uses set
> membership as the source of truth and needs it to reflect every context
> the kfunc may be called from.
> 
> Add the three select_cpu kfuncs to scx_kfunc_ids_unlocked so their full
> set of callable contexts is captured by set membership. This follows the
> existing dual-set convention used by scx_bpf_dsq_move{,_vtime} and
> scx_bpf_dsq_move_set_{slice,vtime}, which are members of both
> scx_kfunc_ids_dispatch and scx_kfunc_ids_unlocked.
> 
> While at it, add brief comments on each duplicate BTF_ID_FLAGS block
> (including the pre-existing dsq_move ones) explaining the dual
> membership.
> 
> No runtime behavior change: the runtime check in select_cpu_from_kfunc()
> remains the authoritative gate until it is removed along with the rest
> of the scx_kf_mask enforcement in a follow-up.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>

Reviewed-by: Andrea Righi <arighi@nvidia.com>

Minor nit below.

> ---
>  kernel/sched/ext.c      | 6 ++++++
>  kernel/sched/ext_idle.c | 4 ++++
>  2 files changed, 10 insertions(+)
> 
> diff --git a/kernel/sched/ext.c b/kernel/sched/ext.c
> index b757b853b42b..cf441fb4b1ad 100644
> --- a/kernel/sched/ext.c
> +++ b/kernel/sched/ext.c
> @@ -8497,6 +8497,7 @@ BTF_ID_FLAGS(func, scx_bpf_dispatch_nr_slots, KF_IMPLICIT_ARGS)
>  BTF_ID_FLAGS(func, scx_bpf_dispatch_cancel, KF_IMPLICIT_ARGS)
>  BTF_ID_FLAGS(func, scx_bpf_dsq_move_to_local, KF_IMPLICIT_ARGS)
>  BTF_ID_FLAGS(func, scx_bpf_dsq_move_to_local___v2, KF_IMPLICIT_ARGS)
> +/* also in scx_kfunc_ids_unlocked: also callable from unlocked contexts */
>  BTF_ID_FLAGS(func, scx_bpf_dsq_move_set_slice, KF_RCU)
>  BTF_ID_FLAGS(func, scx_bpf_dsq_move_set_vtime, KF_RCU)
>  BTF_ID_FLAGS(func, scx_bpf_dsq_move, KF_RCU)

Nit: we don't see it from the patch context, but below there's
scx_bpf_sub_dispatch() which also seems to be callable from unlocked context
with this comment. Maybe move scx_bpf_sub_dispatch() before this comment?

> @@ -8612,10 +8613,15 @@ __bpf_kfunc_end_defs();
>  
>  BTF_KFUNCS_START(scx_kfunc_ids_unlocked)
>  BTF_ID_FLAGS(func, scx_bpf_create_dsq, KF_IMPLICIT_ARGS | KF_SLEEPABLE)
> +/* also in scx_kfunc_ids_dispatch: also callable from ops.dispatch() */
>  BTF_ID_FLAGS(func, scx_bpf_dsq_move_set_slice, KF_RCU)
>  BTF_ID_FLAGS(func, scx_bpf_dsq_move_set_vtime, KF_RCU)
>  BTF_ID_FLAGS(func, scx_bpf_dsq_move, KF_RCU)
>  BTF_ID_FLAGS(func, scx_bpf_dsq_move_vtime, KF_RCU)
> +/* also in scx_kfunc_ids_select_cpu: also callable from ops.select_cpu()/ops.enqueue() */
> +BTF_ID_FLAGS(func, __scx_bpf_select_cpu_and, KF_IMPLICIT_ARGS | KF_RCU)
> +BTF_ID_FLAGS(func, scx_bpf_select_cpu_and, KF_RCU)
> +BTF_ID_FLAGS(func, scx_bpf_select_cpu_dfl, KF_IMPLICIT_ARGS | KF_RCU)
>  BTF_KFUNCS_END(scx_kfunc_ids_unlocked)
>  
>  static const struct btf_kfunc_id_set scx_kfunc_set_unlocked = {
> diff --git a/kernel/sched/ext_idle.c b/kernel/sched/ext_idle.c
> index cd88aee47bd8..8c31fb65477c 100644
> --- a/kernel/sched/ext_idle.c
> +++ b/kernel/sched/ext_idle.c
> @@ -1482,6 +1482,10 @@ static const struct btf_kfunc_id_set scx_kfunc_set_idle = {
>   * contexts where @p's pi_lock state is unknown. Keep them out of
>   * BPF_PROG_TYPE_TRACING by registering them in their own set which is exposed
>   * only to STRUCT_OPS and SYSCALL programs.
> + *
> + * These kfuncs are also members of scx_kfunc_ids_unlocked (see ext.c) because
> + * they're callable from unlocked contexts in addition to ops.select_cpu() and
> + * ops.enqueue().
>   */
>  BTF_KFUNCS_START(scx_kfunc_ids_select_cpu)
>  BTF_ID_FLAGS(func, __scx_bpf_select_cpu_and, KF_IMPLICIT_ARGS | KF_RCU)
> -- 
> 2.53.0
> 

Thanks,
-Andrea

  reply	other threads:[~2026-04-10 16:07 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-10  6:30 [PATCHSET sched_ext/for-7.1] sched_ext: Add verifier-time kfunc context filter Tejun Heo
2026-04-10  6:30 ` [PATCH 01/10] sched_ext: Drop TRACING access to select_cpu kfuncs Tejun Heo
2026-04-10 16:04   ` Andrea Righi
2026-04-10  6:30 ` [PATCH 02/10] sched_ext: Add select_cpu kfuncs to scx_kfunc_ids_unlocked Tejun Heo
2026-04-10 16:07   ` Andrea Righi [this message]
2026-04-10 17:51   ` [PATCH v2 " Tejun Heo
2026-04-10  6:30 ` [PATCH 03/10] sched_ext: Track @p's rq lock across set_cpus_allowed_scx -> ops.set_cpumask Tejun Heo
2026-04-10 16:12   ` Andrea Righi
2026-04-10 17:51   ` [PATCH v2 " Tejun Heo
2026-04-10  6:30 ` [PATCH 04/10] sched_ext: Fix ops.cgroup_move() invocation kf_mask and rq tracking Tejun Heo
2026-04-10 16:16   ` Andrea Righi
2026-04-10 17:51   ` [PATCH v2 " Tejun Heo
2026-04-10  6:30 ` [PATCH 05/10] sched_ext: Decouple kfunc unlocked-context check from kf_mask Tejun Heo
2026-04-10 16:34   ` Andrea Righi
2026-04-10 17:51   ` [PATCH v2 " Tejun Heo
2026-04-10  6:30 ` [PATCH 06/10] sched_ext: Drop redundant rq-locked check from scx_bpf_task_cgroup() Tejun Heo
2026-04-10 16:36   ` Andrea Righi
2026-04-10  6:30 ` [PATCH 07/10] sched_ext: Add verifier-time kfunc context filter Tejun Heo
2026-04-10 16:49   ` Andrea Righi
2026-04-14 12:38   ` Cheng-Yang Chou
2026-04-14 17:25     ` Tejun Heo
2026-04-10  6:30 ` [PATCH 08/10] sched_ext: Remove runtime kfunc mask enforcement Tejun Heo
2026-04-10 16:50   ` Andrea Righi
2026-04-10  6:30 ` [PATCH 09/10] sched_ext: Rename scx_kf_allowed_on_arg_tasks() to scx_kf_arg_task_ok() Tejun Heo
2026-04-10 16:55   ` Andrea Righi
2026-04-10  6:30 ` [PATCH 10/10] sched_ext: Warn on task-based SCX op recursion Tejun Heo
2026-04-10 17:38   ` Andrea Righi
2026-04-10 17:45 ` [PATCHSET sched_ext/for-7.1] sched_ext: Add verifier-time kfunc context filter Andrea Righi
2026-04-11  6:17   ` Cheng-Yang Chou
2026-04-11  7:41     ` Tejun Heo
2026-04-11 15:09       ` Cheng-Yang Chou

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=adkgNHHwGL2uM46o@gpd4 \
    --to=arighi@nvidia.com \
    --cc=changwoo@igalia.com \
    --cc=chia7712@gmail.com \
    --cc=emil@etsalapatis.com \
    --cc=jserv@ccns.ncku.edu.tw \
    --cc=juntong.deng@outlook.com \
    --cc=linux-kernel@vger.kernel.org \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.