mm: split out anon_vma declarations to separate header

Commit Message

Arnd Bergmann Dec. 4, 2021, 5:42 p.m. UTC

From: Arnd Bergmann <[e-mail]>

The patch to add anonymous vma names causes a build failure in
some configurations:

include/linux/mm_types.h: In function 'is_same_vma_anon_name':
include/linux/mm_types.h:924:37: error: implicit declaration of function 'strcmp' [-Werror=implicit-function-declaration]
  924 |         return name && vma_name && !strcmp(name, vma_name);
      |                                     ^~~~~~
include/linux/mm_types.h:22:1: note: 'strcmp' is defined in header '<string.h>'; did you forget to '#include <string.h>'?

This should not really be part of linux/mm_types.h in the first
place, as that header is meant to only contain structure defintions
and need a minimum set of indirect includes itself. While the
header clearly includes more than it should at this point, let's
not make it worse by including string.h as well, which would
pull in the expensive (compile-speed wise) fortify-string logic.

Move the new functions to a separate header that is only included
where necessary to avoid bloating linux/mm_types.h further.

Fixes: 52f545eb6dd7 ("mm: add a field to store names for private anonymous memory")
Signed-off-by: Arnd Bergmann <[e-mail]>
---
 fs/proc/task_mmu.c       |  1 +
 fs/userfaultfd.c         |  1 +
 include/linux/anon_vma.h | 55 ++++++++++++++++++++++++++++++++++++++++
 include/linux/mm_types.h | 48 -----------------------------------
 kernel/fork.c            |  1 +
 mm/madvise.c             |  1 +
 mm/mempolicy.c           |  1 +
 mm/mlock.c               |  1 +
 mm/mmap.c                |  1 +
 mm/mprotect.c            |  1 +
 10 files changed, 63 insertions(+), 48 deletions(-)
 create mode 100644 include/linux/anon_vma.h

Comments

Matthew Wilcox Dec. 4, 2021, 11:53 p.m. UTC | #1

On Sat, Dec 04, 2021 at 06:42:17PM +0100, Arnd Bergmann wrote:
> This should not really be part of linux/mm_types.h in the first
> place, as that header is meant to only contain structure defintions
> and need a minimum set of indirect includes itself. While the
> header clearly includes more than it should at this point, let's
> not make it worse by including string.h as well, which would
> pull in the expensive (compile-speed wise) fortify-string logic.
> 
> Move the new functions to a separate header that is only included
> where necessary to avoid bloating linux/mm_types.h further.

We already have an mm_inline.h.  Why do we need a new header file?

Stephen Rothwell Dec. 4, 2021, 11:57 p.m. UTC | #2

Hi Arnd,

On Sat,  4 Dec 2021 18:42:17 +0100 Arnd Bergmann <[e-mail]> wrote:
>

> diff --git a/include/linux/anon_vma.h b/include/linux/anon_vma.h
> new file mode 100644
> index 000000000000..5ce8b5be31ae
> --- /dev/null
> +++ b/include/linux/anon_vma.h
> @@ -0,0 +1,55 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _LINUX_ANON_VMA_H
> +#define _LINUX_ANON_VMA_H
> +
> +#include <linux/mm_types.h>
> +

Shouldn't this also include string.h to fix the original problem?

Patch

diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index e6998652fd67..5b0106afa870 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -1,6 +1,7 @@ 
 // SPDX-License-Identifier: GPL-2.0
 #include <linux/pagewalk.h>
 #include <linux/vmacache.h>
+#include <linux/anon_vma.h>
 #include <linux/hugetlb.h>
 #include <linux/huge_mm.h>
 #include <linux/mount.h>
diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
index 5b2af7b82776..f1d9265e8581 100644
--- a/fs/userfaultfd.c
+++ b/fs/userfaultfd.c
@@ -15,6 +15,7 @@ 
 #include <linux/sched/signal.h>
 #include <linux/sched/mm.h>
 #include <linux/mm.h>
+#include <linux/anon_vma.h>
 #include <linux/mmu_notifier.h>
 #include <linux/poll.h>
 #include <linux/slab.h>
diff --git a/include/linux/anon_vma.h b/include/linux/anon_vma.h
new file mode 100644
index 000000000000..5ce8b5be31ae
--- /dev/null
+++ b/include/linux/anon_vma.h
@@ -0,0 +1,55 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_ANON_VMA_H
+#define _LINUX_ANON_VMA_H
+
+#include <linux/mm_types.h>
+
+#ifdef CONFIG_ANON_VMA_NAME
+/*
+ * mmap_lock should be read-locked when calling vma_anon_name() and while using
+ * the returned pointer.
+ */
+extern const char *vma_anon_name(struct vm_area_struct *vma);
+
+/*
+ * mmap_lock should be read-locked for orig_vma->vm_mm.
+ * mmap_lock should be write-locked for new_vma->vm_mm or new_vma should be
+ * isolated.
+ */
+extern void dup_vma_anon_name(struct vm_area_struct *orig_vma,
+			      struct vm_area_struct *new_vma);
+
+/*
+ * mmap_lock should be write-locked or vma should have been isolated under
+ * write-locked mmap_lock protection.
+ */
+extern void free_vma_anon_name(struct vm_area_struct *vma);
+
+/* 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);
+
+	/* either both NULL, or pointers to same string */
+	if (vma_name == name)
+		return true;
+
+	return name && vma_name && !strcmp(name, vma_name);
+}
+#else /* CONFIG_ANON_VMA_NAME */
+static inline const char *vma_anon_name(struct vm_area_struct *vma)
+{
+	return NULL;
+}
+static inline void dup_vma_anon_name(struct vm_area_struct *orig_vma,
+			      struct vm_area_struct *new_vma) {}
+static inline void free_vma_anon_name(struct vm_area_struct *vma) {}
+static inline bool is_same_vma_anon_name(struct vm_area_struct *vma,
+					 const char *name)
+{
+	return true;
+}
+#endif  /* CONFIG_ANON_VMA_NAME */
+
+#endif /* _LINUX_ANON_VMA_H */
diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index 850e71986b9d..555f51de1fe0 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -890,52 +890,4 @@  typedef struct {
 	unsigned long val;
 } swp_entry_t;
 
