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

[Wei Yang <richard.weiyang@gmail.com>]

On Sat, Jul 06, 2024 at 10:50:25PM +0900, Masahiro Yamada wrote:
>On Sat, Jul 6, 2024 at 3:12 PM Wei Yang <richard.weiyang@gmail.com> wrote:
>>
>> On Fri, Jul 05, 2024 at 06:54:56AM +0000, Wei Yang wrote:
>> >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@gmail.com> 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@gmail.com>
>> >>> CC: Mike Rapoport (IBM) <rppt@kernel.org>
>> >>> ---
>> >>>  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@ravnborg.org>
>> >>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
>> >>
>> >>That is what we did in the Linux 2.6.* era, which had much worse
>> >>section mismatch coverage.
>> >>
>> >
>> >Masahiro
>> >
>> >If you would give me some suggestions, I'd appreciate it a lot.
>> >
>> >The original thing I want to do is to put function __free_pages_core() in
>> >__meminit section, since this function is only used by __init functions and
>> >in memory_hotplug.c.  This means it is save to release it if
>> >CONFIG_MEMORY_HOTPLUG is set.
>> >
>> >Then I add __meminit to function __free_pages_core() and face the warning from
>> >modpost.
>> >
>> >  WARNING: modpost: vmlinux: section mismatch in reference: generic_online_page+0xa (section: .text) -> __free_pages_core (section: .meminit.text)
>> >
>> >A .text function calls init code is not proper. Then I add __meminit to
>> >generic_online_page too. Then I face this warning from modpost.
>> >
>> >  WARNING: modpost: vmlinux: generic_online_page: EXPORT_SYMBOL used for init symbol. Remove __init or EXPORT_SYMBOL.
>> >
>>
>> I guess I found the correct way.
>>
>> Add __ref to generic_online_page to not issue a warning.
>
>
>
>Yes, __ref is used to bypass the section mismatch check.
>

I am think whether __ref is providing a gate to escape the check of modpost?

>Some functions in mm/memory_hotplug.c are annotated as __ref
>to reference __meminit functions.
>
>Adding __ref is the easy solution.
>
>
>
>Having said that, I started to think
>eb8f689046b857874e964463619f09df06d59fad was the wrong decision.
>I will revert it.
>

Oh, I finally understand it... didn't think that was a wrong decision :-(

>
>
>
>
>
>
>
>
>
>
>-- 
>Best Regards
>Masahiro Yamada

-- 
Wei Yang
Help you, Help me