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. > > > for whatever reason is determining the pinning ownership or should the page > > > ownership be attributed the same way too? If they indeed need to differ, > > > that probably would need pretty strong justifications. > > > > It is inherent to how pin_user_pages() works. It is an API that > > establishs pins on existing pages. There is nothing about it that says > > who the page's memcg owner is. > > > > I don't think we can do anything about this without breaking things. > > That's a discrepancy in an internal interface and we don't wanna codify > something like that into userspace interface. Semantially, it seems like if > pin_user_pages() wanna charge pinning to the cgroup associated with an fd > (or whatever), it should also claim the ownership of the pages > themselves. Multiple cgroup can pin the same page, so it is not as simple as just transfering ownership, we need multi-ownership and to really fix the memcg limitations with MAP_SHARED without an API impact. You are right that pinning is really just a special case of allocation, but there is a reason the memcg was left with weak support for MAP_SHARED and changing that may be more than just hard but an infeasible trade off.. At least I don't have a good idea how to even approach building a reasonable datstructure that can track the number of charges per-cgroup per page. :\ Jason
