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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20090717135847.38B3.E1E9C6FF@jp.fujitsu.com>
Date:	Fri, 17 Jul 2009 14:16:52 +0900
From:	Yasunori Goto <y-goto@...fujitsu.com>
To:	Luming Yu <luming.yu@...il.com>
Cc:	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [RFC patch] delete improper hot pluggable code of memory affinity


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@...el.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 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/