Re: [PATCH v12 mm-new 06/10] mm: bpf-thp: add support for global mode

[Alexei Starovoitov <alexei.starovoitov@gmail.com>]

On Tue, Oct 28, 2025 at 7:14 PM Yafang Shao <laoar.shao@gmail.com> wrote:
>
> On Wed, Oct 29, 2025 at 9:33 AM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > On Sun, Oct 26, 2025 at 3:03 AM Yafang Shao <laoar.shao@gmail.com> wrote:
> > >
> > > The per-process BPF-THP mode is unsuitable for managing shared resources
> > > such as shmem THP and file-backed THP. This aligns with known cgroup
> > > limitations for similar scenarios [0].
> > >
> > > Introduce a global BPF-THP mode to address this gap. When registered:
> > > - All existing per-process instances are disabled
> > > - New per-process registrations are blocked
> > > - Existing per-process instances remain registered (no forced unregistration)
> > >
> > > The global mode takes precedence over per-process instances. Updates are
> > > type-isolated: global instances can only be updated by new global
> > > instances, and per-process instances by new per-process instances.
> >
> > ...
> >
> > >         spin_lock(&thp_ops_lock);
> > > -       /* Each process is exclusively managed by a single BPF-THP. */
> > > -       if (rcu_access_pointer(mm->bpf_mm.bpf_thp)) {
> > > +       /* Each process is exclusively managed by a single BPF-THP.
> > > +        * Global mode disables per-process instances.
> > > +        */
> > > +       if (rcu_access_pointer(mm->bpf_mm.bpf_thp) || rcu_access_pointer(bpf_thp_global)) {
> > >                 err = -EBUSY;
> > >                 goto out;
> > >         }
> >
> > You didn't address the issue and instead doubled down
> > on this broken global approach.
> >
> > This bait-and-switch patchset is frankly disingenuous.
> > 'lets code up some per-mm hack, since people will hate it anyway,
> > and I'm not going to use it either, and add this global mode
> > as a fake "fallback"...'
> >
> > The way the previous thread evolved and this followup hack
> > I don't see a genuine desire to find a solution.
> > Just relentless push for global mode.
> >
> > Nacked-by: Alexei Starovoitov <ast@kernel.org>
> >
> > Please carry it in all future patches.
>
> To move forward, I'm happy to set the global mode aside for now and
> potentially drop it in the next version. I'd really like to hear your
> perspective on the per-process mode. Does this implementation meet
> your needs?

Attaching st_ops to task_struct or to mm_struct is a can of worms.
With cgroup-bpf we went through painful bugs with lifetime
of cgroup vs bpf, dying cgroups, wq deadlock, etc. All these
problems are behind us. With st_ops in mm_struct it will be more
painful. I'd rather not go that route.

And revist cgroup instead, since you were way too quick
to accept the pushback because all you wanted is global mode.

The main reason for pushback was:
"
Cgroup was designed for resource management not for grouping processes and
tune those processes
"

which was true when cgroup-v2 was designed, but that ship sailed
years ago when we introduced cgroup-bpf.
None of the progs are doing resource management and lots of infrastructure,
container management, and open source projects use cgroup-bpf
as a grouping of processes. bpf progs attached to cgroup/hook tuple
only care about processes within that cgroup. No resource management.
See __cgroup_bpf_check_dev_permission or __cgroup_bpf_run_filter_sysctl
and others.
The path is current->cgroup->bpf_progs and progs do exactly
what cgroup wasn't designed to do. They tune a set of processes.

You should do the same.

Also I really don't see a compelling use case for bpf in THP.
Your selftest is beyond primitive:
+int pmd_order;
+
+SEC("struct_ops/thp_get_order")
+int BPF_PROG(thp_not_eligible, struct vm_area_struct *vma, enum tva_type type,
+     unsigned long orders)
+{
+ /* THPeligible in /proc/pid/smaps is 0 */
+ if (type == TVA_SMAPS)
+ return 0;
+ return pmd_order;
+}
hard code this thing. Don't bother with bpf.