Re: [PATCH v10 3/3] mm: add anonymous vma name refcounting
From: Suren Baghdasaryan <surenb@google.com>
Date: 2021-10-07 02:47:12
Also in:
linux-doc, linux-fsdevel, lkml
On Wed, Oct 6, 2021 at 7:29 PM Andrew Morton [off-list ref] wrote:
On Wed, 6 Oct 2021 08:20:20 -0700 Suren Baghdasaryan [off-list ref] wrote:quoted
On Wed, Oct 6, 2021 at 8:08 AM David Hildenbrand [off-list ref] wrote:quoted
On 06.10.21 17:01, Suren Baghdasaryan wrote:quoted
On Wed, Oct 6, 2021 at 2:27 AM David Hildenbrand [off-list ref] wrote:quoted
On 06.10.21 10:27, Michal Hocko wrote:quoted
On Tue 05-10-21 23:57:36, John Hubbard wrote: [...]quoted
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 informationYeah, I should have described it clearer the first time around.If it gets this fancy then the 80 char limit is likely to become a significant limitation and the choice should be explained & justified. Why not 97? 1034? Why not just strndup_user() and be done with it?
The original patch from 8 years ago used 256 as the limit but Rasmus argued that the string content should be human-readable, so 80 chars seems to be a reasonable limit (see: https://lore.kernel.org/all/d8619a98-2380-ca96-001e-60fe9c6204a6@rasmusvillemoes.dk (local)), which makes sense to me. We should be able to handle the 80 char limit by trimming it before calling prctl().
quoted
quoted
My question would be, if we really have to expose these strings to the kernel, or if an id is sufficient. Sure, it would move complexity to user space, but keeping complexity out of the kernel is usually a good idea.My worry here is not the additional complexity on the userspace side but the performance hit we would have to endure due to these conversions.Has the performance hit been quantified?
I'll try to get the data that was collected or at least an estimate. I imagine collecting such data would require considerable userspace redesign.
I've seen this many times down the ages. Something which *could* be done in userspace is instead done in the kernel because coordinating userspace is Just So Damn Hard. I guess the central problem is that userspace isn't centrally coordinated. I wish we were better at this.
It's not just hard, it's also inefficient. And for our usecase performance is important.
-- To unsubscribe from this group and stop receiving emails from it, send an email to kernel-team+unsubscribe@android.com.