Re: [PATCH v1 08/11] mm/memory_hotplug: simplify page onlining

[Michal Hocko]

On Wed 19-08-20 12:11:54, David Hildenbrand wrote:
> We don't allow to offline memory with holes, all boot memory is online,
> and all hotplugged memory cannot have holes.
> 
> We can now simplify onlining of pages. As we only allow to online/offline
> full sections and sections always span full MAX_ORDER_NR_PAGES, we can just
> process MAX_ORDER - 1 pages without further special handling.
> 
> The number of onlined pages simply corresponds to the number of pages we
> were requested to online.
> 
> While at it, refine the comment regarding the callback not exposing all
> pages to the buddy.
> 
> Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> Cc: Michal Hocko <mhocko@xxxxxxxx>
> Cc: Wei Yang <richard.weiyang@xxxxxxxxxxxxxxxxx>
> Cc: Baoquan He <bhe@xxxxxxxxxx>
> Cc: Pankaj Gupta <pankaj.gupta.linux@xxxxxxxxx>
> Cc: Oscar Salvador <osalvador@xxxxxxx>
> Signed-off-by: David Hildenbrand <david@xxxxxxxxxx>

Acked-by: Michal Hocko <mhocko@xxxxxxxx>

> ---
>  mm/memory_hotplug.c | 38 ++++++++++----------------------------
>  1 file changed, 10 insertions(+), 28 deletions(-)
> 
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index 0011a1115381c..3aba0d956f9b1 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -617,31 +617,22 @@ void generic_online_page(struct page *page, unsigned int order)
>  }
>  EXPORT_SYMBOL_GPL(generic_online_page);
>  
> -static int online_pages_range(unsigned long start_pfn, unsigned long nr_pages,
> -			void *arg)
> +static void online_pages_range(unsigned long start_pfn, unsigned long nr_pages)
>  {
>  	const unsigned long end_pfn = start_pfn + nr_pages;
>  	unsigned long pfn;
> -	int order;
>  
>  	/*
> -	 * Online the pages. The callback might decide to keep some pages
> -	 * PG_reserved (to add them to the buddy later), but we still account
> -	 * them as being online/belonging to this zone ("present").
> +	 * Online the pages in MAX_ORDER - 1 aligned chunks. The callback might
> +	 * decide to not expose all pages to the buddy (e.g., expose them
> +	 * later). We account all pages as being online and belonging to this
> +	 * zone ("present").
>  	 */
> -	for (pfn = start_pfn; pfn < end_pfn; pfn += 1ul << order) {
> -		order = min(MAX_ORDER - 1, get_order(PFN_PHYS(end_pfn - pfn)));
> -		/* __free_pages_core() wants pfns to be aligned to the order */
> -		if (WARN_ON_ONCE(!IS_ALIGNED(pfn, 1ul << order)))
> -			order = 0;
> -		(*online_page_callback)(pfn_to_page(pfn), order);
> -	}
> +	for (pfn = start_pfn; pfn < end_pfn; pfn += MAX_ORDER_NR_PAGES)
> +		(*online_page_callback)(pfn_to_page(pfn), MAX_ORDER - 1);
>  
>  	/* mark all involved sections as online */
>  	online_mem_sections(start_pfn, end_pfn);
> -
> -	*(unsigned long *)arg += nr_pages;
> -	return 0;
>  }
>  
>  /* check which state of node_states will be changed when online memory */
> @@ -795,7 +786,6 @@ int __ref online_pages(unsigned long pfn, unsigned long nr_pages,
>  		       int online_type, int nid)
>  {
>  	unsigned long flags;
> -	unsigned long onlined_pages = 0;
>  	struct zone *zone;
>  	int need_zonelists_rebuild = 0;
>  	int ret;
> @@ -831,19 +821,11 @@ int __ref online_pages(unsigned long pfn, unsigned long nr_pages,
>  		setup_zone_pageset(zone);
>  	}
>  
> -	ret = walk_system_ram_range(pfn, nr_pages, &onlined_pages,
> -		online_pages_range);
> -	if (ret) {
> -		/* not a single memory resource was applicable */
> -		if (need_zonelists_rebuild)
> -			zone_pcp_reset(zone);
> -		goto failed_addition;
> -	}
> -
> -	zone->present_pages += onlined_pages;
> +	online_pages_range(pfn, nr_pages);
> +	zone->present_pages += nr_pages;
>  
>  	pgdat_resize_lock(zone->zone_pgdat, &flags);
> -	zone->zone_pgdat->node_present_pages += onlined_pages;
> +	zone->zone_pgdat->node_present_pages += nr_pages;
>  	pgdat_resize_unlock(zone->zone_pgdat, &flags);
>  
>  	/*
> -- 
> 2.26.2
> 

-- 
Michal Hocko
SUSE Labs