Re: [PATCH 12/16] sched_ext: Forbid cpu-form kfuncs from cid-form schedulers

[Zhao Mengmeng <zhaomzhao@126.com>]

On 4/23/26 17:48, Cheng-Yang Chou wrote:
> Hi Zhao,
> 
> On Thu, Apr 23, 2026 at 05:35:16PM +0800, Zhao Mengmeng wrote:
>> On 2026-04-23 17:21 +0800, Cheng-Yang Chou wrote:
>>> Hi Zhao,
>>>
>>> On Thu, Apr 23, 2026 at 05:03:23PM +0800, Zhao Mengmeng wrote:
>>>> non_scx_kfunc_deny.bpf.c:17:6: error: conflicting types for 'scx_bpf_kick_cpu'
>>>> 17 | void scx_bpf_kick_cpu(s32 cpu, u64 flags) __ksym;
>>>> |      ^
>>>> /root/work/source-code/linux-next/tools/testing/selftests/sched_ext/build/include/vmlinux.h:136300:13: note: previous declaration is here
>>>> 136300 | extern void scx_bpf_kick_cpu(s32 cpu, u64 flags, const struct bpf_prog_aux *aux) __weak __ksym;
>>>> |             ^
>>>> non_scx_kfunc_deny.bpf.c:26:23: error: too few arguments to function call, expected 3, have 2
>>>> 26 |         scx_bpf_kick_cpu(0, 0);
>>>> |         ~~~~~~~~~~~~~~~~     ^
>>>> /root/work/source-code/linux-next/tools/testing/selftests/sched_ext/build/include/vmlinux.h:136300:13: note: 'scx_bpf_kick_cpu' declared here
>>>> 136300 | extern void scx_bpf_kick_cpu(s32 cpu, u64 flags, const struct bpf_prog_aux *aux) __weak __ksym;
>>>>
>>>> On the one hand, non_scx_kfunc_deny.bpf.c has it own problem, it should
>>>> not declare scx_bpf_kick_cpu(), on the other hand, the root cause is after
>>>
>>> If your point is that a TCP program shouldn't declare scx_bpf_kick_cpu(),
>>> you are correct. The main goal of non_scx_kfunc_deny is to ensure that,
>>> witch commit 2d2b026c3ea7, no non-SCX programs can use SCX func.
>>
>> No, I mean this bpf program can just `#include <scx/common.bpf.h>` at the
>> start of the file, remove the scx_bpf_kick_cpu() declaration.
>>
>> diff --git a/tools/testing/selftests/sched_ext/non_scx_kfunc_deny.bpf.c b/tools/testing/selftests/sched_ext/non_scx_kfunc_deny.bpf.c
>> index 9f16d39255e7..0d6fcc8e5eb6 100644
>> --- a/tools/testing/selftests/sched_ext/non_scx_kfunc_deny.bpf.c
>> +++ b/tools/testing/selftests/sched_ext/non_scx_kfunc_deny.bpf.c
>> @@ -9,12 +9,7 @@
>> * Copyright (C) 2026 Cheng-Yang Chou <yphbchou0911@gmail.com>
>> */
>>
>> -#include <vmlinux.h>
>> -#include <bpf/bpf_helpers.h>
>> -#include <bpf/bpf_tracing.h>
>> -
>> -/* SCX kfunc from scx_kfunc_ids_any set */
>> -void scx_bpf_kick_cpu(s32 cpu, u64 flags) __ksym;
>> +#include <scx/common.bpf.h>
>>
>> SEC("struct_ops/ssthresh")
>> __u32 BPF_PROG(tcp_ca_ssthresh, struct sock *sk)
>>
>> But before this commit, your code builds well, after this commit, it
>> failed. That's why I suggest Tejun to add the missing KF_IMPLICIT_ARGS, it
>> changed the signature in generated vmlinux.h, which I think we don't need
>> expose aux argument to bpf program.
> 
> Ahh, thanks for the explanation, now I get it. 
> Are you planning to send a patch to fix this, or would you like me to
> spin one?
> 
> I tested the fix above, and everything works fine.
> 

