Thread (29 messages) 29 messages, 8 authors, 2021-09-03

Re: [PATCH v8 2/3] mm: add a field to store names for private anonymous memory

From: Suren Baghdasaryan <surenb@google.com>
Date: 2021-08-28 21:53:36
Also in: linux-fsdevel, linux-mm, lkml

On Sat, Aug 28, 2021 at 2:28 PM Cyrill Gorcunov [off-list ref] wrote:
On Fri, Aug 27, 2021 at 12:18:57PM -0700, Suren Baghdasaryan wrote:
quoted
The name is stored in a pointer in the shared union in vm_area_struct
that points to a null terminated string. Anonymous vmas with the same
name (equivalent strings) and are otherwise mergeable will be merged.
The name pointers are not shared between vmas even if they contain the
same name. The name pointer is stored in a union with fields that are
only used on file-backed mappings, so it does not increase memory usage.

The patch is based on the original patch developed by Colin Cross, more
specifically on its latest version [1] posted upstream by Sumit Semwal.
It used a userspace pointer to store vma names. In that design, name
pointers could be shared between vmas. However during the last upstreaming
attempt, Kees Cook raised concerns [2] about this approach and suggested
to copy the name into kernel memory space, perform validity checks [3]
and store as a string referenced from vm_area_struct.
One big concern is about fork() performance which would need to strdup
anonymous vma names. Dave Hansen suggested experimenting with worst-case
scenario of forking a process with 64k vmas having longest possible names
[4]. I ran this experiment on an ARM64 Android device and recorded a
worst-case regression of almost 40% when forking such a process. This
regression is addressed in the followup patch which replaces the pointer
to a name with a refcounted structure that allows sharing the name pointer
between vmas of the same name. Instead of duplicating the string during
fork() or when splitting a vma it increments the refcount.

[1] https://lore.kernel.org/linux-mm/20200901161459.11772-4-sumit.semwal@linaro.org/ (local)
[2] https://lore.kernel.org/linux-mm/202009031031.D32EF57ED@keescook/ (local)
[3] https://lore.kernel.org/linux-mm/202009031022.3834F692@keescook/ (local)
[4] https://lore.kernel.org/linux-mm/5d0358ab-8c47-2f5f-8e43-23b89d6a8e95@intel.com/ (local)
...
quoted
+
+/* mmap_lock should be read-locked */
+static inline bool is_same_vma_anon_name(struct vm_area_struct *vma,
+                                      const char *name)
+{
+     const char *vma_name = vma_anon_name(vma);
+
+     if (likely(!vma_name))
+             return name == NULL;
+
+     return name && !strcmp(name, vma_name);
+}
Hi Suren! There is very important moment with this new feature: if
we assign a name to some VMA it won't longer be mergeable even if
near VMA matches by all other attributes such as flags, permissions
and etc. I mean our vma_merge() start considering the vma namings
and names mismatch potentially blocks merging which happens now
without this new feature. Is it known behaviour or I miss something
pretty obvious here?
Hi Cyrill,
Correct, this is a known drawback of naming an anonymous VMA. I think
I'll need to document this in prctl(2) manpage, which I should update
to include this new PR_SET_VMA_ANON_NAME option.
Thanks for pointing it out!
Suren.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help