-#ifdef CONFIG_ANON_VMA_NAME
-/*
- * mmap_lock should be read-locked when calling vma_anon_name() and while using
- * the returned pointer.
- */
-extern const char *vma_anon_name(struct vm_area_struct *vma);
-
-/*
- * mmap_lock should be read-locked for orig_vma->vm_mm.
- * mmap_lock should be write-locked for new_vma->vm_mm or new_vma should be
- * isolated.
- */
-extern void dup_vma_anon_name(struct vm_area_struct *orig_vma,
-			      struct vm_area_struct *new_vma);
-
-/*
- * mmap_lock should be write-locked or vma should have been isolated under
- * write-locked mmap_lock protection.
- */
-extern void free_vma_anon_name(struct vm_area_struct *vma);
-
-/* 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);
-
-	/* either both NULL, or pointers to same string */
-	if (vma_name == name)
-		return true;
-
-	return name && vma_name && !strcmp(name, vma_name);
-}
-#else /* CONFIG_ANON_VMA_NAME */
-static inline const char *vma_anon_name(struct vm_area_struct *vma)
-{
-	return NULL;
-}
-static inline void dup_vma_anon_name(struct vm_area_struct *orig_vma,
-			      struct vm_area_struct *new_vma) {}
-static inline void free_vma_anon_name(struct vm_area_struct *vma) {}
-static inline bool is_same_vma_anon_name(struct vm_area_struct *vma,
-					 const char *name)
-{
-	return true;
-}
-#endif  /* CONFIG_ANON_VMA_NAME */
-
 #endif /* _LINUX_MM_TYPES_H */
diff --git a/kernel/fork.c b/kernel/fork.c
index 7c06be0ca31b..8964e1559722 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -42,6 +42,7 @@ 
 #include <linux/mmu_notifier.h>
 #include <linux/fs.h>
 #include <linux/mm.h>
+#include <linux/anon_vma.h>
 #include <linux/vmacache.h>
 #include <linux/nsproxy.h>
 #include <linux/capability.h>
diff --git a/mm/madvise.c b/mm/madvise.c
index c63aacbbfa78..4d0ab22b31c0 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -18,6 +18,7 @@ 
 #include <linux/fadvise.h>
 #include <linux/sched.h>
 #include <linux/sched/mm.h>
+#include <linux/anon_vma.h>
 #include <linux/string.h>
 #include <linux/uio.h>
 #include <linux/ksm.h>
diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index e0066ca91d9a..58fbd8ec527f 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -99,6 +99,7 @@ 
 #include <linux/syscalls.h>
 #include <linux/ctype.h>
 #include <linux/mm_inline.h>
+#include <linux/anon_vma.h>
 #include <linux/mmu_notifier.h>
 #include <linux/printk.h>
 #include <linux/swapops.h>
diff --git a/mm/mlock.c b/mm/mlock.c
index 8f584eddd305..f3179d8169e4 100644
--- a/mm/mlock.c
+++ b/mm/mlock.c
@@ -9,6 +9,7 @@ 
 #include <linux/capability.h>
 #include <linux/mman.h>
 #include <linux/mm.h>
+#include <linux/anon_vma.h>
 #include <linux/sched/user.h>
 #include <linux/swap.h>
 #include <linux/swapops.h>
diff --git a/mm/mmap.c b/mm/mmap.c
index 6ea9e6775fa3..289a40d1d4f3 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -13,6 +13,7 @@ 
 #include <linux/slab.h>
 #include <linux/backing-dev.h>
 #include <linux/mm.h>
+#include <linux/anon_vma.h>
 #include <linux/vmacache.h>
 #include <linux/shm.h>
 #include <linux/mman.h>
diff --git a/mm/mprotect.c b/mm/mprotect.c
index 0138dfcdb1d8..96d57b1b41cd 100644
--- a/mm/mprotect.c
+++ b/mm/mprotect.c
@@ -28,6 +28,7 @@ 
 #include <linux/ksm.h>
 #include <linux/uaccess.h>
 #include <linux/mm_inline.h>
+#include <linux/anon_vma.h>
 #include <linux/pgtable.h>
 #include <asm/cacheflush.h>
 #include <asm/mmu_context.h>