Re: [PATCH 1/3] x86: remove MEMORY_HOTPLUG_RESERVE related code

[Mel Gorman <mel@csn.ul.ie>]

On Tue, May 12, 2009 at 10:29:13PM -0700, Yinghai Lu wrote:
> Mel Gorman wrote:
> > On Fri, May 08, 2009 at 11:45:49PM -0700, Yinghai Lu wrote:
> >> after
> >> | commit b263295dbffd33b0fbff670720fa178c30e3392a
> >> | Author: Christoph Lameter <clameter@sgi.com>
> >> | Date:   Wed Jan 30 13:30:47 2008 +0100
> >> |
> >> |    x86: 64-bit, make sparsemem vmemmap the only memory model
> >>
> >> we don't have MEMORY_HOTPLUG_RESERVE anymore.
> >>
> >> remove related dead code.
> >>
> > 
> > Good spot, this removes a nice amount of code. The changelog could say
> > more though, how about?
> > 
> > =====
> > Historically, x86-64 had an architecture-specific method for memory hotplug
> > whereby it scanned the SRAT for physical memory ranges that could be
> > potentially used for memory hot-add later. By reserving those ranges
> > without physical memory, the memmap would be allocated and left dormant
> > until needed. This depended on the DISCONTIG memory model which has been
> > removed so the code implementing HOTPLUG_RESERVE is now dead.
> > 
> > This patch removes the dead code used by MEMORY_HOTPLUG_RESERVE
> thanks will use that.
> > =====
> > 
> >> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> >>
> >> ---
> >>  arch/x86/include/asm/numa_64.h |    3 -
> >>  arch/x86/mm/numa_64.c          |    5 --
> >>  arch/x86/mm/srat_64.c          |   63 +++++++------------------------------
> >>  include/linux/mm.h             |    2 -
> >>  mm/page_alloc.c                |   69 -----------------------------------------
> >>  5 files changed, 12 insertions(+), 130 deletions(-)
> >>
> >> Index: linux-2.6/arch/x86/include/asm/numa_64.h
> >> ===================================================================
> >> --- linux-2.6.orig/arch/x86/include/asm/numa_64.h
> >> +++ linux-2.6/arch/x86/include/asm/numa_64.h
> >> @@ -17,9 +17,6 @@ extern int compute_hash_shift(struct boo
> >>  extern void numa_init_array(void);
> >>  extern int numa_off;
> >>  
> >> -extern void srat_reserve_add_area(int nodeid);
> >> -extern int hotadd_percent;
> >> -
> >>  extern s16 apicid_to_node[MAX_LOCAL_APIC];
> >>  
> >>  extern unsigned long numa_free_all_bootmem(void);
> >> Index: linux-2.6/arch/x86/mm/numa_64.c
> >> ===================================================================
> >> --- linux-2.6.orig/arch/x86/mm/numa_64.c
> >> +++ linux-2.6/arch/x86/mm/numa_64.c
> >> @@ -272,9 +272,6 @@ void __init setup_node_bootmem(int nodei
> >>  		reserve_bootmem_node(NODE_DATA(nodeid), bootmap_start,
> >>  				 bootmap_pages<<PAGE_SHIFT, BOOTMEM_DEFAULT);
> >>  
> >> -#ifdef CONFIG_ACPI_NUMA
> >> -	srat_reserve_add_area(nodeid);
> >> -#endif
> >>  	node_set_online(nodeid);
> >>  }
> >>  
> >> @@ -608,8 +605,6 @@ static __init int numa_setup(char *opt)
> >>  #ifdef CONFIG_ACPI_NUMA
> >>  	if (!strncmp(opt, "noacpi", 6))
> >>  		acpi_numa = -1;
> >> -	if (!strncmp(opt, "hotadd=", 7))
> >> -		hotadd_percent = simple_strtoul(opt+7, NULL, 10);
> > 
> > Documentation/x86/x86_64/boot-options.txt now needs to be updated to
> > remove the documentation on hotadd=.
> > 
> > Instead of ignoring the option, should a warning now be printed saying the
> > option is deprecated?
> 
> that is dead for 2.6.27 (?), 2.6.28, 2.6.29, ...
> guess we could remove that directly...
> 

If someone did a distro upgrade, the kernel version would jump from before
2.6.27 to 2.6.29 or 2.6.30. If someone is depending on the behaviour, they
might like to hear about it rather than having it myseriously "fail".

> > 
> >>  #endif
> >>  	return 0;
> >>  }
> >> Index: linux-2.6/arch/x86/mm/srat_64.c
> >> ===================================================================
> >> --- linux-2.6.orig/arch/x86/mm/srat_64.c
> >> +++ linux-2.6/arch/x86/mm/srat_64.c
> >> @@ -31,8 +31,6 @@ static nodemask_t nodes_parsed __initdat
> >>  static nodemask_t cpu_nodes_parsed __initdata;
> >>  static struct bootnode nodes[MAX_NUMNODES] __initdata;
> >>  static struct bootnode nodes_add[MAX_NUMNODES];
> >> -static int found_add_area __initdata;
> >> -int hotadd_percent __initdata = 0;
> >>  
> >>  static int num_node_memblks __initdata;
> >>  static struct bootnode node_memblk_range[NR_NODE_MEMBLKS] __initdata;
> >> @@ -66,9 +64,6 @@ static __init void cutoff_node(int i, un
> >>  {
> >>  	struct bootnode *nd = &nodes[i];
> >>  
> >> -	if (found_add_area)
> >> -		return;
> >> -
> >>  	if (nd->start < start) {
> >>  		nd->start = start;
> >>  		if (nd->end < nd->start)
> >> @@ -86,7 +81,6 @@ static __init void bad_srat(void)
> >>  	int i;
> >>  	printk(KERN_ERR "SRAT: SRAT not used.\n");
> >>  	acpi_numa = -1;
> >> -	found_add_area = 0;
> >>  	for (i = 0; i < MAX_LOCAL_APIC; i++)
> >>  		apicid_to_node[i] = NUMA_NO_NODE;
> >>  	for (i = 0; i < MAX_NUMNODES; i++)
> >> @@ -182,24 +176,21 @@ acpi_numa_processor_affinity_init(struct
> >>  	       pxm, apic_id, node);
> >>  }
> >>  
> >> -static int update_end_of_memory(unsigned long end) {return -1;}
> >> -static int hotadd_enough_memory(struct bootnode *nd) {return 1;}
> >>  #ifdef CONFIG_MEMORY_HOTPLUG_SPARSE
> >>  static inline int save_add_info(void) {return 1;}
> >>  #else
> >>  static inline int save_add_info(void) {return 0;}
> >>  #endif
> >>  /*
> >> - * Update nodes_add and decide if to include add are in the zone.
> >> - * Both SPARSE and RESERVE need nodes_add information.
> >> - * This code supports one contiguous hot add area per node.
> >> + * Update nodes_add[]
> >> + * This code supports one contiguous hot add area per node
> >>   */
> >> -static int __init
> >> -reserve_hotadd(int node, unsigned long start, unsigned long end)
> >> +static void __init
> >> +update_nodes_add(int node, unsigned long start, unsigned long end)
> >>  {
> > 
> > It's now very unclear what the purpose of this function is. I'm guessing it
> > should be something like
> > 
> > validate_hotadd_region()
> > This validates that the region of memory described by SRAT as suitable
> > for use with memory hot-add is sane
> > 
> > What it was for was to validate that the SRAT looked sane and then push out the
> > end of the node boundaries so that the memmap would get allocated. However,
> > because we are no longer pushing out the node boundaries, is this doing
> > anything useful at all any more? For sparsemem, memory-hotadd allocates
> > the memmap as it required.
> 
> but it does update nodes_add range.
> 

D'oh, of course. Thanks for correcting me.

-- 
Mel Gorman
Part-time Phd Student                          Linux Technology Center
University of Limerick                         IBM Dublin Software Lab