On Fri, 9 Oct 2009, Lee Schermerhorn wrote:
> > > +static void hugetlb_register_all_nodes(void)
> > > +{
> > > + int nid;
> > > +
> > > + for (nid = 0; nid < nr_node_ids; nid++) {
> > > + struct node *node = &node_devices[nid];
> > > + if (node->sysdev.id == nid)
> > > + hugetlb_register_node(node);
> > > + }
> >
> > This looks like another use of for_each_node_mask over N_HIGH_MEMORY. I
> > previously asked if the check for node->sysdev.id == nid is still
> > necessary at this point?
>
>
> Sorry. The check for sysdev.id == nid is there to ensure that this node
> sysdev has been registered when this function is called. nr_node_ids is
> the maximum node id seen so far, but we can't assume that all nodes
> 0..nr_node_ids are present/on-line.
>
> As for using for_each_node_mask: I think that would be OK. This code
> works because hugetlb_register_node() filters out nodes w/o memory; so
> only visiting nodes with memory should work as well. We can change this
> [for consistency] with an incremental patch, if you like.
>
> I'd hate to respin V11 for just this. But, if we have to for other
> reasons, I'll [try to remember to] do this.
>
I don't think it's necessary for a v11, I'd like to see this patchset
(perhaps minus patch 12/12 until we figure out whether it's actually
needed or not) added to -mm and then work on it there. This particular
case is only a small cleanup, but my curiosity really laid more in why
node->sysdev.id == nid was necessary instead of simply using
for_each_node_mask(nid, node_states[N_HIGH_MEMORY]) since that should
certainly be a subset of for_each_online_node(nid).
Thanks for the clarification, we can do an incremental patch on -mm once
it's merged.
--
To unsubscribe from this list: send the line "unsubscribe linux-numa" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
