| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20211001205657.815551-3-surenb@google.com>
Date: Fri, 1 Oct 2021 13:56:57 -0700
From: Suren Baghdasaryan <surenb@...gle.com>
To: akpm@...ux-foundation.org
Cc: ccross@...gle.com, sumit.semwal@...aro.org, mhocko@...e.com,
dave.hansen@...el.com, keescook@...omium.org, willy@...radead.org,
kirill.shutemov@...ux.intel.com, vbabka@...e.cz,
hannes@...xchg.org, corbet@....net, viro@...iv.linux.org.uk,
rdunlap@...radead.org, kaleshsingh@...gle.com, peterx@...hat.com,
rppt@...nel.org, peterz@...radead.org, catalin.marinas@....com,
vincenzo.frascino@....com, chinwen.chang@...iatek.com,
axelrasmussen@...gle.com, aarcange@...hat.com, jannh@...gle.com,
apopple@...dia.com, jhubbard@...dia.com, yuzhao@...gle.com,
will@...nel.org, fenghua.yu@...el.com, thunder.leizhen@...wei.com,
hughd@...gle.com, feng.tang@...el.com, jgg@...pe.ca, guro@...com,
tglx@...utronix.de, krisman@...labora.com, chris.hyser@...cle.com,
pcc@...gle.com, ebiederm@...ssion.com, axboe@...nel.dk,
legion@...nel.org, eb@...ix.com, gorcunov@...il.com, pavel@....cz,
songmuchun@...edance.com, viresh.kumar@...aro.org,
thomascedeno@...gle.com, sashal@...nel.org, cxfcosmos@...il.com,
linux@...musvillemoes.dk, linux-kernel@...r.kernel.org,
linux-fsdevel@...r.kernel.org, linux-doc@...r.kernel.org,
linux-mm@...ck.org, kernel-team@...roid.com, surenb@...gle.com
Subject: [PATCH v10 3/3] mm: add anonymous vma name refcounting
While forking a process with high number (64K) of named anonymous vmas the
overhead caused by strdup() is noticeable. Experiments with ARM64 Android
device show up to 40% performance regression when forking a process with
64k unpopulated anonymous vmas using the max name lengths vs the same
process with the same number of anonymous vmas having no name.
Introduce anon_vma_name refcounted structure to avoid the overhead of
copying vma names during fork() and when splitting named anonymous vmas.
When a vma is duplicated, instead of copying the name we increment the
refcount of this structure. Multiple vmas can point to the same
anon_vma_name as long as they increment the refcount. The name member of
anon_vma_name structure is assigned at structure allocation time and is
never changed. If vma name changes then the refcount of the original
structure is dropped, a new anon_vma_name structure is allocated
to hold the new name and the vma pointer is updated to point to the new
structure.
With this approach the fork() performance regressions is reduced 3-4x
times and with usecases using more reasonable number of VMAs (a few
thousand) the regressions is not measurable.
Signed-off-by: Suren Baghdasaryan <surenb@...gle.com>
Reviewed-by: Kees Cook <keescook@...omium.org>
---
previous version at:
https://lore.kernel.org/linux-mm/20210902231813.3597709-3-surenb@google.com/
changes in v10
- Changed anon_vma_name_alloc() to use struct_size() and avoid strcpy(),
per Kees Cook
include/linux/mm_types.h | 9 ++++++++-
mm/madvise.c | 42 ++++++++++++++++++++++++++++++++++------
2 files changed, 44 insertions(+), 7 deletions(-)
diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index f1896c9b2da9..30c571b4f8d3 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -5,6 +5,7 @@
#include <linux/mm_types_task.h>
#include <linux/auxvec.h>
+#include <linux/kref.h>
#include <linux/list.h>
#include <linux/spinlock.h>
#include <linux/rbtree.h>
@@ -310,6 +311,12 @@ struct vm_userfaultfd_ctx {
struct vm_userfaultfd_ctx {};
#endif /* CONFIG_USERFAULTFD */
+struct anon_vma_name {
+ struct kref kref;
+ /* The name needs to be at the end because it is dynamically sized. */
+ char name[];
+};
+
/*
* This struct describes a virtual memory area. There is one of these
* per VM-area/task. A VM area is any part of the process virtual memory
@@ -361,7 +368,7 @@ struct vm_area_struct {
unsigned long rb_subtree_last;
} shared;
/* Serialized by mmap_sem. */
- char *anon_name;
+ struct anon_vma_name *anon_name;
};
/*
diff --git a/mm/madvise.c b/mm/madvise.c
index 69729930dde1..2f1ec13faaaa 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -63,6 +63,29 @@ static int madvise_need_mmap_write(int behavior)
}
}
+static struct anon_vma_name *anon_vma_name_alloc(const char *name)
+{
+ struct anon_vma_name *anon_name;
+ size_t count;
+
+ /* Add 1 for NUL terminator at the end of the anon_name->name */
+ count = strlen(name) + 1;
+ anon_name = kmalloc(struct_size(anon_name, name, count), GFP_KERNEL);
+ if (anon_name) {
+ kref_init(&anon_name->kref);
+ memcpy(anon_name->name, name, count);
+ }
+
+ return anon_name;
+}
+
+static void vma_anon_name_free(struct kref *kref)
+{
+ struct anon_vma_name *anon_name =
+ container_of(kref, struct anon_vma_name, kref);
+ kfree(anon_name);
+}
+
static inline bool has_vma_anon_name(struct vm_area_struct *vma)
{
return !vma->vm_file && vma->anon_name;
@@ -75,7 +98,7 @@ const char *vma_anon_name(struct vm_area_struct *vma)
mmap_assert_locked(vma->vm_mm);
- return vma->anon_name;
+ return vma->anon_name->name;
}
void dup_vma_anon_name(struct vm_area_struct *orig_vma,
@@ -84,34 +107,41 @@ void dup_vma_anon_name(struct vm_area_struct *orig_vma,
if (!has_vma_anon_name(orig_vma))
return;
- new_vma->anon_name = kstrdup(orig_vma->anon_name, GFP_KERNEL);
+ kref_get(&orig_vma->anon_name->kref);
+ new_vma->anon_name = orig_vma->anon_name;
}
void free_vma_anon_name(struct vm_area_struct *vma)
{
+ struct anon_vma_name *anon_name;
+
if (!has_vma_anon_name(vma))
return;
- kfree(vma->anon_name);
+ anon_name = vma->anon_name;
vma->anon_name = NULL;
+ kref_put(&anon_name->kref, vma_anon_name_free);
}
/* mmap_lock should be write-locked */
static int replace_vma_anon_name(struct vm_area_struct *vma, const char *name)
{
+ const char *anon_name;
+
if (!name) {
free_vma_anon_name(vma);
return 0;
}
- if (vma->anon_name) {
+ anon_name = vma_anon_name(vma);
+ if (anon_name) {
/* Same name, nothing to do here */
- if (!strcmp(name, vma->anon_name))
+ if (!strcmp(name, anon_name))
return 0;
free_vma_anon_name(vma);
}
- vma->anon_name = kstrdup(name, GFP_KERNEL);
+ vma->anon_name = anon_vma_name_alloc(name);
if (!vma->anon_name)
return -ENOMEM;
--
2.33.0.800.g4c38ced690-goog
Powered by blists - more mailing lists