linux-kernel - Re: [PATCH v2 sched_ext/for-6.19] sched_ext: Use ___v2 suffix for new kfuncs and fix scx build errors

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aPkyphtSDYDydnUm@gpd4>
Date: Wed, 22 Oct 2025 21:38:14 +0200
From: Andrea Righi <arighi@...dia.com>
To: Tejun Heo <tj@...nel.org>
Cc: David Vernet <void@...ifault.com>, Changwoo Min <changwoo@...lia.com>,
	Emil Tsalapatis <emil@...alapatis.com>, sched-ext@...ts.linux.dev,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 sched_ext/for-6.19] sched_ext: Use ___v2 suffix for
 new kfuncs and fix scx build errors

On Wed, Oct 22, 2025 at 06:34:50AM -1000, Tejun Heo wrote:
> Hello,
> 
> On Wed, Oct 22, 2025 at 05:36:10PM +0200, Andrea Righi wrote:
> > Following commit 2dbbdeda77a61 ("sched_ext: Fix scx_bpf_dsq_insert()
> > backward binary compatibility"), consistently use the ___v2 suffix also
> > to the new scx_bpf_dsq_insert_vtime() and scx_bpf_select_cpu_and()
> > kfuncs.
> 
> It's a bit subtle but the assumption around ___VER is that that isn't (going
> to be) visible to BPF users and will eventually be dropped. Here, it's a bit
> different. The arg packing is something we'll need to do indefinitely unless
> BPF lifts the limit on #args. So, we will continue to have the internal
> kfunc which takes the packaged arguments and user-facing wrapper that hides
> that. So, I think __ prefix (something more explicit works top - e.g.
> argpack prefix or suffix) is a better option here.

Ahh ok, so user-space schedulers will always continue to pass all the
arguments "normally" and we just assemble the args struct via an inline
helper.

So, IIUC, using an _argpack suffix or something similar (instead of ___v2)
should be a reasonable solution, right?

> 
> > Introduce __COMPAT_scx_bpf_select_cpu_and() and
> > __COMPAT_scx_bpf_dsq_insert_vtime(), to ensure schedulers can transition
> > smoothly to the updated interfaces, and temporarily mirror the
> > definitions of struct scx_bpf_select_cpu_and_args and struct
> > scx_bpf_dsq_insert_vtime_args to prevent build failures on kernels where
> > these structs are not yet defined.
> 
> Given that there is on capability difference between before and after from
> the scheduler POV, I'm not sure we need to make __COMPAT explicit. There's
> nothing really gained by adding the prefix. This has been evolving over
> time, but I think a reasonable rule of thumb is:
> 
>  If the SCX core introduces a new feature which may affect BPF scheduler
>  operations in a noticeable way, that feature should be gated behind
>  __COMPAT. The BPF scheduler using a __COMPAT prefixed interface should then
>  be able to handle cases where the feature is not implemented. If the BPF
>  scheduler depends on the new feature (ie. it doesn't want to stay
>  compatible with older kernels), it should use the interface without
>  __COMPAT.
> 
> Here, there is no noticeable feature difference before and after for
> existing schedulers, so I don't think it's necessary to introduce __COMPAT
> prefix.

The problem is that some schedulers (i.e., scx_bpfland, scx_cosmos) are
explicitly checking bpf_ksym_exists(scx_bpf_select_cpu_and). If
scx_bpf_select_cpu_and() becomes a static inline, we break the build and we
also break binary compatibility. Hence the __COMPAT for the inline
helpers...

Thanks,
-Andrea