linux-kernel - Re: [patch] x86: set hotpluggable nodes in nodes_possible

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: <4B57C2DD.3050402@linux.intel.com>
Date:	Thu, 21 Jan 2010 10:58:37 +0800
From:	Haicheng Li <haicheng.li@...ux.intel.com>
To:	David Rientjes <rientjes@...gle.com>
CC:	Ingo Molnar <mingo@...hat.com>, Yinghai Lu <yinghai@...nel.org>,
	"H. Peter Anvin" <hpa@...or.com>,
	Thomas Gleixner <tglx@...utronix.de>, x86@...nel.org,
	Andi Kleen <andi@...stfloor.org>, linux-kernel@...r.kernel.org
Subject: Re: [patch] x86: set hotpluggable nodes in nodes_possible_map

David Rientjes wrote:
> On Thu, 21 Jan 2010, Haicheng Li wrote:
> 
>> let's add more changes to fix naming issue as well since it's too confusing
>> for people to understand
>> the code logic. how about below patch?
> 
> That should be a seperate change; there's a bugfix here (my patch) and 
> then a cleanup patch that you could make incrementally on mine.  I don't 
> personally like the name "rest_nodes_parsed" since it's poor English, I 
> suggest renaming nodes_parsed to mem_nodes_parsed as I originally asked 
> and then cpu_nodes_parsed to acpi_nodes_parsed or something similiar.

IMHO, name acpi_nodes_parsed is not appropriate for this intention as nodes_parsed comes from acpi too.

Think about it again, it's better _NOT_ to mix up cpu_nodes_parsed and hotplugpable_nodes together.
We cannot assume cpu_nodes_parsed has no other use in future, like possibly cpu hotplug emulation. I 
think that a clean and straightforward way is to keep nodes with hotpluggable range in a separated 
nodemask, like "hp_nodes_parsed", which could also be used for future mem hotplug emulation. Then 
node data corresponding to nodes_parsed is kept in nodes[], node data for hp_nodes_parsed is kept in 
nodes_add[].

> Ingo, the following is my bugfix patch that addresses the issue at 
> http://patchwork.kernel.org/patch/69499.  

Personally I don't like such patch with confusable info.
I think following patch would be more straightfoward and clean:

diff --git a/arch/x86/mm/srat_64.c b/arch/x86/mm/srat_64.c
index a271241..c5552ae 100644
--- a/arch/x86/mm/srat_64.c
+++ b/arch/x86/mm/srat_64.c
@@ -27,8 +27,18 @@ int acpi_numa __initdata;

  static struct acpi_table_slit *acpi_slit;

+/* nodes_parsed:
+ *  - nodes with memory on
+ * cpu_nodes_parsed:
+ *  - nodes with cpu on
+ * hp_nodes_parsed:
+ *  - nodes with hotpluggable memory region
+ *
+ * We union these tree nodemasks to get node_possible_map.
+ */
  static nodemask_t nodes_parsed __initdata;
  static nodemask_t cpu_nodes_parsed __initdata;
+static nodemask_t hp_nodes_parsed __initdata;
  static struct bootnode nodes[MAX_NUMNODES] __initdata;
  static struct bootnode nodes_add[MAX_NUMNODES];

@@ -229,9 +239,11 @@ update_nodes_add(int node, unsigned long start, unsigned long end)
  			printk(KERN_ERR "SRAT: Hotplug zone not continuous. Partly ignored\n");
  	}

-	if (changed)
+	if (changed) {
+		node_set(node, hp_nodes_parsed);
  		printk(KERN_INFO "SRAT: hot plug zone found %Lx - %Lx\n",
  				 nd->start, nd->end);
+	}
  }

  /* Callback for parsing of the Proximity Domain <-> Memory Area mappings */
@@ -380,8 +392,10 @@ int __init acpi_scan_nodes(unsigned long start, unsigned long end)
  		return -1;
  	}

-	/* Account for nodes with cpus and no memory */
+	/* Account for nodes with memory and cpus */
  	nodes_or(node_possible_map, nodes_parsed, cpu_nodes_parsed);
+	/* Account for nodes with hotpluggable memory regions */
+	nodes_or(node_possible_map, node_possible_map, hp_nodes_parsed);

  	/* Finally register nodes */
  	for_each_node_mask(i, node_possible_map)

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