On Tue, Jul 19, 2022 at 12:54 PM Tejun Heo <tj@xxxxxxxxxx> 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@xxxxxxxxxx/ 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