Re: cgroup specific sticky resources (was: Re: [PATCH bpf-next 0/5] bpf: BPF specific memory allocator.)

[Mina Almasry <almasrymina@google.com>]

On Tue, Jul 19, 2022 at 12:54 PM Tejun Heo <tj@kernel.org> wrote:
>
> Hello,
>
> On Tue, Jul 19, 2022 at 12:47:39PM -0700, Mina Almasry wrote:
> > Hmm, sorry I might be missing something but I don't think we have the
> > same thing in mind?
> >
> > My understanding is that the sysadmin can do something like this which
> > is relatively inexpensive to implement in the kernel:
> >
> >
> > mount -t tmpfs /mnt/mymountpoint
> > echo "/mnt/mymountpoint" > /path/to/cgroup/cgroup.charge_for.tmpfs
> >
> >
> > At that point all tmpfs charges for this tmpfs are directed to
> > /path/to/cgroup/memory.current.
> >
> > Then the sysadmin can do something like:
> >
> >
> > echo "/mnt/mymountpoint" > /path/to/cgroup2/cgroup.charge_for.tmpfs
> >
> >
> > At that point all _future_ charges of that tmpfs will go to
> > cgroup2/memory.current. All existing charges remain at
> > cgroup/memory.current and get uncharged from there. Per my
> > understanding there is no need to move all the _existing_ charges from
> > cgroup/memory.current to cgroup2/memory.current.
>
> So, it's a lot better if the existing charges aren't moved around but it's
> also kinda confusing if something can be moved around the tree arbitrarily
> leaving charges behind. We already do get that from moving processes around
> but most common usages are pretty static at this point and I think it'd be
> better to avoid expanding the interface in that direction.
>

I think I'm flexible in this sense. Would you like the kernel to
prevent reattaching the tmpfs to a different cgroup? To be honest we
have a use case for that, but I'm not going to die on this hill. I
guess the worst case scenario is that I can carry a local patch on our
kernel which allows reattaching to a different cgroup and directs
future charges there...

> I'd much prefer something alont the line of `mount -t tmpfs -o cgroup=XXX`
> where the tmpfs code checks whether the specified cgroup is one of the
> ancestors and the mounting task has enough permission to shift the resource
> there.
>

Actually this is pretty much the same interface I opted for in my
original proposal (except I named it memcg= rather than cgroup=):
https://lore.kernel.org/linux-mm/20211120045011.3074840-1-almasrymina@google.com/

Curious, why do we need to check if the cgroup= is an ancestor? We
actually do have a use case where the cgroups are unrelated and the
common ancestor is root. Again, I'm not sure I want to die on this
hill. At worst I can remove the restriction in a local patch for our
kernel again...

Before I get too excited and implement these changes and submit
another iteration of my proposal above, I'd love to hear from
Johannes/Michal/Roman. My previous proposal was a pretty strong nack
from Johannes and Michal in particular.

> Thanks.
>
> --
> tejun