Re: [PATCH v5 1/4] mm: hugetlb_vmemmap: introduce STRUCT_PAGE_SIZE_IS_POWER_OF_2

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

 



On Fri, Mar 25, 2022 at 1:11 PM Masahiro Yamada <masahiroy@xxxxxxxxxx> wrote:
>
> On Wed, Mar 23, 2022 at 9:57 PM Muchun Song <songmuchun@xxxxxxxxxxxxx> wrote:
> >
> > If the size of "struct page" is not the power of two and this
> > feature is enabled, then the vmemmap pages of HugeTLB will be
> > corrupted after remapping (panic is about to happen in theory).
> > But this only exists when !CONFIG_MEMCG && !CONFIG_SLUB on
> > x86_64.  However, it is not a conventional configuration nowadays.
> > So it is not a real word issue, just the result of a code review.
> > But we have to prevent anyone from configuring that combined
> > configuration.  In order to avoid many checks like "is_power_of_2
> > (sizeof(struct page))" through mm/hugetlb_vmemmap.c.  Introduce
> > STRUCT_PAGE_SIZE_IS_POWER_OF_2 to detect if the size of struct
> > page is power of 2 and make this feature depends on this new
> > config.  Then we could prevent anyone do any unexpected
> > configuration.
> >
> > Signed-off-by: Muchun Song <songmuchun@xxxxxxxxxxxxx>
> > Suggested-by: Luis Chamberlain <mcgrof@xxxxxxxxxx>
> > ---
> >  Kbuild                           | 14 ++++++++++++++
> >  fs/Kconfig                       |  1 +
> >  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 |  9 +++++++++
> >  7 files changed, 48 insertions(+), 6 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..21415c3b2728 100644
> > --- a/Kbuild
> > +++ b/Kbuild
> > @@ -37,6 +37,20 @@ $(offsets-file): arch/$(SRCARCH)/kernel/asm-offsets.s FORCE
> >         $(call filechk,offsets,__ASM_OFFSETS_H__)
> >
> >  #####
> > +# Generate struct_page_size.h.
> > +
> > +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
>
>
> No, please do not do this.
> It is terrible to feed back this to Kconfig again.

OK. I'll remove syncconfig.

>
> If you know this happens on !CONFIG_MEMCG && !CONFIG_SLUB on x86_64,
> why don't you add this dependency directly?

It is not enough since the size of the struct page also depends on
LAST_CPUPID_NOT_IN_PAGE_FLAGS && CONFIG_SLAB.
We cannot know the result of LAST_CPUPID_NOT_IN_PAGE_FLAGS in
Kconfig.

>
>
> If you want to avoid the run-time check,
> why don't you use  BUILD_BUG_ON() ?
>

Now I have another solution to avoid the run-time check.
We could use macro STRUCT_PAGE_SIZE_IS_POWER_OF_2
to do that like the following.

#ifdef STRUCT_PAGE_SIZE_IS_POWER_OF_2
/* code */
#endif

Thanks.



[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux