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

[Luming Yu]

Without a fix like my proposal, I have seen NUMA configure disabled by
kernel (due to the code the patch deletes) on a system with Enabled
bit set , and Hotplug-able bit cleared, and
CONFIG_MEMORY_HOTPLUG_SPARSE disabled.

On Fri, Jul 17, 2009 at 1:16 PM, Yasunori Goto<y-goto@xxxxxxxxxxxxxx> wrote:
>
> 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@xxxxxxxxx>
>>
>> Â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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/