On Mon, Jun 30, 2014 at 11:26:10AM -0700, Nishanth Aravamudan wrote: [...] > > > - if (i == nb_numa_nodes) { > > > + if (i == max_numa_node) { > > > for (i = 0; i < max_cpus; i++) { > > > - set_bit(i, numa_info[i % nb_numa_nodes].node_cpu); > > > + do { > > > + j = (j + 1) % max_numa_node; > > > + } while (!numa_info[j].present); > > > > If you change it from "do { } while" to "while { }", you don't need to > > initialize j to -1. > > I don't think that's quite as simple as you make it out to be. If you > use a while() loop, we won't always increment j, which means once we've > found a present node, we'll always use that node? j here basically > represents the *last* used nodeid, which we don't want to use again when > we re-enter the for-loop, we want to use the next present nodeid. It > seems like the do {} while() does this fine? I could use a while() if I > added another increment outside the loop, as follows: > > if (i == max_numa_nodeid) { > for (i = 0, j = 0; i < max_cpus; i++) { > while (!numa_info[j].present) { > j = (j + 1) % (max_numa_nodeid); > } > set_bit(i, numa_info[j].node_cpu); > j = (j + 1) % (max_numa_nodeid); > } > } > > If you think that is cleaner, I'll use that version.
You are right, and your "do { } while" version looks cleaner than duplicating the "j = (j + 1) % (max_numa_nodeid)" line. -- Eduardo