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.
