On Tue, Feb 21, 2023 at 09:45:15AM -1000, Tejun Heo wrote: > Hello, > > On Tue, Feb 21, 2023 at 03:26:33PM -0400, Jason Gunthorpe wrote: > > On Tue, Feb 21, 2023 at 08:07:13AM -1000, Tejun Heo wrote: > > > > AFAIK there are few real use cases to establish a pin on MAP_SHARED > > > > mappings outside your cgroup. However, it is possible, the APIs allow > > > > it, and for security sandbox purposes we can't allow a process inside > > > > a cgroup to triger a charge on a different cgroup. That breaks the > > > > sandbox goal. > > > > > > It seems broken anyway. Please consider the following scenario: > > > > Yes, this is broken like this already today - memcg doesn't work > > entirely perfectly for MAP_SHARED scenarios, IMHO. > > It is far from perfect but the existing behavior isn't that broken. e.g. in > the same scenario, without pinning, even if the larger cgroup keeps using > the same page, the smaller cgroup should be able to evict the pages as they > are not pinned and the cgroup is under heavy reclaim pressure. The larger > cgroup will refault them back in and end up owning those pages. > > memcg can't capture the case of the same pages being actively shared by > multiple cgroups concurrently (I think those cases should be handled by > pushing them to the common parent as discussed elswhere but that's a > separate topic) but it can converge when page usage transfers across cgroups > if needed. Disassociating ownership and pinning will break that in an > irreversible way. It is already disassociated - memcg is broken as you describe today with pin_user_pages(). If you want to fix that, then we need to redefine how memcg works with pin_user_pages() and I'm open to ideas.. > the owner and pinner disagreeing with each other. At least > conceptually, the solution is rather straight-forward - whoever pins > a page should also claim the ownership of it. If the answer is pinner is owner, then multi-pinners must mean multi-owner too. We probably can't block multi-pinner without causing uAPI problems. You are not wrong on any of these remarks, but this looses sight of the point - it is take the existing broken RLIMIT scheme and make it incrementally better by being the same broken scheme just with cgroups. If we eventually fix everything so memcg can do multi-pinners/owners then would it be reasonable to phase out the new pincg at that time? Jason
