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

[Tejun Heo <tj@kernel.org>]

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.

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.

- 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.

- 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.

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.

Thanks.

--
tejun