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

[David Rientjes]

On Thu, 21 Jan 2010, Haicheng Li wrote:

> > It's pointless to add another nodemask, the semantics of cpu_nodes_parsed is
> > perfectly legitimate for hotpluggable nodes as well.  Instead of fixating on
> > the name, look at the code that uses it.
> 
> I'm not meant to be rude, but it's illogical excuse. that it is now used by a
> single function doesn't mean that it will never be used by others in future or
> it is only useful for that single function.
> 

It will not be used by a single function in this context for any valid 
purpose, hp_nodes_parsed is completely unnecessary and overkill to address 
a problem that is a relative one-liner.  You're completely missing the 
points that (i) no other SRAT parsing code cares about distinguishing only 
nodes with hotpluggable memory ranges, and (ii) no other SRAT parsing code 
even cares about distinguishing memoryless nodes with only cpus attached.  
Thus, we can use cpu_nodes_allowed to represent _all_ nodes without online 
memory ranges (and you can even rename it to no_mem_nodes later, if you 
want).

This discussion is becoming very annoying because you have constantly 
proposed new patches (I think 4 now?) that are much more complex and 
without any real consistent idea behind them.  Your latest proposal is to 
add a nodemask because you speculate that someday it will become useful; 
the truth of the matter is that we don't need to do anything with them 
beyond detection so the bit gets set in nodes_possible_map.

Ingo should not need to look through what is becoming an extremely long 
discussion for a bugfix that should be applied for rc5 because we're 
getting _very_ late in the 2.6.33 release cycle.  Do you expect Ingo to 
push your fix to Linus with the rationale that "maybe someday we'll use 
this new nodemask even though it may be rc5 and nobody knows what we'd 
ever use it for"?  Is that appropriate for -stable candidates as well?

You've already tested my patch that this thread was restarted with and it 
works, so let's fix the bug.  Then, later, you can rename cpu_nodes_parsed 
to no_mems_nodes, which I'd agree with.  You may even try to seperate the 
hotpluggable nodes out into their own nodemask, but I trust that the x86 
maintainers will be looking for some rationale behind that other than "it 
may one day be useful."
--
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/