[PATCH] mm: move dummy_vm_ops out of a header

[PATCH] mm: move dummy_vm_ops out of a header

[Mateusz Guzik]

Otherwise the kernel ends up with multiple copies:
$ nm vmlinux | grep dummy_vm_ops
ffffffff81e4ea00 d dummy_vm_ops.2
ffffffff81e11760 d dummy_vm_ops.254
ffffffff81e406e0 d dummy_vm_ops.4
ffffffff81e3c780 d dummy_vm_ops.7

[if someone has better ideas where to put it, please move it]

Signed-off-by: Mateusz Guzik <mjguzik@gmail.com>
---
 include/linux/mm.h | 6 +++---
 mm/init-mm.c       | 2 ++
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 406ab9ea818f..14898e76bbf1 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -756,17 +756,17 @@ static inline void vma_mark_detached(struct vm_area_struct *vma,
 
 #endif /* CONFIG_PER_VMA_LOCK */
 
+extern const struct vm_operations_struct vma_dummy_vm_ops;
+
 /*
  * WARNING: vma_init does not initialize vma->vm_lock.
  * Use vm_area_alloc()/vm_area_free() if vma needs locking.
  */
 static inline void vma_init(struct vm_area_struct *vma, struct mm_struct *mm)
 {
-	static const struct vm_operations_struct dummy_vm_ops = {};
-
 	memset(vma, 0, sizeof(*vma));
 	vma->vm_mm = mm;
-	vma->vm_ops = &dummy_vm_ops;
+	vma->vm_ops = &vma_dummy_vm_ops;
 	INIT_LIST_HEAD(&vma->anon_vma_chain);
 	vma_mark_detached(vma, false);
 	vma_numab_state_init(vma);
diff --git a/mm/init-mm.c b/mm/init-mm.c
index efa97b57acfd..cfd367822cdd 100644
--- a/mm/init-mm.c
+++ b/mm/init-mm.c
@@ -17,6 +17,8 @@
 #define INIT_MM_CONTEXT(name)
 #endif
 
+const struct vm_operations_struct vma_dummy_vm_ops;
+
 /*
  * For dynamically allocated mm_structs, there is a dynamically sized cpumask
  * at the end of the structure, the size of which depends on the maximum CPU
-- 
2.39.2

Re: [PATCH] mm: move dummy_vm_ops out of a header

[Suren Baghdasaryan]

On Fri, Aug 4, 2023 at 6:41 PM Mateusz Guzik <mjguzik@gmail.com> wrote:
>
> Otherwise the kernel ends up with multiple copies:
> $ nm vmlinux | grep dummy_vm_ops
> ffffffff81e4ea00 d dummy_vm_ops.2
> ffffffff81e11760 d dummy_vm_ops.254
> ffffffff81e406e0 d dummy_vm_ops.4
> ffffffff81e3c780 d dummy_vm_ops.7
>
> [if someone has better ideas where to put it, please move it]
>
> Signed-off-by: Mateusz Guzik <mjguzik@gmail.com>

Thanks for the patch and it looks fine to me but why is it sent to me
and not to the mm maintainer? Also nobody else is CC'ed...
AFAIKT this code has been there long before per-vma locks got
introduced. Did my patchset somehow cause additional side-effects?

> ---
>  include/linux/mm.h | 6 +++---
>  mm/init-mm.c       | 2 ++
>  2 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 406ab9ea818f..14898e76bbf1 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -756,17 +756,17 @@ static inline void vma_mark_detached(struct vm_area_struct *vma,
>
>  #endif /* CONFIG_PER_VMA_LOCK */
>
> +extern const struct vm_operations_struct vma_dummy_vm_ops;
> +
>  /*
>   * WARNING: vma_init does not initialize vma->vm_lock.
>   * Use vm_area_alloc()/vm_area_free() if vma needs locking.
>   */
>  static inline void vma_init(struct vm_area_struct *vma, struct mm_struct *mm)
>  {
> -       static const struct vm_operations_struct dummy_vm_ops = {};
> -
>         memset(vma, 0, sizeof(*vma));
>         vma->vm_mm = mm;
> -       vma->vm_ops = &dummy_vm_ops;
> +       vma->vm_ops = &vma_dummy_vm_ops;
>         INIT_LIST_HEAD(&vma->anon_vma_chain);
>         vma_mark_detached(vma, false);
>         vma_numab_state_init(vma);
> diff --git a/mm/init-mm.c b/mm/init-mm.c
> index efa97b57acfd..cfd367822cdd 100644
> --- a/mm/init-mm.c
> +++ b/mm/init-mm.c
> @@ -17,6 +17,8 @@
>  #define INIT_MM_CONTEXT(name)
>  #endif
>
> +const struct vm_operations_struct vma_dummy_vm_ops;
> +
>  /*
>   * For dynamically allocated mm_structs, there is a dynamically sized cpumask
>   * at the end of the structure, the size of which depends on the maximum CPU
> --
> 2.39.2
>

Re: [PATCH] mm: move dummy_vm_ops out of a header

[Mateusz Guzik]

On 8/7/23, Matthew Wilcox <willy@infradead.org> wrote:
> On Mon, Aug 07, 2023 at 01:16:11AM +0200, Mateusz Guzik wrote:
>> Otherwise the kernel ends up with multiple copies:
>> $ nm vmlinux | grep dummy_vm_ops
>> ffffffff81e4ea00 d dummy_vm_ops.2
>> ffffffff81e11760 d dummy_vm_ops.254
>> ffffffff81e406e0 d dummy_vm_ops.4
>> ffffffff81e3c780 d dummy_vm_ops.7
>>
>> While here prefix it with vma_.
>
> It really shouldn't be prefixed with vma.  Other than that, I love this
> patch.
>

I think an unprefixed global is iffy, but I'm not going to insist on
semi-cosmetics.

The one thing I expected people to complain about is the location of
the struct. The mm_init.c file was the least bad choice I found, but
maybe something else is preferred?
I'm just trying to avoid sending V3 for this patch after someone
claims dummy ops should land elsewhere. ;)

-- 
Mateusz Guzik <mjguzik gmail.com>

Re: [PATCH] mm: move dummy_vm_ops out of a header

[Matthew Wilcox]

On Mon, Aug 07, 2023 at 01:16:11AM +0200, Mateusz Guzik wrote:
> Otherwise the kernel ends up with multiple copies:
> $ nm vmlinux | grep dummy_vm_ops
> ffffffff81e4ea00 d dummy_vm_ops.2
> ffffffff81e11760 d dummy_vm_ops.254
> ffffffff81e406e0 d dummy_vm_ops.4
> ffffffff81e3c780 d dummy_vm_ops.7
> 
> While here prefix it with vma_.

It really shouldn't be prefixed with vma.  Other than that, I love this
patch.

> [if someone has better ideas where to put it, please move it]
> 
> Signed-off-by: Mateusz Guzik <mjguzik@gmail.com>
> ---
>  include/linux/mm.h | 6 +++---
>  mm/init-mm.c       | 2 ++
>  2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 406ab9ea818f..14898e76bbf1 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -756,17 +756,17 @@ static inline void vma_mark_detached(struct vm_area_struct *vma,
>  
>  #endif /* CONFIG_PER_VMA_LOCK */
>  
> +extern const struct vm_operations_struct vma_dummy_vm_ops;
> +
>  /*
>   * WARNING: vma_init does not initialize vma->vm_lock.
>   * Use vm_area_alloc()/vm_area_free() if vma needs locking.
>   */
>  static inline void vma_init(struct vm_area_struct *vma, struct mm_struct *mm)
>  {
> -	static const struct vm_operations_struct dummy_vm_ops = {};
> -
>  	memset(vma, 0, sizeof(*vma));
>  	vma->vm_mm = mm;
> -	vma->vm_ops = &dummy_vm_ops;
> +	vma->vm_ops = &vma_dummy_vm_ops;
>  	INIT_LIST_HEAD(&vma->anon_vma_chain);
>  	vma_mark_detached(vma, false);
>  	vma_numab_state_init(vma);
> diff --git a/mm/init-mm.c b/mm/init-mm.c
> index efa97b57acfd..cfd367822cdd 100644
> --- a/mm/init-mm.c
> +++ b/mm/init-mm.c
> @@ -17,6 +17,8 @@
>  #define INIT_MM_CONTEXT(name)
>  #endif
>  
> +const struct vm_operations_struct vma_dummy_vm_ops;
> +
>  /*
>   * For dynamically allocated mm_structs, there is a dynamically sized cpumask
>   * at the end of the structure, the size of which depends on the maximum CPU
> -- 
> 2.39.2
> 
> 

[PATCH] mm: move dummy_vm_ops out of a header

[Mateusz Guzik]

Otherwise the kernel ends up with multiple copies:
$ nm vmlinux | grep dummy_vm_ops
ffffffff81e4ea00 d dummy_vm_ops.2
ffffffff81e11760 d dummy_vm_ops.254
ffffffff81e406e0 d dummy_vm_ops.4
ffffffff81e3c780 d dummy_vm_ops.7

While here prefix it with vma_.

[if someone has better ideas where to put it, please move it]

Signed-off-by: Mateusz Guzik <mjguzik@gmail.com>
---
 include/linux/mm.h | 6 +++---
 mm/init-mm.c       | 2 ++
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 406ab9ea818f..14898e76bbf1 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -756,17 +756,17 @@ static inline void vma_mark_detached(struct vm_area_struct *vma,
 
 #endif /* CONFIG_PER_VMA_LOCK */
 
+extern const struct vm_operations_struct vma_dummy_vm_ops;
+
 /*
  * WARNING: vma_init does not initialize vma->vm_lock.
  * Use vm_area_alloc()/vm_area_free() if vma needs locking.
  */
 static inline void vma_init(struct vm_area_struct *vma, struct mm_struct *mm)
 {
-	static const struct vm_operations_struct dummy_vm_ops = {};
-
 	memset(vma, 0, sizeof(*vma));
 	vma->vm_mm = mm;
-	vma->vm_ops = &dummy_vm_ops;
+	vma->vm_ops = &vma_dummy_vm_ops;
 	INIT_LIST_HEAD(&vma->anon_vma_chain);
 	vma_mark_detached(vma, false);
 	vma_numab_state_init(vma);
diff --git a/mm/init-mm.c b/mm/init-mm.c
index efa97b57acfd..cfd367822cdd 100644
--- a/mm/init-mm.c
+++ b/mm/init-mm.c
@@ -17,6 +17,8 @@
 #define INIT_MM_CONTEXT(name)
 #endif
 
+const struct vm_operations_struct vma_dummy_vm_ops;
+
 /*
  * For dynamically allocated mm_structs, there is a dynamically sized cpumask
  * at the end of the structure, the size of which depends on the maximum CPU
-- 
2.39.2