| Message ID | 20240510020422.8038-1-richard.weiyang@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] mm/memblock: discard .text/.data if CONFIG_ARCH_KEEP_MEMBLOCK not set | expand |
Hi, On Fri, May 10, 2024 at 02:04:22AM +0000, Wei Yang wrote: > When CONFIG_ARCH_KEEP_MEMBLOCK not set, we expect to discard related > code and data. But it doesn't until CONFIG_MEMORY_HOTPLUG not set > neither. > > This patch puts memblock's .text/.data into its own section, so that it > only depends on CONFIG_ARCH_KEEP_MEMBLOCK to discard related code and > data. > > After this, from the log message in mem_init_print_info(), init size > increase from 2420K to 2432K on arch x86. > > Signed-off-by: Wei Yang <richard.weiyang@gmail.com> > > --- > v2: fix orphan section for powerpc > --- > arch/powerpc/kernel/vmlinux.lds.S | 1 + > include/asm-generic/vmlinux.lds.h | 14 +++++++++++++- > include/linux/memblock.h | 8 ++++---- > 3 files changed, 18 insertions(+), 5 deletions(-) > > +#define __init_memblock __section(".mbinit.text") __cold notrace \ > + __latent_entropy > +#define __initdata_memblock __section(".mbinit.data") > + The new .mbinit.* sections should be added to scripts/mod/modpost.c alongside .meminit.* sections and then I expect modpost to report a bunch of section mismatches because many memblock functions are called on memory hotplug even on architectures that don't select ARCH_KEEP_MEMBLOCK. > #ifndef CONFIG_ARCH_KEEP_MEMBLOCK > -#define __init_memblock __meminit > -#define __initdata_memblock __meminitdata > void memblock_discard(void); > #else > -#define __init_memblock > -#define __initdata_memblock > static inline void memblock_discard(void) {} > #endif > > -- > 2.34.1 > >
On Tue, May 21, 2024 at 10:21:52AM +0300, Mike Rapoport wrote: >Hi, > >On Fri, May 10, 2024 at 02:04:22AM +0000, Wei Yang wrote: >> When CONFIG_ARCH_KEEP_MEMBLOCK not set, we expect to discard related >> code and data. But it doesn't until CONFIG_MEMORY_HOTPLUG not set >> neither. >> >> This patch puts memblock's .text/.data into its own section, so that it >> only depends on CONFIG_ARCH_KEEP_MEMBLOCK to discard related code and >> data. >> >> After this, from the log message in mem_init_print_info(), init size >> increase from 2420K to 2432K on arch x86. >> >> Signed-off-by: Wei Yang <richard.weiyang@gmail.com> >> >> --- >> v2: fix orphan section for powerpc >> --- >> arch/powerpc/kernel/vmlinux.lds.S | 1 + >> include/asm-generic/vmlinux.lds.h | 14 +++++++++++++- >> include/linux/memblock.h | 8 ++++---- >> 3 files changed, 18 insertions(+), 5 deletions(-) >> >> +#define __init_memblock __section(".mbinit.text") __cold notrace \ >> + __latent_entropy >> +#define __initdata_memblock __section(".mbinit.data") >> + > >The new .mbinit.* sections should be added to scripts/mod/modpost.c >alongside .meminit.* sections and then I expect modpost to report a bunch >of section mismatches because many memblock functions are called on memory >hotplug even on architectures that don't select ARCH_KEEP_MEMBLOCK. > I tried to add some code in modpost.c, "make all" looks good. May I ask how can I trigger the "mismatch" warning? BTW, if ARCH_KEEP_MEMBLOCK unset, we would discard memblock meta-data. If hotplug would call memblock function, it would be dangerous? The additional code I used is like below. --- scripts/mod/modpost.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index 937294ff164f..c837e2882904 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -777,14 +777,14 @@ static void check_section(const char *modname, struct elf_info *elf, #define ALL_INIT_DATA_SECTIONS \ ".init.setup", ".init.rodata", ".meminit.rodata", \ - ".init.data", ".meminit.data" + ".init.data", ".meminit.data", "mbinit.data" #define ALL_PCI_INIT_SECTIONS \ ".pci_fixup_early", ".pci_fixup_header", ".pci_fixup_final", \ ".pci_fixup_enable", ".pci_fixup_resume", \ ".pci_fixup_resume_early", ".pci_fixup_suspend" -#define ALL_XXXINIT_SECTIONS ".meminit.*" +#define ALL_XXXINIT_SECTIONS ".meminit.*", "mbinit.*" #define ALL_INIT_SECTIONS INIT_SECTIONS, ALL_XXXINIT_SECTIONS #define ALL_EXIT_SECTIONS ".exit.*" @@ -799,7 +799,7 @@ static void check_section(const char *modname, struct elf_info *elf, #define INIT_SECTIONS ".init.*" -#define ALL_TEXT_SECTIONS ".init.text", ".meminit.text", ".exit.text", \ +#define ALL_TEXT_SECTIONS ".init.text", ".meminit.text", ".mbinit.text", ".exit.text", \ TEXT_SECTIONS, OTHER_TEXT_SECTIONS enum mismatch {
On Fri, May 24, 2024 at 01:46:56AM +0000, Wei Yang wrote: > On Tue, May 21, 2024 at 10:21:52AM +0300, Mike Rapoport wrote: > >Hi, > > > >On Fri, May 10, 2024 at 02:04:22AM +0000, Wei Yang wrote: > >> When CONFIG_ARCH_KEEP_MEMBLOCK not set, we expect to discard related > >> code and data. But it doesn't until CONFIG_MEMORY_HOTPLUG not set > >> neither. > >> > >> This patch puts memblock's .text/.data into its own section, so that it > >> only depends on CONFIG_ARCH_KEEP_MEMBLOCK to discard related code and > >> data. > >> > >> After this, from the log message in mem_init_print_info(), init size > >> increase from 2420K to 2432K on arch x86. > >> > >> Signed-off-by: Wei Yang <richard.weiyang@gmail.com> > >> > >> --- > >> v2: fix orphan section for powerpc > >> --- > >> arch/powerpc/kernel/vmlinux.lds.S | 1 + > >> include/asm-generic/vmlinux.lds.h | 14 +++++++++++++- > >> include/linux/memblock.h | 8 ++++---- > >> 3 files changed, 18 insertions(+), 5 deletions(-) > >> > >> +#define __init_memblock __section(".mbinit.text") __cold notrace \ > >> + __latent_entropy > >> +#define __initdata_memblock __section(".mbinit.data") > >> + > > > >The new .mbinit.* sections should be added to scripts/mod/modpost.c > >alongside .meminit.* sections and then I expect modpost to report a bunch > >of section mismatches because many memblock functions are called on memory > >hotplug even on architectures that don't select ARCH_KEEP_MEMBLOCK. > > > > I tried to add some code in modpost.c, "make all" looks good. > > May I ask how can I trigger the "mismatch" warning? > > BTW, if ARCH_KEEP_MEMBLOCK unset, we would discard memblock meta-data. If > hotplug would call memblock function, it would be dangerous? > > The additional code I used is like below. > > --- > scripts/mod/modpost.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c > index 937294ff164f..c837e2882904 100644 > --- a/scripts/mod/modpost.c > +++ b/scripts/mod/modpost.c > @@ -777,14 +777,14 @@ static void check_section(const char *modname, struct elf_info *elf, > > #define ALL_INIT_DATA_SECTIONS \ > ".init.setup", ".init.rodata", ".meminit.rodata", \ > - ".init.data", ".meminit.data" > + ".init.data", ".meminit.data", "mbinit.data" should be ".mbinit.data" > > #define ALL_PCI_INIT_SECTIONS \ > ".pci_fixup_early", ".pci_fixup_header", ".pci_fixup_final", \ > ".pci_fixup_enable", ".pci_fixup_resume", \ > ".pci_fixup_resume_early", ".pci_fixup_suspend" > > -#define ALL_XXXINIT_SECTIONS ".meminit.*" > +#define ALL_XXXINIT_SECTIONS ".meminit.*", "mbinit.*" and ".mbinit.*" But regardless of typos, when ARCH_KEEP_MEMBLOCK=n the .mbinit is equivalent to .init and it should not be referenced from .meminit, so I don't think adding it here is correct. If I simply alias __init_memblock to __init then with CONFIG_MEMORY_HOTPLUG=y I get WARNING: modpost: vmlinux: section mismatch in reference: early_pfn_to_nid+0x42 (section: .meminit.text) -> memblock_search_pfn_nid (section: .init.text) WARNING: modpost: vmlinux: section mismatch in reference: memmap_init_range+0x142 (section: .meminit.text) -> mirrored_kernelcore (section: .init.data) WARNING: modpost: vmlinux: section mismatch in reference: memmap_init_range+0x1e1 (section: .meminit.text) -> memblock (section: .init.data) WARNING: modpost: vmlinux: section mismatch in reference: memmap_init_range+0x1e8 (section: .meminit.text) -> memblock (section: .init.data) WARNING: modpost: vmlinux: section mismatch in reference: sparse_buffer_alloc+0x3b (section: .meminit.text) -> memblock_free (section: .init.text) > #define ALL_INIT_SECTIONS INIT_SECTIONS, ALL_XXXINIT_SECTIONS > #define ALL_EXIT_SECTIONS ".exit.*" > @@ -799,7 +799,7 @@ static void check_section(const char *modname, struct elf_info *elf, > > #define INIT_SECTIONS ".init.*" > > -#define ALL_TEXT_SECTIONS ".init.text", ".meminit.text", ".exit.text", \ > +#define ALL_TEXT_SECTIONS ".init.text", ".meminit.text", ".mbinit.text", ".exit.text", \ > TEXT_SECTIONS, OTHER_TEXT_SECTIONS > > enum mismatch { > > -- > Wei Yang > Help you, Help me >
diff --git a/arch/powerpc/kernel/vmlinux.lds.S b/arch/powerpc/kernel/vmlinux.lds.S index f420df7888a7..d6d33bec597a 100644 --- a/arch/powerpc/kernel/vmlinux.lds.S +++ b/arch/powerpc/kernel/vmlinux.lds.S @@ -125,6 +125,7 @@ SECTIONS *(.text.asan.* .text.tsan.*) MEM_KEEP(init.text) MEM_KEEP(exit.text) + MEMBLOCK_KEEP(init.text) } :text . = ALIGN(PAGE_SIZE); diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h index f7749d0f2562..775c5eedb9e6 100644 --- a/include/asm-generic/vmlinux.lds.h +++ b/include/asm-generic/vmlinux.lds.h @@ -147,6 +147,14 @@ #define MEM_DISCARD(sec) *(.mem##sec) #endif +#if defined(CONFIG_ARCH_KEEP_MEMBLOCK) +#define MEMBLOCK_KEEP(sec) *(.mb##sec) +#define MEMBLOCK_DISCARD(sec) +#else +#define MEMBLOCK_KEEP(sec) +#define MEMBLOCK_DISCARD(sec) *(.mb##sec) +#endif + #ifndef CONFIG_HAVE_DYNAMIC_FTRACE_NO_PATCHABLE #define KEEP_PATCHABLE KEEP(*(__patchable_function_entries)) #define PATCHABLE_DISCARDS @@ -356,6 +364,7 @@ *(.ref.data) \ *(.data..shared_aligned) /* percpu related */ \ MEM_KEEP(init.data*) \ + MEMBLOCK_KEEP(init.data*) \ *(.data.unlikely) \ __start_once = .; \ *(.data.once) \ @@ -573,6 +582,7 @@ *(.ref.text) \ *(.text.asan.* .text.tsan.*) \ MEM_KEEP(init.text*) \ + MEMBLOCK_KEEP(init.text*) \ /* sched.text is aling to function alignment to secure we have same @@ -680,6 +690,7 @@ KEEP(*(SORT(___kentry+*))) \ *(.init.data .init.data.*) \ MEM_DISCARD(init.data*) \ + MEMBLOCK_DISCARD(init.data*) \ KERNEL_CTORS() \ MCOUNT_REC() \ *(.init.rodata .init.rodata.*) \ @@ -706,7 +717,8 @@ #define INIT_TEXT \ *(.init.text .init.text.*) \ *(.text.startup) \ - MEM_DISCARD(init.text*) + MEM_DISCARD(init.text*) \ + MEMBLOCK_DISCARD(init.text*) #define EXIT_DATA \ *(.exit.data .exit.data.*) \ diff --git a/include/linux/memblock.h b/include/linux/memblock.h index e2082240586d..3e1f1d42dde7 100644 --- a/include/linux/memblock.h +++ b/include/linux/memblock.h @@ -100,13 +100,13 @@ struct memblock { extern struct memblock memblock; +#define __init_memblock __section(".mbinit.text") __cold notrace \ + __latent_entropy +#define __initdata_memblock __section(".mbinit.data") + #ifndef CONFIG_ARCH_KEEP_MEMBLOCK -#define __init_memblock __meminit -#define __initdata_memblock __meminitdata void memblock_discard(void); #else -#define __init_memblock -#define __initdata_memblock static inline void memblock_discard(void) {} #endif
When CONFIG_ARCH_KEEP_MEMBLOCK not set, we expect to discard related code and data. But it doesn't until CONFIG_MEMORY_HOTPLUG not set neither. This patch puts memblock's .text/.data into its own section, so that it only depends on CONFIG_ARCH_KEEP_MEMBLOCK to discard related code and data. After this, from the log message in mem_init_print_info(), init size increase from 2420K to 2432K on arch x86. Signed-off-by: Wei Yang <richard.weiyang@gmail.com> --- v2: fix orphan section for powerpc --- arch/powerpc/kernel/vmlinux.lds.S | 1 + include/asm-generic/vmlinux.lds.h | 14 +++++++++++++- include/linux/memblock.h | 8 ++++---- 3 files changed, 18 insertions(+), 5 deletions(-)