Re: [RFC PATCH bpf-next v4 0/4] BPF-based per-cgroup NUMA balancing mode selection

[Yafang Shao]

On Wed, Nov 11, 2026 at 2:45 AM Tejun Heo <tj@xxxxxxxxxx> wrote:
>
> Hello, Yafang.
>
> I'm not going to engage further in this series until the situation
> changes.
>
> LLMs have made it much cheaper to produce plausible-looking patchsets
> for new features. That makes it easy to keep sending iterations of a
> feature which touches code that the submitter doesn't really understand
> or hasn't thought through in depth. The reviewer ends up doing much of
> the developer's work while the submitter sits between the reviewer and
> whatever the LLM produces. I think this series is an example of that.

Thanks, and I'm sorry for the time this has taken.

>
> Some examples from v4:
>
> - A folio belongs to a memcg and that memcg decides the policies around
>   it. That's how memcg works for every other memory policy, and tasks
>   belong to a memcg too. The series still splits NUMA balancing into a
>   task side and a folio side with different semantics. DISABLED applies
>   to tasks but not to their memory, and the two sides combine the
>   hierarchy differently.
>
>   On v3, I asked why. v4 moves the storage into memcg and keeps the
>   split, but doesn't give a high-level reason why NUMA balancing should
>   work differently from how memcg handles every other memory policy.
>   More generally, the major design decisions come without the reasoning
>   behind them, and asking didn't produce it. Each iteration only barely
>   updates what was pointed out in the previous one.

The task/folio split was my attempt to avoid defining a single owner
for shared folios. That was not a principled design choice.

Shared page-cache accounting in memcg is not a clean ownership model:
pages can remain charged to a memcg after the tasks that used them
have moved on or exited. That limitation is real, but it does not
justify giving tasks and folios different policy semantics, and I
don't currently have a defensible model for the split.

>
> - numab_memcg_mode() walks up to the root memcg and recombines the
>   requests on every call. The callers include every load-balancing
>   candidate in can_migrate_task(), every task migration, every NUMA hint
>   fault and every page migration including compaction. This happens
>   whenever any cgroup has a mode set, although the result changes only
>   when a mode is set or a memcg goes offline.

You're right: the ancestor walk is not an optional optimization. It is
on hot paths, and leaving it for follow-up measurements is not
acceptable.

>
> - numab_memcg_active() reads css->flags once and the comment says that
>   this makes the three state bits describe the same moment. However,
>   nothing synchronizes those flags with the mode. In the lookups, the
>   check is redundant anyway because offlining a memcg or disabling its
>   controller already clears the mode.

For numab_memcg_active(): the technical point is that the check is
only needed where the memcg does not come from a task/folio.

In the task/folio lookups, the memcg is tied to the object being
examined, So the active check is indeed redundant there, and I'll
remove it.

bpf_numab_get_mode() is different: it resolves an arbitrary cgroup id.
There is no task/folio context that pins the policy. So I use
numab_memcg_active() there to skip an unusable memcg and continue with
its parent.

I agree the check should not be in the task/folio lookups.

>
> A developer could cost reviewers time before too, but each round took
> the developer much more effort in comparison. That kept the effort
> balanced between developer and reviewer in a sustainable way, and it
> made sure that the developer learned and improved while working through
> each iteration. LLMs broke that balance.
>
> I don't know what the right solution is, but at this point shepherding
> patchsets like this doesn't make sense for me.

Anyway thanks for taking time into it.


--
Regards
Yafang