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

["David Hildenbrand (Red Hat)" <david@kernel.org>]

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

I haven't had the capacity to follow the evolution of this patch set 
unfortunately, just to comment on some points from my perspective.

First, I agree that the global mode is not what we want, not even as a 
fallback.

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

That's valuable information, thanks. I would have hoped that per-MM 
policies would be easier.

Are there some pointers to explore regarding the "can of worms" you 
mention when it comes to per-MM policies?

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

Also valuable information.

Personally I don't have a preference regarding per-mm or per-cgroup. 
Whatever we can get working reliably. Sounds like cgroup-bpf has sorted 
out most of the mess.

memcg/cgroup maintainers might disagree, but it's probably worth having 
that discussion once again.

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

There is a lot more potential there to write fine-tuned policies that 
thack VMA information into account.

The tests likely reflect what Yafang seems to focus on: IIUC primarily 
enabling+disabling traditional THPs (e.g., 2M) on a per-process basis.

Some of what Yafang might want to achieve could maybe at this point be 
maybe achieved through the prctl(PR_SET_THP_DISABLE) support, including 
extensions we recently added [1].

Systemd support still seems to be in the works [2] for some of that.


[1] https://lwn.net/Articles/1032014/
[2] https://github.com/systemd/systemd/pull/39085

-- 
Cheers

David