I am working on a patch and is about to send it, I will add your Tested-by,
WDYT? 
>>>
>>>> this commit, the signature of scx_bpf_kick_cpu() changes from
>>>>
>>>> `extern void scx_bpf_kick_cpu(s32 cpu, u64 flags) __weak __ksym;` to
>>>> `extern void scx_bpf_kick_cpu(s32 cpu, u64 flags, const struct bpf_prog_aux *aux) __weak __ksym` 
>>>
>>> Regarding the non_scx_kfunc_deny.bpf.c build failure, I'll fix it. If
>>> needed, we can test with another SCX function that doesn't require the
>>> *aux parameter.
>>>
>>> Thanks.
>>
>> Like I said above, it builds well on for-next branch, just this commit
>> trigger the failure. I believe your test case is very reasonable.
>>
>> Best regards,
>> --  
>> Zhao Mengmeng <zhaomengmeng@kylinos.cn>
>>>>
>>>> After code analysis and test, I believe scx_bpf_kick_cpu miss the 
>>>> KF_IMPILCIT_ARGS, just like the defination in scx_kfunc_ids_any.
>>>>
>>>> So here misses  KF_IMPLICIT_ARGS 
>>>>> +BTF_ID_FLAGS(func, scx_bpf_task_cpu)
>>>> Missing KF_RCU.
>>>>> +BTF_ID_FLAGS(func, scx_bpf_cpu_rq)
>>>> Missing KF_IMPLICIT_ARGS
>>>>> +BTF_ID_FLAGS(func, scx_bpf_cpu_curr)
>>>> Missing KF_IMPLICIT_ARGS | KF_RET_NULL | KF_RCU_PROTECTED
>>>>> +BTF_ID_FLAGS(func, scx_bpf_cpu_node)
>>>>> +BTF_ID_FLAGS(func, scx_bpf_cpuperf_cap)
>>>> Missing KF_IMPLICIT_ARGS
>>>>> +BTF_ID_FLAGS(func, scx_bpf_cpuperf_cur)
>>>> Missing KF_IMPLICIT_ARGS
>>>>> +BTF_ID_FLAGS(func, scx_bpf_cpuperf_set)
>>>> Missing KF_IMPLICIT_ARGS
>>>>> +BTF_ID_FLAGS(func, scx_bpf_get_possible_cpumask)
>>>> Missing KF_ACQUIRE
>>>>> +BTF_ID_FLAGS(func, scx_bpf_get_online_cpumask)
>>>> Missing KF_ACQUIRE
>>>>> +BTF_ID_FLAGS(func, scx_bpf_put_cpumask)
>>>> Missing KF_RELEASE
>>>>> +BTF_ID_FLAGS(func, scx_bpf_select_cpu_dfl)
>>>> Missing KF_IMPLICIT_ARGS | KF_RCU
>>>>> +BTF_ID_FLAGS(func, __scx_bpf_select_cpu_and)
>>>> Missing KF_IMPLICIT_ARGS | KF_RCU
>>>>> +BTF_ID_FLAGS(func, scx_bpf_select_cpu_and)
>>>> Missing KF_RCU
>>>>
>>>> Please correct me if I miss something.
>>>>> +BTF_ID_FLAGS(func, scx_bpf_get_idle_cpumask)
>>>>> +BTF_ID_FLAGS(func, scx_bpf_get_idle_cpumask_node)
>>>>> +BTF_ID_FLAGS(func, scx_bpf_get_idle_smtmask)
>>>>> +BTF_ID_FLAGS(func, scx_bpf_get_idle_smtmask_node)
>>>>> +BTF_ID_FLAGS(func, scx_bpf_put_idle_cpumask)
>>>>> +BTF_ID_FLAGS(func, scx_bpf_test_and_clear_cpu_idle)
>>>>> +BTF_ID_FLAGS(func, scx_bpf_pick_idle_cpu)
>>>>> +BTF_ID_FLAGS(func, scx_bpf_pick_idle_cpu_node)
>>>>> +BTF_ID_FLAGS(func, scx_bpf_pick_any_cpu)
>>>>> +BTF_ID_FLAGS(func, scx_bpf_pick_any_cpu_node)
>>>>> +BTF_KFUNCS_END(scx_kfunc_ids_cpu_only)
>>>>> +
>>>>>  /*
>>>>>   * Per-op kfunc allow flags. Each bit corresponds to a context-sensitive kfunc
>>>>>   * group; an op may permit zero or more groups, with the union expressed in
>>>>> @@ -10031,6 +10067,7 @@ int scx_kfunc_context_filter(const struct bpf_prog *prog, u32 kfunc_id)
>>>>>  	bool in_cpu_release = btf_id_set8_contains(&scx_kfunc_ids_cpu_release, kfunc_id);
>>>>>  	bool in_idle = btf_id_set8_contains(&scx_kfunc_ids_idle, kfunc_id);
>>>>>  	bool in_any = btf_id_set8_contains(&scx_kfunc_ids_any, kfunc_id);
>>>>> +	bool in_cpu_only = btf_id_set8_contains(&scx_kfunc_ids_cpu_only, kfunc_id);
>>>>>  	u32 moff, flags;
>>>>>  
>>>>>  	/* Not an SCX kfunc - allow. */
>>>>> @@ -10068,6 +10105,15 @@ int scx_kfunc_context_filter(const struct bpf_prog *prog, u32 kfunc_id)
>>>>>  	    prog->aux->st_ops != &bpf_sched_ext_ops_cid)
>>>>>  		return -EACCES;
>>>>>  
>>>>> +	/*
>>>>> +	 * cid-form schedulers must use cid/cmask kfuncs. cid and cpu are both
>>>>> +	 * small s32s and trivially confused, so cpu-only kfuncs are rejected at
>>>>> +	 * load time. The reverse (cpu-form calling cid-form kfuncs) is
>>>>> +	 * intentionally permissive to ease gradual cpumask -> cid migration.
>>>>> +	 */
>>>>> +	if (prog->aux->st_ops == &bpf_sched_ext_ops_cid && in_cpu_only)
>>>>> +		return -EACCES;
>>>>> +
>>>>>  	/* SCX struct_ops: check the per-op allow list. */
>>>>>  	if (in_any || in_idle)
>>>>>  		return 0;
>>>>> -- 
>>>>> 2.53.0
>>>>>
>>>>>
>>>>
>>>>
>>>>
>>>
>>> -- 
>>> Cheers,
>>> Cheng-Yang
>>>
>>
>>
>