Re: [PATCH 03/21] mm: remove CONFIG_HAVE_MEMBLOCK_NODE_MAP option
On 04/21/20 at 12:09pm, Mike Rapoport wrote:
> > > diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> > > index fc0aad0bc1f5..e67dc501576a 100644
> > > --- a/mm/memory_hotplug.c
> > > +++ b/mm/memory_hotplug.c
> > > @@ -1372,11 +1372,7 @@ check_pages_isolated_cb(unsigned long start_pfn,
> > > unsigned long nr_pages,
> > >
> > > static int __init cmdline_parse_movable_node(char *p)
> > > {
> > > -#ifdef CONFIG_HAVE_MEMBLOCK_NODE_MAP
> > > movable_node_enabled = true;
> > > -#else
> > > - pr_warn("movable_node parameter depends on
> > > CONFIG_HAVE_MEMBLOCK_NODE_MAP to work properly\n");
> > > -#endif
> >
> > Wondering if this change will impact anything. Before, those ARCHes with
> > CONFIG_HAVE_MEMBLOCK_NODE_MAP support movable_node. With this patch
> > applied, those ARCHes which don't support CONFIG_HAVE_MEMBLOCK_NODE_MAP
> > can also have 'movable_node' specified in kernel cmdline.
> >
> > > return 0;
> > > }
> > > early_param("movable_node", cmdline_parse_movable_node);
> > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > > index 1ac775bfc9cf..4530e9cfd9f7 100644
> > > --- a/mm/page_alloc.c
> > > +++ b/mm/page_alloc.c
> > > @@ -335,7 +335,6 @@ static unsigned long nr_kernel_pages __initdata;
> > > static unsigned long nr_all_pages __initdata;
> > > static unsigned long dma_reserve __initdata;
> > >
> > > -#ifdef CONFIG_HAVE_MEMBLOCK_NODE_MAP
> > > static unsigned long arch_zone_lowest_possible_pfn[MAX_NR_ZONES]
> > > __initdata;
> > > static unsigned long arch_zone_highest_possible_pfn[MAX_NR_ZONES]
> > > __initdata;
> > > static unsigned long required_kernelcore __initdata;
> >
> > Does it mean those ARCHes which don't support
> > CONFIG_HAVE_MEMBLOCK_NODE_MAP before, will have 'kernelcore=' and
> > 'movablecore=' now, and will have MOVABLE zone?
>
> I hesitated a lot about whether to hide the kernelcore/movablecore and
> related code behind an #ifdef.
> In the end I've decided to keep the code compiled unconditionally as it
> is anyway __init and no sane person would pass "kernelcore=" to the
> kernel on a UMA system.
I see. Then maybe can do something if someone complains about it
in the future, e.g warn out with a message in
cmdline_parse_movable_node(), cmdline_parse_kernelcore().
>
> > > @@ -348,7 +347,6 @@ static bool mirrored_kernelcore __meminitdata;
> > > /* movable_zone is the "real" zone pages in ZONE_MOVABLE are taken from
> > > */
> > > int movable_zone;
> > > EXPORT_SYMBOL(movable_zone);
> > > -#endif /* CONFIG_HAVE_MEMBLOCK_NODE_MAP */
> > >
> > > #if MAX_NUMNODES > 1
> > > unsigned int nr_node_ids __read_mostly = MAX_NUMNODES;
> > > @@ -1499,8 +1497,7 @@ void __free_pages_core(struct page *page, unsigned
> > > int order)
> > > __free_pages(page, order);
> > > }
> > >
> > > -#if defined(CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID) || \
> > > - defined(CONFIG_HAVE_MEMBLOCK_NODE_MAP)
> > > +#ifdef CONFIG_NEED_MULTIPLE_NODES
> > >
> > > static struct mminit_pfnnid_cache early_pfnnid_cache __meminitdata;
> > >
> > > @@ -1542,7 +1539,7 @@ int __meminit early_pfn_to_nid(unsigned long pfn)
> > >
> > > return nid;
> > > }
> > > -#endif
> > > +#endif /* CONFIG_NEED_MULTIPLE_NODES */
> > >
> > > #ifdef CONFIG_NODES_SPAN_OTHER_NODES
> > > /* Only safe to use early in boot when initialisation is single-threaded
> > > */
> > > @@ -5924,7 +5921,6 @@ void __ref build_all_zonelists(pg_data_t *pgdat)
> > > static bool __meminit
> > > overlap_memmap_init(unsigned long zone, unsigned long *pfn)
> > > {
> > > -#ifdef CONFIG_HAVE_MEMBLOCK_NODE_MAP
> > > static struct memblock_region *r;
> > >
> > > if (mirrored_kernelcore && zone == ZONE_MOVABLE) {
> > > @@ -5940,7 +5936,6 @@ overlap_memmap_init(unsigned long zone, unsigned
> > > long *pfn)
> > > return true;
> > > }
> > > }
> > > -#endif
> > > return false;
> > > }
> > >
> > > @@ -6573,8 +6568,7 @@ static unsigned long __init
> > > zone_absent_pages_in_node(int nid,
> > > return nr_absent;
> > > }
> > >
> > > -#else /* CONFIG_HAVE_MEMBLOCK_NODE_MAP */
> > > -static inline unsigned long __init zone_spanned_pages_in_node(int nid,
> > > +static inline unsigned long __init compat_zone_spanned_pages_in_node(int
> > > nid,
> >
> > Is it compact zone which has continuous memory region, and the
> > compat here is typo? Or it's compatible zone? The name seems a little
> > confusing, or I miss something.
>
> It's 'compat' from 'compatibility'. This is kinda "the old way" and the
> version that was defined when CONFIG_HAVE_MEMBLOCK_NODE_MAP=y is the
> "new way", so I picked 'compat' for backwards compatibility.
> Anyway, it will go away later in pacth 19.
Got it, thanks for telling.
>
> > > unsigned long zone_type,
> > > unsigned long node_start_pfn,
> > > unsigned long node_end_pfn,
> > > @@ -6593,7 +6587,7 @@ static inline unsigned long __init
> > >
Re: [PATCH 03/21] mm: remove CONFIG_HAVE_MEMBLOCK_NODE_MAP option
On Tue, Apr 21, 2020 at 12:23:16PM +0800, Baoquan He wrote:
> On 04/12/20 at 10:48pm, Mike Rapoport wrote:
> > From: Mike Rapoport
> >
> > The CONFIG_HAVE_MEMBLOCK_NODE_MAP is used to differentiate initialization
> > of nodes and zones structures between the systems that have region to node
> > mapping in memblock and those that don't.
> >
> > Currently all the NUMA architectures enable this option and for the
> > non-NUMA systems we can presume that all the memory belongs to node 0 and
> > therefore the compile time configuration option is not required.
> >
> > The remaining few architectures that use DISCONTIGMEM without NUMA are
> > easily updated to use memblock_add_node() instead of memblock_add() and
> > thus have proper correspondence of memblock regions to NUMA nodes.
> >
> > Still, free_area_init_node() must have a backward compatible version
> > because its semantics with and without CONFIG_HAVE_MEMBLOCK_NODE_MAP is
> > different. Once all the architectures will use the new semantics, the
> > entire compatibility layer can be dropped.
> >
> > To avoid addition of extra run time memory to store node id for
> > architectures that keep memblock but have only a single node, the node id
> > field of the memblock_region is guarded by CONFIG_NEED_MULTIPLE_NODES and
> > the corresponding accessors presume that in those cases it is always 0.
> >
> > Signed-off-by: Mike Rapoport
> > ---
> ...
> > diff --git a/include/linux/memblock.h b/include/linux/memblock.h
> > index 6bc37a731d27..45abfc54da37 100644
> > --- a/include/linux/memblock.h
> > +++ b/include/linux/memblock.h
> > @@ -50,7 +50,7 @@ struct memblock_region {
> > phys_addr_t base;
> > phys_addr_t size;
> > enum memblock_flags flags;
> > -#ifdef CONFIG_HAVE_MEMBLOCK_NODE_MAP
> > +#ifdef CONFIG_NEED_MULTIPLE_NODES
> > int nid;
> > #endif
> > };
> > @@ -215,7 +215,6 @@ static inline bool memblock_is_nomap(struct
> > memblock_region *m)
> > return m->flags & MEMBLOCK_NOMAP;
> > }
> >
> > -#ifdef CONFIG_HAVE_MEMBLOCK_NODE_MAP
> > int memblock_search_pfn_nid(unsigned long pfn, unsigned long *start_pfn,
> > unsigned long *end_pfn);
> > void __next_mem_pfn_range(int *idx, int nid, unsigned long *out_start_pfn,
> > @@ -234,7 +233,6 @@ void __next_mem_pfn_range(int *idx, int nid, unsigned
> > long *out_start_pfn,
> > #define for_each_mem_pfn_range(i, nid, p_start, p_end, p_nid)
> > \
> > for (i = -1, __next_mem_pfn_range(, nid, p_start, p_end, p_nid); \
> > i >= 0; __next_mem_pfn_range(, nid, p_start, p_end, p_nid))
> > -#endif /* CONFIG_HAVE_MEMBLOCK_NODE_MAP */
> >
> > #ifdef CONFIG_DEFERRED_STRUCT_PAGE_INIT
> > void __next_mem_pfn_range_in_zone(u64 *idx, struct zone *zone,
> > @@ -310,10 +308,10 @@ void __next_mem_pfn_range_in_zone(u64 *idx, struct
> > zone *zone,
> > for_each_mem_range_rev(i, , , \
> >nid, flags, p_start, p_end, p_nid)
> >
> > -#ifdef CONFIG_HAVE_MEMBLOCK_NODE_MAP
> > int memblock_set_node(phys_addr_t base, phys_addr_t size,
> > struct memblock_type *type, int nid);
> >
> > +#ifdef CONFIG_NEED_MULTIPLE_NODES
> > static inline void memblock_set_region_node(struct memblock_region *r, int
> > nid)
> > {
> > r->nid = nid;
> > @@ -332,7 +330,7 @@ static inline int memblock_get_region_node(const struct
> > memblock_region *r)
> > {
> > return 0;
> > }
> > -#endif /* CONFIG_HAVE_MEMBLOCK_NODE_MAP */
> > +#endif /* CONFIG_NEED_MULTIPLE_NODES */
> >
> > /* Flags for memblock allocation APIs */
> > #define MEMBLOCK_ALLOC_ANYWHERE(~(phys_addr_t)0)
> > diff --git a/include/linux/mm.h b/include/linux/mm.h
> > index a404026d14d4..5903bbbdb336 100644
> > --- a/include/linux/mm.h
> > +++ b/include/linux/mm.h
> > @@ -2344,9 +2344,8 @@ static inline unsigned long get_num_physpages(void)
> > return phys_pages;
> > }
> >
> > -#ifdef CONFIG_HAVE_MEMBLOCK_NODE_MAP
> > /*
> > - * With CONFIG_HAVE_MEMBLOCK_NODE_MAP set, an architecture may initialise
> > its
> > + * Using memblock node mappings, an architecture may initialise its
> > * zones, allocate the backing mem_map and account for memory holes in a
> > more
> > * architecture independent manner. This is a substitute for creating the
> > * zone_sizes[] and zholes_size[] arrays and passing them to
> > @@ -2367,9 +2366,6 @@ static inline unsigned long get_num_physpages(void)
> > * registered physical page range. Similarly
> > * sparse_memory_present_with_active_regions() calls memory_present() for
> > * each range when SPARSEMEM is enabled.
> > - *
> > - * See mm/page_alloc.c for more information on each function exposed by
> > - * CONFIG_HAVE_MEMBLOCK_NODE_MAP.
> > */
> > extern void free_area_init_nodes(unsigned long *max_zone_pfn);
> > unsigned long node_map_pfn_alignment(void);
> > @@ -2384,13 +2380,9 @@ extern void free_bootmem_with_active_regions(int nid,
> >
Re: [PATCH 03/21] mm: remove CONFIG_HAVE_MEMBLOCK_NODE_MAP option
On 04/12/20 at 10:48pm, Mike Rapoport wrote:
> From: Mike Rapoport
>
> The CONFIG_HAVE_MEMBLOCK_NODE_MAP is used to differentiate initialization
> of nodes and zones structures between the systems that have region to node
> mapping in memblock and those that don't.
>
> Currently all the NUMA architectures enable this option and for the
> non-NUMA systems we can presume that all the memory belongs to node 0 and
> therefore the compile time configuration option is not required.
>
> The remaining few architectures that use DISCONTIGMEM without NUMA are
> easily updated to use memblock_add_node() instead of memblock_add() and
> thus have proper correspondence of memblock regions to NUMA nodes.
>
> Still, free_area_init_node() must have a backward compatible version
> because its semantics with and without CONFIG_HAVE_MEMBLOCK_NODE_MAP is
> different. Once all the architectures will use the new semantics, the
> entire compatibility layer can be dropped.
>
> To avoid addition of extra run time memory to store node id for
> architectures that keep memblock but have only a single node, the node id
> field of the memblock_region is guarded by CONFIG_NEED_MULTIPLE_NODES and
> the corresponding accessors presume that in those cases it is always 0.
>
> Signed-off-by: Mike Rapoport
> ---
...
> diff --git a/include/linux/memblock.h b/include/linux/memblock.h
> index 6bc37a731d27..45abfc54da37 100644
> --- a/include/linux/memblock.h
> +++ b/include/linux/memblock.h
> @@ -50,7 +50,7 @@ struct memblock_region {
> phys_addr_t base;
> phys_addr_t size;
> enum memblock_flags flags;
> -#ifdef CONFIG_HAVE_MEMBLOCK_NODE_MAP
> +#ifdef CONFIG_NEED_MULTIPLE_NODES
> int nid;
> #endif
> };
> @@ -215,7 +215,6 @@ static inline bool memblock_is_nomap(struct
> memblock_region *m)
> return m->flags & MEMBLOCK_NOMAP;
> }
>
> -#ifdef CONFIG_HAVE_MEMBLOCK_NODE_MAP
> int memblock_search_pfn_nid(unsigned long pfn, unsigned long *start_pfn,
> unsigned long *end_pfn);
> void __next_mem_pfn_range(int *idx, int nid, unsigned long *out_start_pfn,
> @@ -234,7 +233,6 @@ void __next_mem_pfn_range(int *idx, int nid, unsigned
> long *out_start_pfn,
> #define for_each_mem_pfn_range(i, nid, p_start, p_end, p_nid)
> \
> for (i = -1, __next_mem_pfn_range(, nid, p_start, p_end, p_nid); \
>i >= 0; __next_mem_pfn_range(, nid, p_start, p_end, p_nid))
> -#endif /* CONFIG_HAVE_MEMBLOCK_NODE_MAP */
>
> #ifdef CONFIG_DEFERRED_STRUCT_PAGE_INIT
> void __next_mem_pfn_range_in_zone(u64 *idx, struct zone *zone,
> @@ -310,10 +308,10 @@ void __next_mem_pfn_range_in_zone(u64 *idx, struct zone
> *zone,
> for_each_mem_range_rev(i, , , \
> nid, flags, p_start, p_end, p_nid)
>
> -#ifdef CONFIG_HAVE_MEMBLOCK_NODE_MAP
> int memblock_set_node(phys_addr_t base, phys_addr_t size,
> struct memblock_type *type, int nid);
>
> +#ifdef CONFIG_NEED_MULTIPLE_NODES
> static inline void memblock_set_region_node(struct memblock_region *r, int
> nid)
> {
> r->nid = nid;
> @@ -332,7 +330,7 @@ static inline int memblock_get_region_node(const struct
> memblock_region *r)
> {
> return 0;
> }
> -#endif /* CONFIG_HAVE_MEMBLOCK_NODE_MAP */
> +#endif /* CONFIG_NEED_MULTIPLE_NODES */
>
> /* Flags for memblock allocation APIs */
> #define MEMBLOCK_ALLOC_ANYWHERE (~(phys_addr_t)0)
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index a404026d14d4..5903bbbdb336 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -2344,9 +2344,8 @@ static inline unsigned long get_num_physpages(void)
> return phys_pages;
> }
>
> -#ifdef CONFIG_HAVE_MEMBLOCK_NODE_MAP
> /*
> - * With CONFIG_HAVE_MEMBLOCK_NODE_MAP set, an architecture may initialise its
> + * Using memblock node mappings, an architecture may initialise its
> * zones, allocate the backing mem_map and account for memory holes in a more
> * architecture independent manner. This is a substitute for creating the
> * zone_sizes[] and zholes_size[] arrays and passing them to
> @@ -2367,9 +2366,6 @@ static inline unsigned long get_num_physpages(void)
> * registered physical page range. Similarly
> * sparse_memory_present_with_active_regions() calls memory_present() for
> * each range when SPARSEMEM is enabled.
> - *
> - * See mm/page_alloc.c for more information on each function exposed by
> - * CONFIG_HAVE_MEMBLOCK_NODE_MAP.
> */
> extern void free_area_init_nodes(unsigned long *max_zone_pfn);
> unsigned long node_map_pfn_alignment(void);
> @@ -2384,13 +2380,9 @@ extern void free_bootmem_with_active_regions(int nid,
> unsigned long max_low_pfn);
> extern void sparse_memory_present_with_active_regions(int nid);
>
> -#endif /* CONFIG_HAVE_MEMBLOCK_NODE_MAP */
> -
> -#if !defined(CONFIG_HAVE_MEMBLOCK_NODE_MAP) && \
> -!defined(CONFIG_HAVE_ARCH_EARLY_PFN_TO_NID)
>
