Re: Subject: [PATCH bpf-next 2/3] bpf: drop KF_ACQUIRE flag on BPF kfunc bpf_get_root_mem_cgroup()

[Roman Gushchin <roman.gushchin@linux.dev>]

Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:

> On Fri, Jan 16, 2026 at 1:18 PM Roman Gushchin <roman.gushchin@linux.dev> wrote:
>>
>>
>> E.g. in my bpfoom case:
>>
>> SEC("struct_ops.s/handle_out_of_memory")
>> int BPF_PROG(test_out_of_memory, struct oom_control *oc, struct bpf_struct_ops_link *link)
>> {
>>         struct task_struct *task;
>>         struct mem_cgroup *root_memcg = oc->memcg;
>
> And you'll annotate oom_control->memcg with
> BTF_TYPE_SAFE_TRUSTED_OR_NULL ?

Yes.

>
>>         struct mem_cgroup *memcg, *victim = NULL;
>>         struct cgroup_subsys_state *css_pos, *css;
>>         unsigned long usage, max_usage = 0;
>>         unsigned long pagecache = 0;
>>         int ret = 0;
>>
>>         if (root_memcg)
>>                 root_memcg = bpf_get_mem_cgroup(&root_memcg->css);
>
> similar for mem_cgroup and css types ?
> or as BTF_TYPE_SAFE_RCU_OR_NULL ?

css is embedded into memcg, so in theory it doesn't require it.
also bpf_get_mem_cgroup() has acquire semantics, so in my understanding,
it can take non-trusted arguments.

>
>>         else
>>                 root_memcg = bpf_get_root_mem_cgroup();
>>
>>         if (!root_memcg)
>>                 return 0;
>>
>>         css = &root_memcg->css;
>>         if (css && css->cgroup == link->cgroup)
>>                 goto exit;
>>
>>         bpf_rcu_read_lock();
>
> then this is a bug ? and rcu_read_lock needs to move up?

No, rcu read lock is required to protect the iterator within the for
each loop. root_memcg is protected by a bumped ref counter.

>
>>         bpf_for_each(css, css_pos, &root_memcg->css, BPF_CGROUP_ITER_DESCENDANTS_POST) {
>>                 if (css_pos->cgroup->nr_descendants + css_pos->cgroup->nr_dying_descendants)
>>                         continue;
>>
>>                 memcg = bpf_get_mem_cgroup(css_pos);
>>                 if (!memcg)
>>                         continue;
>>
>>                 < ... >
>>
>>                 bpf_put_mem_cgroup(memcg);
>>         }
>>         bpf_rcu_read_unlock();
>>
>>         < ... >
>>
>>         bpf_put_mem_cgroup(victim);
>> exit:
>>         bpf_put_mem_cgroup(root_memcg);
>
> Fair enough.
> Looks like quite a few pieces are still missing for this to work end-to-end,
> but, sure, let's revert back to acquire semantics.

It's close, I hope to send out v3 in few days.
I was rebasing to the latest bpf-next and then noticed that the test is
not working anymore because of this change.

>
> Matt,
> please come with a way to fix a selftest. Introduce test kfunc or
> something.

Thanks!