Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
MIME-Version: 1.0
References: <20220318100720.14524-1-songmuchun@bytedance.com>
 <20220318100720.14524-2-songmuchun@bytedance.com> <YjS2BiqKlDrPYslc@bombadil.infradead.org>
In-Reply-To: <YjS2BiqKlDrPYslc@bombadil.infradead.org>
From:   Muchun Song <songmuchun@bytedance.com>
Date:   Sat, 19 Mar 2022 11:56:18 +0800
Message-ID: <CAMZfGtXPJ=0Xm_rm6AVGdL9hupkXyw7EtykY-B315LCkzdaZNg@mail.gmail.com>
Subject: Re: [PATCH v4 1/4] mm: hugetlb_vmemmap: introduce STRUCT_PAGE_SIZE_IS_POWER_OF_2
To:     Luis Chamberlain <mcgrof@kernel.org>
Cc:     Jonathan Corbet <corbet@lwn.net>,
        Mike Kravetz <mike.kravetz@oracle.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Kees Cook <keescook@chromium.org>,
        Iurii Zaikin <yzaikin@google.com>,
        Oscar Salvador <osalvador@suse.de>,
        David Hildenbrand <david@redhat.com>,
        Linux Doc Mailing List <linux-doc@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Linux Memory Management List <linux-mm@kvack.org>,
        Xiongchun duan <duanxiongchun@bytedance.com>,
        Muchun Song <smuchun@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Sat, Mar 19, 2022 at 12:40 AM Luis Chamberlain <mcgrof@kernel.org> wrote:
>
> On Fri, Mar 18, 2022 at 06:07:17PM +0800, Muchun Song wrote:
>
> You can add Suggested-by tag here.

Will do. Sorry for forgetting it.

>
> > Signed-off-by: Muchun Song <songmuchun@bytedance.com>
> > ---
> >  Kbuild                           | 12 ++++++++++++
> >  fs/Kconfig                       |  2 +-
> >  include/linux/mm_types.h         |  2 ++
> >  mm/Kconfig                       |  3 +++
> >  mm/hugetlb_vmemmap.c             |  6 ------
> >  mm/struct_page_size.c            | 19 +++++++++++++++++++
> >  scripts/check_struct_page_po2.sh | 11 +++++++++++
> >  7 files changed, 48 insertions(+), 7 deletions(-)
> >  create mode 100644 mm/struct_page_size.c
> >  create mode 100755 scripts/check_struct_page_po2.sh
> >
> > diff --git a/Kbuild b/Kbuild
> > index fa441b98c9f6..6bb97d348d62 100644
> > --- a/Kbuild
> > +++ b/Kbuild
> > @@ -14,6 +14,18 @@ $(bounds-file): kernel/bounds.s FORCE
> >       $(call filechk,offsets,__LINUX_BOUNDS_H__)
> >
> >  #####
> > +# Generate struct_page_size.h. Must follows bounds.h.
> > +
> > +struct_page_size-file := include/generated/struct_page_size.h
> > +
> > +always-y := $(struct_page_size-file)
> > +targets := mm/struct_page_size.s
> > +
> > +$(struct_page_size-file): mm/struct_page_size.s FORCE
> > +     $(call filechk,offsets,__LINUX_STRUCT_PAGE_SIZE_H__)
> > +     $(Q)$(MAKE) -f $(srctree)/Makefile syncconfig
> > +
> > +#####
>
> Shouldn't this go into mm/Makefile instead?
>

We should guarantee that the include/generated/struct_page_size.h
which Kconfig depends on is created before processing Makefile
since processing Kconfig is before Makefile. Right?

> > diff --git a/mm/Kconfig b/mm/Kconfig
> > index 034d87953600..9314bd34f49e 100644
> > --- a/mm/Kconfig
> > +++ b/mm/Kconfig
> > @@ -2,6 +2,9 @@
> >
> >  menu "Memory Management options"
> >
> > +config STRUCT_PAGE_SIZE_IS_POWER_OF_2
> > +     def_bool $(success,test "$(shell, $(srctree)/scripts/check_struct_page_po2.sh)" = 1)
> > +
> >  config SELECT_MEMORY_MODEL
> >       def_bool y
> >       depends on ARCH_SELECT_MEMORY_MODEL
> > new file mode 100755
> > index 000000000000..9547ad3aca05
> > --- /dev/null
> > +++ b/scripts/check_struct_page_po2.sh
> > @@ -0,0 +1,11 @@
> > +#!/bin/sh
> > +# SPDX-License-Identifier: GPL-2.0
> > +#
> > +# Check if the size of "struct page" is power of 2
> > +
> > +file="include/generated/struct_page_size.h"
> > +if [ ! -f "$file" ]; then
>
> Does this really work if one is workig off of a very clean build
> like make mrproper and then make menuconfig or or mrproper followed
> by a defconfig file ? Have you tried it for both cases po2 and npo2?
>
> Because isn't include/generated/struct_page_size.h generated? At
> which point does it get generated and why would the condition hole
> true that the file exists at a new 'make menuconfig' time?
>

You are right. include/generated/struct_page_size.h does not exist
in this case, CONFIG_STRUCT_PAGE_SIZE_IS_POWER_OF_2
will be default off in .config. Then it will be switched on/off accordingly
when you build the kernel, for instance make bzImage, which is done via
"make syncconfig" which follows the generation of struct_page_size.h.

While testing this case, I found some bugs. The following patch
could fix this.  Thanks.

--- a/Kbuild
+++ b/Kbuild
@@ -21,6 +21,8 @@ struct_page_size-file := include/generated/struct_page_size.h
 always-y := $(struct_page_size-file)
 targets := mm/struct_page_size.s

+mm/struct_page_size.s: $(timeconst-file) $(bounds-file)
+
 $(struct_page_size-file): mm/struct_page_size.s FORCE
        $(call filechk,offsets,__LINUX_STRUCT_PAGE_SIZE_H__)
        $(Q)$(MAKE) -f $(srctree)/Makefile syncconfig
diff --git a/scripts/check_struct_page_po2.sh b/scripts/check_struct_page_po2.sh
index 9547ad3aca05..1764ef9a4f1d 100755
--- a/scripts/check_struct_page_po2.sh
+++ b/scripts/check_struct_page_po2.sh
@@ -4,8 +4,6 @@
 # Check if the size of "struct page" is power of 2

 file="include/generated/struct_page_size.h"
-if [ ! -f "$file" ]; then
-       exit 1
+if [ -f "$file" ]; then
+       grep STRUCT_PAGE_SIZE_IS_POWER_OF_2 "$file" | cut -d' ' -f3
 fi
-
-grep STRUCT_PAGE_SIZE_IS_POWER_OF_2 "$file" | cut -d' ' -f3