Re: [PATCH v10 3/3] mm: add anonymous vma name refcounting

[Suren Baghdasaryan <surenb@google.com>]

On Mon, Oct 11, 2021 at 1:36 AM Michal Hocko <mhocko@suse.com> wrote:
>
> On Fri 08-10-21 13:58:01, Kees Cook wrote:
> > - Strings for "anon" specifically have no required format (this is good)
> >   it's informational like the task_struct::comm and can (roughly)
> >   anything. There's no naming convention for memfds, AF_UNIX, etc. Why
> >   is one needed here? That seems like a completely unreasonable
> >   requirement.
>
> I might be misreading the justification for the feature. Patch 2 is
> talking about tools that need to understand memeory usage to make
> further actions. Also Suren was suggesting "numbering convetion" as an
> argument against.
>
> So can we get a clear example how is this being used actually? If this
> is just to be used to debug by humans than I can see an argument for
> human readable form. If this is, however, meant to be used by tools to
> make some actions then the argument for strings is much weaker.

The simplest usecase is when we notice that a process consumes more
memory than usual and we do "cat /proc/$(pidof my_process)/maps" to
check which area is contributing to this growth. The names we assign
to anonymous areas are descriptive enough for a developer to get an
idea where the increased consumption is coming from and how to proceed
with their investigation.
There are of course cases when tools are involved, but the end-user is
always a human and the final report should contain easily
understandable data.

IIUC, the main argument here is whether the userspace can provide
tools to perform the translations between ids and names, with the
kernel accepting and reporting ids instead of strings. Technically
it's possible, but to be practical that conversion should be fast
because we will need to make name->id conversion potentially for each
mmap. On the consumer side the performance is not as critical, but the
fact that instead of dumping /proc/$pid/maps we will have to parse the
file, do id->name conversion and replace all [anon:id] with
[anon:name] would be an issue when we do that in bulk, for example
when collecting system-wide data for a bugreport.

I went ahead and implemented the proposed userspace solution involving
tmpfs as a repository for name->id mapping (more precisely
filename->inode mapping). Profiling shows that open()+fstat()+close()
takes:
- roughly 15 times longer than mmap() with 1000 unique names each
being reused 50 times.
- roughly 3 times longer than mmap() with 100 unique names each being
reused 500 times. This is due to lstat() optimization suggested by
Rasmus which avoids open() and close().
For comparison, proposed prctl() takes roughly the same amount of time
as mmap() and does not depend on the number of unique names.

I'm still evaluating the proposal to use memfds but I'm not sure if
the issue that David Hildenbrand mentioned about additional memory
consumed in pagecache (which has to be addressed) is the only one we
will encounter with this approach. If anyone knows of any potential
issues with using memfds as named anonymous memory, I would really
appreciate your feedback before I go too far in that direction.
Thanks,
Suren.

> --
> Michal Hocko
> SUSE Labs