linux-kernel - Re: [PATCH] x86/mm/srat_64.c: nodes_parsed should include all nodes detected by ACPI.

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: <86802c441001172230y137b4916h7d744a96ab75873d@mail.gmail.com>
Date:	Sun, 17 Jan 2010 22:30:14 -0800
From:	Yinghai Lu <yinghai@...nel.org>
To:	David Rientjes <rientjes@...gle.com>
Cc:	Haicheng Li <haicheng.li@...ux.intel.com>,
	"H. Peter Anvin" <hpa@...or.com>, Ingo Molnar <mingo@...hat.com>,
	Thomas Gleixner <tglx@...utronix.de>, x86@...nel.org,
	Andi Kleen <andi@...stfloor.org>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] x86/mm/srat_64.c: nodes_parsed should include all nodes 
	detected by ACPI.

On Sun, Jan 17, 2010 at 1:53 PM, David Rientjes <rientjes@...gle.com> wrote:
> On Fri, 15 Jan 2010, Haicheng Li wrote:
>
>> This is to fix the bug discussed in email thread:
>> http://patchwork.kernel.org/patch/69499/.
>>
>
> It would be better to include a brief synopsis of the problem in the
> changelog instead of a citation, otherwise it obfuscates the issue it
> solves.
>
>> Currently node_possible_map won't include the offlined node that has neither
>> CPU onlined nor MEM onlined at booting time. As a result, nr_node_ids won't be
>> equal to possible nodes.
>>
>> CC: Thomas Gleixner <tglx@...utronix.de>
>> CC: Ingo Molnar <mingo@...hat.com>
>> CC: H. Peter Anvin <hpa@...or.com>
>> CC: Andi Kleen <andi@...stfloor.org>
>> Signed-off-by: Haicheng Li <haicheng.li@...ux.intel.com>
>> ---
>>  arch/x86/mm/srat_64.c |   10 ++--------
>>  1 files changed, 2 insertions(+), 8 deletions(-)
>>
>> diff --git a/arch/x86/mm/srat_64.c b/arch/x86/mm/srat_64.c
>> index a271241..a5bc297 100644
>> --- a/arch/x86/mm/srat_64.c
>> +++ b/arch/x86/mm/srat_64.c
>> @@ -238,7 +238,7 @@ update_nodes_add(int node, unsigned long start, unsigned
>> long end)
>>  void __init
>>  acpi_numa_memory_affinity_init(struct acpi_srat_mem_affinity *ma)
>>  {
>> -     struct bootnode *nd, oldnode;
>> +     struct bootnode *nd;
>>       unsigned long start, end;
>>       int node, pxm;
>>       int i;
>> @@ -277,7 +277,6 @@ acpi_numa_memory_affinity_init(struct
>> acpi_srat_mem_affinity *ma)
>>               return;
>>       }
>>       nd = &nodes[node];
>> -     oldnode = *nd;
>>       if (!node_test_and_set(node, nodes_parsed)) {
>>               nd->start = start;
>>               nd->end = end;
>> @@ -291,13 +290,8 @@ acpi_numa_memory_affinity_init(struct
>> acpi_srat_mem_affinity *ma)
>>       printk(KERN_INFO "SRAT: Node %u PXM %u %lx-%lx\n", node, pxm,
>>              start, end);
>>
>> -     if (ma->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE) {
>> +     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;
>
> There're a couple of issues with this patch:
>
>  - it breaks CONFIG_MEMORY_HOTPLUG_SPARSE kernels when the SRAT reports
>   more than one entry for a single node id and the later entry does not
>   specify an address range, and

not sure this one.

>
>  - carrying the bit for the node over in nodes_parsed is inappropriate
>   since other x86 code depends on that map including only nodes with
>   memory such as e820_register_active_regions() and nodes_cover_memory().

assume if e820 is right, even srat table have a range for hot plug
aka the e820 have hole already. then e820_register_active_regions()
and nodes_cover_memory
should be ok even bootnode have some pre reserved range.

YH
--
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/