[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAGM2reZ+KhsuFhOVvJzRkQO=66TosvxDW0BYAXNf8Gw8zoRQXQ@mail.gmail.com>
Date: Tue, 31 Jul 2018 11:06:48 -0400
From: Pavel Tatashin <pasha.tatashin@...cle.com>
To: osalvador@...hadventures.net
Cc: Andrew Morton <akpm@...ux-foundation.org>,
Michal Hocko <mhocko@...e.com>,
Vlastimil Babka <vbabka@...e.cz>,
kirill.shutemov@...ux.intel.com, iamjoonsoo.kim@....com,
Mel Gorman <mgorman@...e.de>,
Souptick Joarder <jrdr.linux@...il.com>,
Linux Memory Management List <linux-mm@...ck.org>,
LKML <linux-kernel@...r.kernel.org>, osalvador@...e.de
Subject: Re: [PATCH] mm: make __paginginit based on CONFIG_MEMORY_HOTPLUG
On Tue, Jul 31, 2018 at 11:01 AM Oscar Salvador
<osalvador@...hadventures.net> 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@...hadventures.net> 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@...cle.com>
> > > > 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@...cle.com>
> > >
> > > 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
>
Powered by blists - more mailing lists