Re: [PATCH v3 4/7] mm: move internal core VMA manipulation functions to own file

[SeongJae Park]

Hi Lorenzo,

On Mon, 22 Jul 2024 12:50:22 +0100 Lorenzo Stoakes <lorenzo.stoakes@xxxxxxxxxx> wrote:

> This patch introduces vma.c and moves internal core VMA manipulation
> functions to this file from mmap.c.
> 
> This allows us to isolate VMA functionality in a single place such that we
> can create userspace testing code that invokes this functionality in an
> environment where we can implement simple unit tests of core functionality.
> 
> This patch ensures that core VMA functionality is explicitly marked as such
> by its presence in mm/vma.h.
> 
> It also places the header includes required by vma.c in vma_internal.h,
> which is simply imported by vma.c. This makes the VMA functionality
> testable, as userland testing code can simply stub out functionality
> as required.
> 
> Reviewed-by: Vlastimil Babka <vbabka@xxxxxxx>
> Reviewed-by: Liam R. Howlett <Liam.Howlett@xxxxxxxxxx>
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@xxxxxxxxxx>
> ---
>  include/linux/mm.h |   35 -
>  mm/Makefile        |    2 +-
>  mm/internal.h      |  236 +-----
>  mm/mmap.c          | 1980 +++-----------------------------------------
>  mm/mmu_notifier.c  |    2 +
>  mm/vma.c           | 1766 +++++++++++++++++++++++++++++++++++++++
>  mm/vma.h           |  364 ++++++++
>  mm/vma_internal.h  |   52 ++
>  8 files changed, 2294 insertions(+), 2143 deletions(-)
>  create mode 100644 mm/vma.c
>  create mode 100644 mm/vma.h
>  create mode 100644 mm/vma_internal.h
> 
[...]
> diff --git a/mm/vma_internal.h b/mm/vma_internal.h
> new file mode 100644
> index 000000000000..e13e5950df78
> --- /dev/null
> +++ b/mm/vma_internal.h
> @@ -0,0 +1,52 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * vma_internal.h
> + *
> + * Headers required by vma.c, which can be substituted accordingly when testing
> + * VMA functionality.
> + */
> +
> +#ifndef __MM_VMA_INTERNAL_H
> +#define __MM_VMA_INTERNAL_H
> +
[...]
> +#include <asm/current.h>
> +#include <asm/page_types.h>
> +#include <asm/pgtable_types.h>

I found the latest mm-unstable fails build for arm64 and kunit (tenically
speaking, UM) with errors including below.  And 'git bisect' points this patch.

>From arm64 build:
      CC      mm/vma.o
    In file included from /mm/vma.c:7:
    /mm/vma_internal.h:46:10: fatal error: asm/page_types.h: No such file or directory
       46 | #include <asm/page_types.h>
          |          ^~~~~~~~~~~~~~~~~~
    compilation terminated.

>From kunit build:

    $ ./tools/testing/kunit/kunit.py build
    [...]
    $ make ARCH=um O=.kunit --jobs=36
    ERROR:root:../lib/iomap.c:156:5: warning: no previous prototype for ‘ioread64_lo_hi’ [-Wmissing-prototypes]
      156 | u64 ioread64_lo_hi(const void __iomem *addr)
          |     ^~~~~~~~~~~~~~
    ../lib/iomap.c:163:5: warning: no previous prototype for ‘ioread64_hi_lo’ [-Wmissing-prototypes]
      163 | u64 ioread64_hi_lo(const void __iomem *addr)
          |     ^~~~~~~~~~~~~~
    ../lib/iomap.c:170:5: warning: no previous prototype for ‘ioread64be_lo_hi’ [-Wmissing-prototypes]
      170 | u64 ioread64be_lo_hi(const void __iomem *addr)
          |     ^~~~~~~~~~~~~~~~
    ../lib/iomap.c:178:5: warning: no previous prototype for ‘ioread64be_hi_lo’ [-Wmissing-prototypes]
      178 | u64 ioread64be_hi_lo(const void __iomem *addr)
          |     ^~~~~~~~~~~~~~~~
    ../lib/iomap.c:264:6: warning: no previous prototype for ‘iowrite64_lo_hi’ [-Wmissing-prototypes]
      264 | void iowrite64_lo_hi(u64 val, void __iomem *addr)
          |      ^~~~~~~~~~~~~~~
    ../lib/iomap.c:272:6: warning: no previous prototype for ‘iowrite64_hi_lo’ [-Wmissing-prototypes]
      272 | void iowrite64_hi_lo(u64 val, void __iomem *addr)
          |      ^~~~~~~~~~~~~~~
    ../lib/iomap.c:280:6: warning: no previous prototype for ‘iowrite64be_lo_hi’ [-Wmissing-prototypes]
      280 | void iowrite64be_lo_hi(u64 val, void __iomem *addr)
          |      ^~~~~~~~~~~~~~~~~
    ../lib/iomap.c:288:6: warning: no previous prototype for ‘iowrite64be_hi_lo’ [-Wmissing-prototypes]
      288 | void iowrite64be_hi_lo(u64 val, void __iomem *addr)
          |      ^~~~~~~~~~~~~~~~~
    In file included from ../mm/vma_internal.h:46,
                     from ../mm/vma.c:7:

Maybe the above two #include need to be removed or protected for some configs?
I confirmed simply removing the two lines as below makes at least kunit, arm64,
and my x86_64 builds happy, but would like to hear your thoughts.

"""
diff --git a/mm/vma_internal.h b/mm/vma_internal.h
index e13e5950df78..14c24d5cb582 100644
--- a/mm/vma_internal.h
+++ b/mm/vma_internal.h
@@ -43,8 +43,6 @@
 #include <linux/userfaultfd_k.h>

 #include <asm/current.h>
-#include <asm/page_types.h>
-#include <asm/pgtable_types.h>
 #include <asm/tlb.h>

 #include "internal.h"
"""


Thanks,
SJ

[...]