Re: [PATCH] mm: make __paginginit based on CONFIG_MEMORY_HOTPLUG

[Pavel Tatashin]

On Tue, Jul 31, 2018 at 11:01 AM Oscar Salvador
<osalvador@xxxxxxxxxxxxxxxxxx> wrote:
>
> On Tue, Jul 31, 2018 at 10:53:52AM -0400, Pavel Tatashin wrote:
> > Thats correct on arches where no sparsemem setup_usemap() will not be
> > freed up. It is a tiny function, just a few instructions. Not a big
> > deal.
> >
> > Pavel
> > On Tue, Jul 31, 2018 at 10:51 AM Oscar Salvador
> > <osalvador@xxxxxxxxxxxxxxxxxx> wrote:
> > >
> > > On Tue, Jul 31, 2018 at 10:45:45AM -0400, Pavel Tatashin wrote:
> > > > Here the patch would look like this:
> > > >
> > > > From e640b32dbd329bba5a785cc60050d5d7e1ca18ce Mon Sep 17 00:00:00 2001
> > > > From: Pavel Tatashin <pasha.tatashin@xxxxxxxxxx>
> > > > Date: Tue, 31 Jul 2018 10:37:44 -0400
> > > > Subject: [PATCH] mm: remove __paginginit
> > > >
> > > > __paginginit is the same thing as __meminit except for platforms without
> > > > sparsemem, there it is defined as __init.
> > > >
> > > > Remove __paginginit and use __meminit. Use __ref in one single function
> > > > that merges __meminit and __init sections: setup_usemap().
> > > >
> > > > Signed-off-by: Pavel Tatashin <pasha.tatashin@xxxxxxxxxx>
> > >
> > > Uhm, I am probably missing something, but with this change, the functions will not be freed up
> > > while freeing init memory, right?
> > Thats correct on arches where no sparsemem setup_usemap() will not be
> > freed up. It is a tiny function, just a few instructions. Not a big
> > deal.
>
> I must be missing something.
>
> What about:
>
> calc_memmap_size
> free_area_init_node
> free_area_init_core
>
> These functions are marked with __meminit now.
> If we have CONFIG_PARSEMEM but not CONFIG_MEMORY_HOTPLUG, these functions will
> be left there.

I hope we free meminit section if no hotplug configured. If not, than
sure we should have something like what you suggest not only for these
functions, but for all other meminit functions in kernel.

>
> I mean, it is not that it is a big amount, but still.
>
> Do not we need something like:
>
> diff --git a/include/linux/init.h b/include/linux/init.h
> index 2538d176dd1f..3b3a88ba80ed 100644
> --- a/include/linux/init.h
> +++ b/include/linux/init.h
> @@ -83,8 +83,12 @@
>  #define __exit          __section(.exit.text) __exitused __cold notrace
>
>  /* Used for MEMORY_HOTPLUG */
> +#ifdef CONFIG_MEMORY_HOTPLUG
>  #define __meminit        __section(.meminit.text) __cold notrace \
>                                                   __latent_entropy
> +#else
> +#define __meminit       __init
> +#endif
>  #define __meminitdata    __section(.meminit.data)
>  #define __meminitconst   __section(.meminit.rodata)
>  #define __memexit        __section(.memexit.text) __exitused __cold notrace
>
> on top?
>
> Thanks
> --
> Oscar Salvador
> SUSE L3
>