* Re: [PATCH 2/2] init/modpost: conditionally check section mismatch to __meminit*
2024-07-06 16:05 ` [PATCH 2/2] init/modpost: conditionally check section mismatch to __meminit* Masahiro Yamada
@ 2024-07-06 18:07 ` Masahiro Yamada
2024-07-07 0:01 ` Wei Yang
1 sibling, 0 replies; 6+ messages in thread
From: Masahiro Yamada @ 2024-07-06 18:07 UTC (permalink / raw)
To: linux-kbuild
Cc: linux-kernel, Wei Yang, Andrew Morton, Arnd Bergmann,
Nathan Chancellor, Nicolas Schier, linux-arch
On Sun, Jul 7, 2024 at 1:05 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> This reverts commit eb8f689046b8 ("Use separate sections for __dev/
> _cpu/__mem code/data").
>
> Check section mismatch to __meminit* only when CONFIG_MEMORY_HOTPLUG=y.
This is the opposite. The correct statement is:
... only when CONFIG_MEMORY_HOTPLUG=n
>
> With this change, the linker script and modpost become simpler, and we
> can get rid of the __ref annotations from the memory hotplug code.
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
>
> include/asm-generic/vmlinux.lds.h | 18 ++----------------
> include/linux/init.h | 14 +++++++++-----
> scripts/mod/modpost.c | 19 ++++---------------
> 3 files changed, 15 insertions(+), 36 deletions(-)
>
> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> index 62b4cb0462e6..c23f7d0645ad 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -141,14 +141,6 @@
> * often happens at runtime)
> */
>
> -#if defined(CONFIG_MEMORY_HOTPLUG)
> -#define MEM_KEEP(sec) *(.mem##sec)
> -#define MEM_DISCARD(sec)
> -#else
> -#define MEM_KEEP(sec)
> -#define MEM_DISCARD(sec) *(.mem##sec)
> -#endif
> -
> #ifndef CONFIG_HAVE_DYNAMIC_FTRACE_NO_PATCHABLE
> #define KEEP_PATCHABLE KEEP(*(__patchable_function_entries))
> #define PATCHABLE_DISCARDS
> @@ -357,7 +349,6 @@
> *(.data..decrypted) \
> *(.ref.data) \
> *(.data..shared_aligned) /* percpu related */ \
> - MEM_KEEP(init.data*) \
> *(.data.unlikely) \
> __start_once = .; \
> *(.data.once) \
> @@ -523,7 +514,6 @@
> /* __*init sections */ \
> __init_rodata : AT(ADDR(__init_rodata) - LOAD_OFFSET) { \
> *(.ref.rodata) \
> - MEM_KEEP(init.rodata) \
> } \
> \
> /* Built-in module parameters. */ \
> @@ -574,8 +564,7 @@
> *(.text.unknown .text.unknown.*) \
> NOINSTR_TEXT \
> *(.ref.text) \
> - *(.text.asan.* .text.tsan.*) \
> - MEM_KEEP(init.text*) \
> + *(.text.asan.* .text.tsan.*)
>
>
> /* sched.text is aling to function alignment to secure we have same
> @@ -682,7 +671,6 @@
> #define INIT_DATA \
> KEEP(*(SORT(___kentry+*))) \
> *(.init.data .init.data.*) \
> - MEM_DISCARD(init.data*) \
> KERNEL_CTORS() \
> MCOUNT_REC() \
> *(.init.rodata .init.rodata.*) \
> @@ -690,7 +678,6 @@
> TRACE_SYSCALLS() \
> KPROBE_BLACKLIST() \
> ERROR_INJECT_WHITELIST() \
> - MEM_DISCARD(init.rodata) \
> CLK_OF_TABLES() \
> RESERVEDMEM_OF_TABLES() \
> TIMER_OF_TABLES() \
> @@ -708,8 +695,7 @@
>
> #define INIT_TEXT \
> *(.init.text .init.text.*) \
> - *(.text.startup) \
> - MEM_DISCARD(init.text*)
> + *(.text.startup)
>
> #define EXIT_DATA \
> *(.exit.data .exit.data.*) \
> diff --git a/include/linux/init.h b/include/linux/init.h
> index b2e9dfff8691..ee1309473bc6 100644
> --- a/include/linux/init.h
> +++ b/include/linux/init.h
> @@ -84,11 +84,15 @@
>
> #define __exit __section(".exit.text") __exitused __cold notrace
>
> -/* Used for MEMORY_HOTPLUG */
> -#define __meminit __section(".meminit.text") __cold notrace \
> - __latent_entropy
> -#define __meminitdata __section(".meminit.data")
> -#define __meminitconst __section(".meminit.rodata")
> +#ifdef CONFIG_MEMORY_HOTPLUG
> +#define __meminit
> +#define __meminitdata
> +#define __meminitconst
> +#else
> +#define __meminit __init
> +#define __meminitdata __initdata
> +#define __meminitconst __initconst
> +#endif
>
> /* For assembly routines */
> #define __HEAD .section ".head.text","ax"
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index 3e5313ed6065..8c8ad7485f73 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -776,17 +776,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.setup", ".init.rodata", ".init.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_INIT_SECTIONS INIT_SECTIONS, ALL_XXXINIT_SECTIONS
> +#define ALL_INIT_SECTIONS ".init.*"
> #define ALL_EXIT_SECTIONS ".exit.*"
>
> #define DATA_SECTIONS ".data", ".data.rel"
> @@ -797,9 +794,7 @@ static void check_section(const char *modname, struct elf_info *elf,
> ".fixup", ".entry.text", ".exception.text", \
> ".coldtext", ".softirqentry.text"
>
> -#define INIT_SECTIONS ".init.*"
> -
> -#define ALL_TEXT_SECTIONS ".init.text", ".meminit.text", ".exit.text", \
> +#define ALL_TEXT_SECTIONS ".init.text", ".exit.text", \
> TEXT_SECTIONS, OTHER_TEXT_SECTIONS
>
> enum mismatch {
> @@ -839,12 +834,6 @@ static const struct sectioncheck sectioncheck[] = {
> .bad_tosec = { ALL_INIT_SECTIONS, ALL_EXIT_SECTIONS, NULL },
> .mismatch = TEXTDATA_TO_ANY_INIT_EXIT,
> },
> -/* Do not reference init code/data from meminit code/data */
> -{
> - .fromsec = { ALL_XXXINIT_SECTIONS, NULL },
> - .bad_tosec = { INIT_SECTIONS, NULL },
> - .mismatch = XXXINIT_TO_SOME_INIT,
> -},
> /* Do not use exit code/data from init code */
> {
> .fromsec = { ALL_INIT_SECTIONS, NULL },
> @@ -859,7 +848,7 @@ static const struct sectioncheck sectioncheck[] = {
> },
> {
> .fromsec = { ALL_PCI_INIT_SECTIONS, NULL },
> - .bad_tosec = { INIT_SECTIONS, NULL },
> + .bad_tosec = { ALL_INIT_SECTIONS, NULL },
> .mismatch = ANY_INIT_TO_ANY_EXIT,
> },
> {
> --
> 2.43.0
>
>
--
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 2/2] init/modpost: conditionally check section mismatch to __meminit*
2024-07-06 16:05 ` [PATCH 2/2] init/modpost: conditionally check section mismatch to __meminit* Masahiro Yamada
2024-07-06 18:07 ` Masahiro Yamada
@ 2024-07-07 0:01 ` Wei Yang
2024-07-07 5:58 ` Masahiro Yamada
1 sibling, 1 reply; 6+ messages in thread
From: Wei Yang @ 2024-07-07 0:01 UTC (permalink / raw)
To: Masahiro Yamada
Cc: linux-kbuild, linux-kernel, Wei Yang, Andrew Morton,
Arnd Bergmann, Nathan Chancellor, Nicolas Schier, linux-arch
On Sun, Jul 07, 2024 at 01:05:06AM +0900, Masahiro Yamada wrote:
>This reverts commit eb8f689046b8 ("Use separate sections for __dev/
>_cpu/__mem code/data").
>
>Check section mismatch to __meminit* only when CONFIG_MEMORY_HOTPLUG=y.
>
>With this change, the linker script and modpost become simpler, and we
>can get rid of the __ref annotations from the memory hotplug code.
>
Oh, totally get rid of .meminit.*. Looks nice.
Maybe we can plan a __ref cleanup after this on is merged?
Reviewed-by: Wei Yang <richard.weiyang@gmail.com>
>Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
>---
>
> include/asm-generic/vmlinux.lds.h | 18 ++----------------
> include/linux/init.h | 14 +++++++++-----
> scripts/mod/modpost.c | 19 ++++---------------
> 3 files changed, 15 insertions(+), 36 deletions(-)
>
>diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
>index 62b4cb0462e6..c23f7d0645ad 100644
>--- a/include/asm-generic/vmlinux.lds.h
>+++ b/include/asm-generic/vmlinux.lds.h
>@@ -141,14 +141,6 @@
> * often happens at runtime)
> */
>
>-#if defined(CONFIG_MEMORY_HOTPLUG)
>-#define MEM_KEEP(sec) *(.mem##sec)
>-#define MEM_DISCARD(sec)
>-#else
>-#define MEM_KEEP(sec)
>-#define MEM_DISCARD(sec) *(.mem##sec)
>-#endif
>-
> #ifndef CONFIG_HAVE_DYNAMIC_FTRACE_NO_PATCHABLE
> #define KEEP_PATCHABLE KEEP(*(__patchable_function_entries))
> #define PATCHABLE_DISCARDS
>@@ -357,7 +349,6 @@
> *(.data..decrypted) \
> *(.ref.data) \
> *(.data..shared_aligned) /* percpu related */ \
>- MEM_KEEP(init.data*) \
> *(.data.unlikely) \
> __start_once = .; \
> *(.data.once) \
>@@ -523,7 +514,6 @@
> /* __*init sections */ \
> __init_rodata : AT(ADDR(__init_rodata) - LOAD_OFFSET) { \
> *(.ref.rodata) \
>- MEM_KEEP(init.rodata) \
> } \
> \
> /* Built-in module parameters. */ \
>@@ -574,8 +564,7 @@
> *(.text.unknown .text.unknown.*) \
> NOINSTR_TEXT \
> *(.ref.text) \
>- *(.text.asan.* .text.tsan.*) \
>- MEM_KEEP(init.text*) \
>+ *(.text.asan.* .text.tsan.*)
>
>
> /* sched.text is aling to function alignment to secure we have same
>@@ -682,7 +671,6 @@
> #define INIT_DATA \
> KEEP(*(SORT(___kentry+*))) \
> *(.init.data .init.data.*) \
>- MEM_DISCARD(init.data*) \
> KERNEL_CTORS() \
> MCOUNT_REC() \
> *(.init.rodata .init.rodata.*) \
>@@ -690,7 +678,6 @@
> TRACE_SYSCALLS() \
> KPROBE_BLACKLIST() \
> ERROR_INJECT_WHITELIST() \
>- MEM_DISCARD(init.rodata) \
> CLK_OF_TABLES() \
> RESERVEDMEM_OF_TABLES() \
> TIMER_OF_TABLES() \
>@@ -708,8 +695,7 @@
>
> #define INIT_TEXT \
> *(.init.text .init.text.*) \
>- *(.text.startup) \
>- MEM_DISCARD(init.text*)
>+ *(.text.startup)
>
> #define EXIT_DATA \
> *(.exit.data .exit.data.*) \
>diff --git a/include/linux/init.h b/include/linux/init.h
>index b2e9dfff8691..ee1309473bc6 100644
>--- a/include/linux/init.h
>+++ b/include/linux/init.h
>@@ -84,11 +84,15 @@
>
> #define __exit __section(".exit.text") __exitused __cold notrace
>
>-/* Used for MEMORY_HOTPLUG */
>-#define __meminit __section(".meminit.text") __cold notrace \
>- __latent_entropy
>-#define __meminitdata __section(".meminit.data")
>-#define __meminitconst __section(".meminit.rodata")
>+#ifdef CONFIG_MEMORY_HOTPLUG
>+#define __meminit
>+#define __meminitdata
>+#define __meminitconst
>+#else
>+#define __meminit __init
>+#define __meminitdata __initdata
>+#define __meminitconst __initconst
>+#endif
>
> /* For assembly routines */
> #define __HEAD .section ".head.text","ax"
>diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
>index 3e5313ed6065..8c8ad7485f73 100644
>--- a/scripts/mod/modpost.c
>+++ b/scripts/mod/modpost.c
>@@ -776,17 +776,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.setup", ".init.rodata", ".init.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_INIT_SECTIONS INIT_SECTIONS, ALL_XXXINIT_SECTIONS
>+#define ALL_INIT_SECTIONS ".init.*"
> #define ALL_EXIT_SECTIONS ".exit.*"
>
> #define DATA_SECTIONS ".data", ".data.rel"
>@@ -797,9 +794,7 @@ static void check_section(const char *modname, struct elf_info *elf,
> ".fixup", ".entry.text", ".exception.text", \
> ".coldtext", ".softirqentry.text"
>
>-#define INIT_SECTIONS ".init.*"
>-
>-#define ALL_TEXT_SECTIONS ".init.text", ".meminit.text", ".exit.text", \
>+#define ALL_TEXT_SECTIONS ".init.text", ".exit.text", \
> TEXT_SECTIONS, OTHER_TEXT_SECTIONS
>
> enum mismatch {
>@@ -839,12 +834,6 @@ static const struct sectioncheck sectioncheck[] = {
> .bad_tosec = { ALL_INIT_SECTIONS, ALL_EXIT_SECTIONS, NULL },
> .mismatch = TEXTDATA_TO_ANY_INIT_EXIT,
> },
>-/* Do not reference init code/data from meminit code/data */
>-{
>- .fromsec = { ALL_XXXINIT_SECTIONS, NULL },
>- .bad_tosec = { INIT_SECTIONS, NULL },
>- .mismatch = XXXINIT_TO_SOME_INIT,
>-},
> /* Do not use exit code/data from init code */
> {
> .fromsec = { ALL_INIT_SECTIONS, NULL },
>@@ -859,7 +848,7 @@ static const struct sectioncheck sectioncheck[] = {
> },
> {
> .fromsec = { ALL_PCI_INIT_SECTIONS, NULL },
>- .bad_tosec = { INIT_SECTIONS, NULL },
>+ .bad_tosec = { ALL_INIT_SECTIONS, NULL },
> .mismatch = ANY_INIT_TO_ANY_EXIT,
> },
> {
>--
>2.43.0
--
Wei Yang
Help you, Help me
^ permalink raw reply [flat|nested] 6+ messages in thread