Re: [PATCH] mm/bpf_memcontrol: mark BPF memcg kfuncs static

[JP Kobryn <jp.kobryn@linux.dev>]

On 6/19/26 8:27 PM, Alexei Starovoitov wrote:
> On Wed Jun 17, 2026 at 3:31 PM PDT, Roman Gushchin wrote:
>> Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
>>
>>> On Wed, Jun 17, 2026 at 1:22 PM JP Kobryn <jp.kobryn@linux.dev> wrote:
>>>>
>>>> The kfuncs in bpf_memcontrol.c are not called by in-kernel C code. They are
>>>> only referenced by BPF programs and resolved through BTF.
>>>>
>>>> Since no external linkage is needed, mark these functions static.
>>>>
>>>> Fixes: 5904db9891f8 ("mm: introduce BPF kfuncs to deal with memcg pointers")
>>>> Fixes: 5c7db3239c9f ("mm: introduce bpf_get_root_mem_cgroup() BPF kfunc")
>>>> Fixes: 99430ab8b804 ("mm: introduce BPF kfuncs to access memcg statistics and events")
>>>> Reported-by: kernel test robot <lkp@intel.com>
>>>> Closes: https://lore.kernel.org/oe-kbuild-all/202606150332.Xy4Egd9s-lkp@intel.com/
>>>> Signed-off-by: JP Kobryn <jp.kobryn@linux.dev>
>>>> ---
>>>>  mm/bpf_memcontrol.c | 21 +++++++++++----------
>>>>  1 file changed, 11 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/mm/bpf_memcontrol.c b/mm/bpf_memcontrol.c
>>>> index 716df49d7647..eff79bb2c758 100644
>>>> --- a/mm/bpf_memcontrol.c
>>>> +++ b/mm/bpf_memcontrol.c
>>>> @@ -20,7 +20,7 @@ __bpf_kfunc_start_defs();
>>>>   *
>>>>   * Return: A pointer to the root memory cgroup.
>>>>   */
>>>> -__bpf_kfunc struct mem_cgroup *bpf_get_root_mem_cgroup(void)
>>>> +__bpf_kfunc static struct mem_cgroup *bpf_get_root_mem_cgroup(void)
>>>
>>> sashiko disappoints. It couldn't notice that kfuncs should not be static.
>>> We have few "__bpf_kfunc static" in net/ipv4/
>>> that work by sort-of "luck", since their addresses are taken.
>>>
>>> In general kfuncs cannot be declared static.
>>
>> It seems like it thought hard on it but failed based on the existing
>> code and a bit vague documentation in Documentation/bpf/kfuncs.rst:
>>
>>   kfunc definitions should also always be annotated with the ``__bpf_kfunc``
>>   macro. This prevents issues such as the compiler inlining the kfunc if it's a
>>   static kernel function, or the function ...
>>
>> So it reads like bpf kfuncs can be static.
>>
>> This is the reasoning from the logs (a dismissed concern, to be precise):
>> {
>>       "type": "Dead Code / Linker Error",
>>
>>       "description": "Marking kfuncs as static could cause the compiler
>>       to optimize them away, breaking BPF verifier BTF resolution.",
>>
>>       "reasoning": "The patch adds `static` to multiple `__bpf_kfunc`
>>       annotated functions that are not called anywhere in C
>>       code. Normally, a compiler would remove unused static functions or
>>       fail to expose them in object files, which could prevent the BTF
>>       ID generation logic (`resolve_btfids`) from finding them. However,
>>       the `__bpf_kfunc` macro expands to include `__used` and
>>       `__retain`, which strictly instructs the compiler and linker to
>>       preserve the functions in the object file regardless of
>>       usage. Additionally, `resolve_btfids` and `BTF_ID_FLAGS`
>>       successfully parse and resolve `STB_LOCAL` static symbols, making
>>       this change completely safe and correct.",
> 
> Almost, except it's not clear what takes priority for the compiler.
> __bpf_kfunc used to have '__used' and it was enough until clang
> got too aggressive in LTO mode. So we added '__retain' and so far
> it seems to be holding, but sashiko is missing that both of these
> attributes don't say anything about _name_ of the function.
> 'static' keyword means that compiler is free to rename it and
> compilers do take advantage of that from time to time,
> while __used and __retain don't add 'do-not-rename' restriction
> to the compiler.
> 
> Also look at it from the other angle.. why anyone would add
> 'static' when the function is clearly not static to this particular
> compilation unit. It will be used outside of it by bpf progs
> that are obviuosly not part of this .c file.
> 
> Documentation/bpf/kfuncs.rst needs to be updated.
> 
> JP,
> pls send a patch to update the docs and include few words
> to ignore sparse.
> 

Sounds good. I'll get that out this week.