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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Jul 23, 2024 at 09:58:25AM GMT, SeongJae Park wrote:
> 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.

Thanks, good spot!

Yeah they can just be dropped, this is pedantry from wanting to absolutely
nail down the sources of declarations, something I pared down in the final
release, but obviously these were unfortunately arch-specific.

You're right that they're just not needed.

I will send a -fix patch in a second.

>
> """
> 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
>
> [...]




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux