On Tue 29-11-16 15:08:10, Thomas Gleixner wrote: > On Mon, 28 Nov 2016, Michal Hocko wrote: > > On Sun 27-11-16 00:13:36, Sebastian Andrzej Siewior wrote: > > [...] > > > static void __init init_cpu_node_state(void) > > > { > > > - int cpu; > > > + int node; > > > > > > - for_each_online_cpu(cpu) > > > - node_set_state(cpu_to_node(cpu), N_CPU); > > > + for_each_online_node(node) > > > + node_set_state(node, N_CPU); > > > > Is this really correct? The point of the original code was to mark only > > those nodes which have at least one CPU. Or am I missing something? > > You're right. An online node does not necessarily have an online CPU. > > for_each_online_node(node) { > if (cpumask_weight(cpumask_of_node(node)) > 0) > node_set_state(node, N_CPU); > } > > is probably more correct. Yes, this looks correct. Considering that the same cpumask_weight is used in another function I guess a small helper would be nice. E.g. bool node_has_cpus(int node) { return cpumask_weight(cpumask_of_node(node)) > 0; } -- Michal Hocko SUSE Labs -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx"> email@xxxxxxxxx </a>