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

[Andrea Righi <arighi@nvidia.com>]

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