On Thu, Oct 7, 2021 at 12:33 AM David Hildenbrand <david@xxxxxxxxxx> wrote: > > On 06.10.21 17:20, Suren Baghdasaryan wrote: > > On Wed, Oct 6, 2021 at 8:08 AM David Hildenbrand <david@xxxxxxxxxx> wrote: > >> > >> On 06.10.21 17:01, Suren Baghdasaryan wrote: > >>> On Wed, Oct 6, 2021 at 2:27 AM David Hildenbrand <david@xxxxxxxxxx> wrote: > >>>> > >>>> On 06.10.21 10:27, Michal Hocko wrote: > >>>>> On Tue 05-10-21 23:57:36, John Hubbard wrote: > >>>>> [...] > >>>>>> 1) Yes, just leave the strings in the kernel, that's simple and > >>>>>> it works, and the alternatives don't really help your case nearly > >>>>>> enough. > >>>>> > >>>>> I do not have a strong opinion. Strings are easier to use but they > >>>>> are more involved and the necessity of kref approach just underlines > >>>>> that. There are going to be new allocations and that always can lead > >>>>> to surprising side effects. These are small (80B at maximum) so the > >>>>> overall footpring shouldn't all that large by default but it can grow > >>>>> quite large with a very high max_map_count. There are workloads which > >>>>> really require the default to be set high (e.g. heavy mremap users). So > >>>>> if anything all those should be __GFP_ACCOUNT and memcg accounted. > >>>>> > >>>>> I do agree that numbers are just much more simpler from accounting, > >>>>> performance and implementation POV. > >>>> > >>>> +1 > >>>> > >>>> I can understand that having a string can be quite beneficial e.g., when > >>>> dumping mmaps. If only user space knows the id <-> string mapping, that > >>>> can be quite tricky. > >>>> > >>>> However, I also do wonder if there would be a way to standardize/reserve > >>>> ids, such that a given id always corresponds to a specific user. If we > >>>> use an uint64_t for an id, there would be plenty room to reserve ids ... > >>>> > >>>> I'd really prefer if we can avoid using strings and instead using ids. > >>> > >>> I wish it was that simple and for some names like [anon:.bss] or > >>> [anon:dalvik-zygote space] reserving a unique id would work, however > >>> some names like [anon:dalvik-/system/framework/boot-core-icu4j.art] > >>> are generated dynamically at runtime and include package name. > >> > >> Valuable information > > > > Yeah, I should have described it clearer the first time around. > > > >> > >>> Packages are constantly evolving, new ones are developed, names can > >>> change, etc. So assigning a unique id for these names is not really > >>> feasible. > >> > >> So, you'd actually want to generate/reserve an id for a given string at > >> runtime, assign that id to the VMA, and have a way to match id <-> > >> string somehow? > > > > If we go with ids then yes, that is what we would have to do. > > > >> That reservation service could be inside the kernel or even (better?) in > >> user space. The service could for example de-duplicates strings. > > > > Yes but it would require an IPC call to that service potentially on > > every mmap() when we want to name a mapped vma. This would be > > prohibitive. Even on consumption side, instead of just dumping > > /proc/$pid/maps we would have to parse the file and convert all > > [anon:id] into [anon:name] with each conversion requiring an IPC call > > (assuming no id->name pair caching on the client side). > > mmap() and prctl() already do take the mmap sem in write, so they are > not the "most lightweight" operations so to say. > > We already to have two separate operations, first the mmap(), then the > prctl(). IMHO you could defer the "naming" part to a later point in > time, without creating too many issues, moving it out of the > "hot/performance critical phase" > > Reading https://lwn.net/Articles/867818/, to me it feels like the use > case could live with a little larger delay between the mmap popping up > and a name getting assigned. That might be doable if occasional inconsistency can be tolerated (we can't guarantee that maps won't be read before the deferred work name the vma). However I would prefer an efficient solution vs the one which is inefficient but can be deferred. > > -- > Thanks, > > David / dhildenb >
