> >> + long node_size;
> >> + unsigned long i;
> >> + unsigned long max_node = numa_max_node();
> >> + /*
> >> + * start from node and find the next memory node
> >> + */
> >> +restart:
> >> + for (i = node + 1; i <= max_node; i++) {
> >> + node_size = numa_node_size(i, NULL);
> >> + if (node_size > 0)
> >> + return i;
> >> + }
> >> + node = -1;
> >> + goto restart;
> >> +}
> >
> > I would rewrite the above without goto, and possibility of stacking in
> > an infinite loop. Something like this should work:
> >
> > for (i = node + 1; i <= max_node + node; i++) {
> > node_size = numa_node_size(i % (max_node + 1), NULL);
> > if (node_size > 0)
> > break;
> > }
> >
> > return i % (max_node + 1);
> >
>
>
> I didn't quiet follow this. not all nodes can have memory and node
> numbers are discontiguous.
I understand, the above loop searches for the first node with memory,
and returns it. It cannot end up in the infinite loop in case if there
are bugs where numa_node_size() are zero for all nodes for example.
Also, it does not have gotos.
>
> >> +
> >> +static unsigned long get_first_mem_node(void)
> >> +{
> >> + return get_next_mem_node(-1);
> >
> > Next after the last node would make more sense:
> >
> > return get_next_mem_node(numa_max_node());
> >
> >
>
> Yes, that would work, but is that really useful? We would essentially
> skip the for loop in first iteration set node = -1 internally and do the
> for loop again.
It is useful in terms that get_next_mem_node() is supplied with real
nodes. Also, it works better with the version of loop that I am
proposing above: "max_node + node" when node == -1 does not yield the
desired result.
Thanks,
Pasha
