Re: [PATCH 2/3] modpost: .meminit.* is not in init section when CONFIG_MEMORY_HOTPLUG set

[Wei Yang]

On Wed, Jul 03, 2024 at 11:44:38PM +0900, Masahiro Yamada wrote:
>On Wed, Jul 3, 2024 at 8:40 AM Wei Yang <richard.weiyang@xxxxxxxxx> wrote:
>>
>> .meminit.* is not put into init section when CONFIG_MEMORY_HOTPLUG is
>> set, since we define MEM_KEEP()/MEM_DISCARD() according to
>> CONFIG_MEMORY_HOTPLUG.
>>
>> Signed-off-by: Wei Yang <richard.weiyang@xxxxxxxxx>
>> CC: Mike Rapoport (IBM) <rppt@xxxxxxxxxx>
>> ---
>>  scripts/mod/modpost.c | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>
>
>
>NACK.
>
>
>The section mismatch is performed _unconditionally_.
>
>
>
>In the old days, we did this depending on relevant CONFIG options.
>It was more than 15 years ago that we stopped doing that.
>
>
>See this:
>
>
>commit eb8f689046b857874e964463619f09df06d59fad
>Author: Sam Ravnborg <sam@xxxxxxxxxxxx>
>Date:   Sun Jan 20 20:07:28 2008 +0100
>
>    Use separate sections for __dev/__cpu/__mem code/data
>
>
>
>
>So, if you wanted to check this only when CONFIG_MEMORY_HOTPLUG=n,
>you would need to add #ifdef CONFIG_MEMORY_HOTPLUG to include/linux/init.h
>

You mean something like this?

diff --git a/include/linux/init.h b/include/linux/init.h
index 58cef4c2e59a..388f0a4c34e9 100644
--- a/include/linux/init.h
+++ b/include/linux/init.h
@@ -85,10 +85,12 @@
 #define __exit          __section(".exit.text") __exitused __cold notrace
 
 /* Used for MEMORY_HOTPLUG */
+#ifndef CONFIG_MEMORY_HOTPLUG
 #define __meminit        __section(".meminit.text") __cold notrace \
 						  __latent_entropy
 #define __meminitdata    __section(".meminit.data")
 #define __meminitconst   __section(".meminit.rodata")
+#endif
 
 /* For assembly routines */
 #define __HEAD		.section	".head.text","ax"

>That is what we did in the Linux 2.6.* era, which had much worse
>section mismatch coverage.
>

I guess you mean this is not a good practice.

Then I am confused how we do the mismatch check unconditionally?

After commit 

commit eb8f689046b857874e964463619f09df06d59fad
Author: Sam Ravnborg <sam@xxxxxxxxxxxx>
Date:   Sun Jan 20 20:07:28 2008 +0100

    Use separate sections for __dev/__cpu/__mem code/data

Sections .meminit.* will be put into INIT_SECTION conditionally, but we always
do the mismatch check unconditionally. It will report mismatch when .meminit.*
is not in INIT_SECTION. It looks not correct to me.

Maybe I am not fully understand your message. Would you mind explaining more
on what is the correct way to do?

-- 
Wei Yang
Help you, Help me