Re: [RFC patch] delete improper hot pluggable code of memory affinity

[Yasunori Goto <y-goto@jp.fujitsu.com>]

Hi, Luming-san.

> The current kernel code *wrongly* interprets Hot Pluggable bit of
> Memory Affinity Structure (SRAT table in ACPI spec).

I'm not sure your patch is correct or not yet, but I would like
to tell you a critical point about the definition of 
Memory Affinity Structure.

The spec says the Enable bit of Memory Affinity Structure means that
the contents of its memory affinity structure is only VALID.
It doesn't mean memory is really connected at the area.
It means only that OS can read the entry.

When the enabled bit and hot pluggable bit is set on, 
it may mean that the area may be hot-added after boot up.
So, kernel must check e820 or efi to confirm that memory is
really connected.

If you already know it, sorry for noise....
Just for your information.

Thanks.



> if Hot Pluggable bit is set and CONFIG_MEMORY_HOTPLUG_SPARSE is NOT
> set, the memory Affinity will
> be ignored. And a faked Node will be used...
> 
> An alternative is to enable CONFIG_MEMORY_HOTPLUG_SPARSE *always*
> along with acpi_numa_memory_affinity_init.
> Please decide which one is appropriate.
> 
> The downside of this patch is *some useful info* is lost and a follow
> up patch is needed.
> 
> **The patch is enclosed in text attachment*
> **Using web client to send the patch* *
> **below is for review, please apply attached  patch*/
> 
> Thanks,
> Luming
> 
> 
> Signed-off-by: Yu Luming <luming.yu@intel.com>
> 
>  srat_64.c |   16 ----------------
>  1 file changed, 16 deletions(-)
> 
> 
> diff --git a/arch/x86/mm/srat_64.c b/arch/x86/mm/srat_64.c
> index 2dfcbf9..82423e5 100644
> --- a/arch/x86/mm/srat_64.c
> +++ b/arch/x86/mm/srat_64.c
> @@ -172,11 +172,6 @@ acpi_numa_processor_affinity_init(struct
> acpi_srat_cpu_affinity *pa)
>  	       pxm, apic_id, node);
>  }
> 
> -#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[]
>   * This code supports one contiguous hot add area per node
> @@ -249,9 +244,6 @@ acpi_numa_memory_affinity_init(struct
> acpi_srat_mem_affinity *ma)
>  	}
>  	if ((ma->flags & ACPI_SRAT_MEM_ENABLED) == 0)
>  		return;
> -
> -	if ((ma->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE) && !save_add_info())
> -		return;
>  	start = ma->base_address;
>  	end = start + ma->length;
>  	pxm = ma->proximity_domain;
> @@ -291,14 +283,6 @@ acpi_numa_memory_affinity_init(struct
> acpi_srat_mem_affinity *ma)
>  	e820_register_active_regions(node, start >> PAGE_SHIFT,
>  				     end >> PAGE_SHIFT);
> 
> -	if (ma->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE) {
> -		update_nodes_add(node, start, end);
> -		/* restore nodes[node] */
> -		*nd = oldnode;
> -		if ((nd->start | nd->end) == 0)
> -			node_clear(node, nodes_parsed);
> -	}
> -
>  	node_memblk_range[num_node_memblks].start = start;
>  	node_memblk_range[num_node_memblks].end = end;
>  	memblk_nodeid[num_node_memblks] = node;

-- 
Yasunori Goto