Re: [PATCH 3/6] mm, memory_hotplug: drop arch_free_nodedata

[David Hildenbrand]

On 27.01.22 09:53, Michal Hocko wrote:
> From: Michal Hocko <mhocko@xxxxxxxx>
> 
> Prior to "mm: handle uninitialized numa nodes gracefully" memory hotplug
> used to allocate pgdat when memory has been added to a node
> (hotadd_init_pgdat) arch_free_nodedata has been only used in the
> failure path because once the pgdat is exported (to be visible
> by NODA_DATA(nid)) it cannot really be freed because there is no
> synchronization available for that.
> 
> pgdat is allocated for each possible nodes now so the memory hotplug
> doesn't need to do the ever use arch_free_nodedata so drop it.
> 
> This patch doesn't introduce any functional change.
> 
> Acked-by: Rafael Aquini <raquini@xxxxxxxxxx>
> Signed-off-by: Michal Hocko <mhocko@xxxxxxxx>
> ---
>  arch/ia64/mm/discontig.c       |  5 -----
>  include/linux/memory_hotplug.h |  3 ---
>  mm/memory_hotplug.c            | 10 ----------
>  3 files changed, 18 deletions(-)
> 
> diff --git a/arch/ia64/mm/discontig.c b/arch/ia64/mm/discontig.c
> index dd0cf4834eaa..73d0db36edb6 100644
> --- a/arch/ia64/mm/discontig.c
> +++ b/arch/ia64/mm/discontig.c
> @@ -615,11 +615,6 @@ pg_data_t * __init arch_alloc_nodedata(int nid)
>  	return memblock_alloc(size, SMP_CACHE_BYTES);
>  }
>  
> -void arch_free_nodedata(pg_data_t *pgdat)
> -{
> -	kfree(pgdat);
> -}
> -
>  void arch_refresh_nodedata(int update_node, pg_data_t *update_pgdat)
>  {
>  	pgdat_list[update_node] = update_pgdat;
> diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
> index cdd66bfdf855..60f09d3ebb3d 100644
> --- a/include/linux/memory_hotplug.h
> +++ b/include/linux/memory_hotplug.h
> @@ -24,17 +24,14 @@ struct vmem_altmap;
>   * node_data[nid] = kzalloc() works well. But it depends on the architecture.
>   *
>   * In general, generic_alloc_nodedata() is used.
> - * Now, arch_free_nodedata() is just defined for error path of node_hot_add.
>   *
>   */
>  extern pg_data_t *arch_alloc_nodedata(int nid);
> -extern void arch_free_nodedata(pg_data_t *pgdat);
>  extern void arch_refresh_nodedata(int nid, pg_data_t *pgdat);
>  
>  #else /* CONFIG_HAVE_ARCH_NODEDATA_EXTENSION */
>  
>  #define arch_alloc_nodedata(nid)	generic_alloc_nodedata(nid)
> -#define arch_free_nodedata(pgdat)	generic_free_nodedata(pgdat)
>  
>  #ifdef CONFIG_NUMA
>  /*
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index fc991831d296..875cdc7ffa58 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -1217,16 +1217,6 @@ static pg_data_t __ref *hotadd_init_pgdat(int nid)
>  	return pgdat;
>  }
>  
> -static void rollback_node_hotadd(int nid)
> -{
> -	pg_data_t *pgdat = NODE_DATA(nid);
> -
> -	arch_refresh_nodedata(nid, NULL);
> -	free_percpu(pgdat->per_cpu_nodestats);
> -	arch_free_nodedata(pgdat);
> -}

As mentioned, maybe we want to rip out rollback_node_hotadd() in the
previous patch.

Acked-by: David Hildenbrand <david@xxxxxxxxxx>

-- 
Thanks,

David / dhildenb