| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <aOTh_PAGkX9y2Tsy@gpd4>
Date: Tue, 7 Oct 2025 11:48:44 +0200
From: Andrea Righi <arighi@...dia.com>
To: Tejun Heo <tj@...nel.org>
Cc: David Vernet <void@...ifault.com>, Changwoo Min <changwoo@...lia.com>,
linux-kernel@...r.kernel.org, sched-ext@...ts.linux.dev
Subject: Re: [PATCH 3/4] sched_ext: Wrap kfunc args in struct to prepare for
aux__prog
Hi Tejun,
On Mon, Oct 06, 2025 at 03:51:46PM -1000, Tejun Heo wrote:
> scx_bpf_dsq_insert_vtime() and scx_bpf_select_cpu_and() currently have 5
> parameters. An upcoming change will add aux__prog parameter which will exceed
> BPF's 5 argument limit.
>
> Prepare by adding new kfuncs __scx_bpf_dsq_insert_vtime() and
> __scx_bpf_select_cpu_and() that take args structs. The existing kfuncs are
> kept as compatibility wrappers. BPF programs use inline wrappers that detect
> kernel API version via bpf_core_type_exists() and use the new struct-based
> kfuncs when available, falling back to compat kfuncs otherwise. This allows
> BPF programs to work with both old and new kernels.
>
> Signed-off-by: Tejun Heo <tj@...nel.org>
> ---
> kernel/sched/ext.c | 82 ++++++++++++++++++------
> kernel/sched/ext_idle.c | 43 +++++++++++--
> tools/sched_ext/include/scx/common.bpf.h | 6 +-
> tools/sched_ext/include/scx/compat.bpf.h | 72 +++++++++++++++++++++
> 4 files changed, 173 insertions(+), 30 deletions(-)
>
> diff --git a/kernel/sched/ext.c b/kernel/sched/ext.c
> index 6d76efaaa9b2..a34e731229de 100644
> --- a/kernel/sched/ext.c
> +++ b/kernel/sched/ext.c
> @@ -5345,54 +5345,94 @@ __bpf_kfunc void scx_bpf_dsq_insert(struct task_struct *p, u64 dsq_id, u64 slice
> scx_dsq_insert_commit(sch, p, dsq_id, enq_flags);
> }
>
> +static void scx_dsq_insert_vtime(struct scx_sched *sch, struct task_struct *p,
> + u64 dsq_id, u64 slice, u64 vtime, u64 enq_flags)
> +{
> + if (!scx_dsq_insert_preamble(sch, p, enq_flags))
> + return;
> +
> + if (slice)
> + p->scx.slice = slice;
> + else
> + p->scx.slice = p->scx.slice ?: 1;
> +
> + p->scx.dsq_vtime = vtime;
> +
> + scx_dsq_insert_commit(sch, p, dsq_id, enq_flags | SCX_ENQ_DSQ_PRIQ);
> +}
> +
> +struct scx_bpf_dsq_insert_vtime_args {
> + /* @p can't be packed together as KF_RCU is not transitive */
> + u64 dsq_id;
> + u64 slice;
> + u64 vtime;
> + u64 enq_flags;
> +};
With PATCH 2/4 introducing scx_bpf_task_set_slice() and
scx_bpf_task_set_dsq_vtime(), would it be reasonable to use those to set
these task properties and then completely get rid of these args in
scx_bpf_dsq_insert[_vtime]()?
> +
> /**
> - * scx_bpf_dsq_insert_vtime - Insert a task into the vtime priority queue of a DSQ
> + * __scx_bpf_dsq_insert_vtime - Arg-wrapped vtime DSQ insertion
> * @p: task_struct to insert
> - * @dsq_id: DSQ to insert into
> - * @slice: duration @p can run for in nsecs, 0 to keep the current value
> - * @vtime: @p's ordering inside the vtime-sorted queue of the target DSQ
> - * @enq_flags: SCX_ENQ_*
> + * @args: struct containing the rest of the arguments
> + * @args->dsq_id: DSQ to insert into
> + * @args->slice: duration @p can run for in nsecs, 0 to keep the current value
> + * @args->vtime: @p's ordering inside the vtime-sorted queue of the target DSQ
> + * @args->enq_flags: SCX_ENQ_*
> *
> - * Insert @p into the vtime priority queue of the DSQ identified by @dsq_id.
> - * Tasks queued into the priority queue are ordered by @vtime. All other aspects
> - * are identical to scx_bpf_dsq_insert().
> + * Wrapper kfunc that takes arguments via struct to work around BPF's 5 argument
> + * limit. BPF programs should use scx_bpf_dsq_insert_vtime() which is provided
> + * as an inline wrapper in common.bpf.h.
> *
> - * @vtime ordering is according to time_before64() which considers wrapping. A
> - * numerically larger vtime may indicate an earlier position in the ordering and
> - * vice-versa.
> + * Insert @p into the vtime priority queue of the DSQ identified by
> + * @args->dsq_id. Tasks queued into the priority queue are ordered by
> + * @args->vtime. All other aspects are identical to scx_bpf_dsq_insert().
> + *
> + * @args->vtime ordering is according to time_before64() which considers
> + * wrapping. A numerically larger vtime may indicate an earlier position in the
> + * ordering and vice-versa.
> *
> * A DSQ can only be used as a FIFO or priority queue at any given time and this
> * function must not be called on a DSQ which already has one or more FIFO tasks
> * queued and vice-versa. Also, the built-in DSQs (SCX_DSQ_LOCAL and
> * SCX_DSQ_GLOBAL) cannot be used as priority queues.
> */
> -__bpf_kfunc void scx_bpf_dsq_insert_vtime(struct task_struct *p, u64 dsq_id,
> - u64 slice, u64 vtime, u64 enq_flags)
> +__bpf_kfunc void
> +__scx_bpf_dsq_insert_vtime(struct task_struct *p,
> + struct scx_bpf_dsq_insert_vtime_args *args)
> {
> struct scx_sched *sch;
>
> guard(rcu)();
> +
> sch = rcu_dereference(scx_root);
> if (unlikely(!sch))
> return;
>
> - if (!scx_dsq_insert_preamble(sch, p, enq_flags))
> - return;
> + scx_dsq_insert_vtime(sch, p, args->dsq_id, args->slice, args->vtime,
> + args->enq_flags);
> +}
>
> - if (slice)
> - p->scx.slice = slice;
> - else
> - p->scx.slice = p->scx.slice ?: 1;
> +/*
> + * COMPAT: Will be removed in v6.23.
> + */
> +__bpf_kfunc void scx_bpf_dsq_insert_vtime(struct task_struct *p, u64 dsq_id,
> + u64 slice, u64 vtime, u64 enq_flags)
> +{
> + struct scx_sched *sch;
>
> - p->scx.dsq_vtime = vtime;
> + guard(rcu)();
>
> - scx_dsq_insert_commit(sch, p, dsq_id, enq_flags | SCX_ENQ_DSQ_PRIQ);
> + sch = rcu_dereference(scx_root);
> + if (unlikely(!sch))
> + return;
> +
> + scx_dsq_insert_vtime(sch, p, dsq_id, slice, vtime, enq_flags);
> }
>
> __bpf_kfunc_end_defs();
>
> BTF_KFUNCS_START(scx_kfunc_ids_enqueue_dispatch)
> BTF_ID_FLAGS(func, scx_bpf_dsq_insert, KF_RCU)
> +BTF_ID_FLAGS(func, __scx_bpf_dsq_insert_vtime, KF_RCU)
> BTF_ID_FLAGS(func, scx_bpf_dsq_insert_vtime, KF_RCU)
> BTF_KFUNCS_END(scx_kfunc_ids_enqueue_dispatch)
>
> diff --git a/kernel/sched/ext_idle.c b/kernel/sched/ext_idle.c
> index d2434c954848..3d9d404d5cd2 100644
> --- a/kernel/sched/ext_idle.c
> +++ b/kernel/sched/ext_idle.c
> @@ -995,26 +995,56 @@ __bpf_kfunc s32 scx_bpf_select_cpu_dfl(struct task_struct *p, s32 prev_cpu,
> return prev_cpu;
> }
>
> +struct scx_bpf_select_cpu_and_args {
> + /* @p and @cpus_allowed can't be packed together as KF_RCU is not transitive */
> + s32 prev_cpu;
> + u64 wake_flags;
> + u64 flags;
> +};
And for this one, would it make sense to pack flags and wake_flags in a
single u64?
Thanks,
-Andrea
Powered by blists - more mailing lists