Re: [patch] x86: set hotpluggable nodes in nodes_possible_map

[Haicheng Li]

David Rientjes wrote:
> On Thu, 21 Jan 2010, Haicheng Li wrote:
>
> > 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)
>
> Nack, we don't need to add yet another nodemask because you're having 
> trouble finding a new name for a cpu_nodes_parsed.  It would be perfectly 
Hey Dave, why do you think it's just a naming issue?

What I'm concerning is that your assumption of cpu_nodes_parsed use is wrong,
cpu_nodes_parsed is needed anyway since its semantics represent the node with cpu affinity rather 
than memless node, that's also why I originally doubted cpu_node_parsed cannot handle hotplug node.
we also need hp_nodes_parsed to represent the node with hotpluggable memory region, just like why we 
need nodes_parsed to repsent node with mem on.

The code could be straightforward and easy to undertand in this way.

> acceptable to rename nodes_parsed to mem_nodes and cpu_nodes_parsed to 
> no_mem_nodes, which are even shorter.  But we definitely don't need 
> another nodemask and I think we're convoluting the bug fix here way too 
> much.  Regardless of the future nomenclature, let's please merge my patch 
> to fix the outstanding kernel issue and then propose an alternative naming 
It's important to fix the outstanding kernel issue, that's also why I keep shooting down this issue. 
I wanna fix it thru a right way rather than just get it fixed.


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