On Mon, Jul 04, 2016 at 09:06:16AM +0200, Igor Mammedov wrote:
> On Fri, 1 Jul 2016 14:30:12 +0200
> Andrew Jones <[email protected]> wrote:
> > On Fri, Jul 01, 2016 at 01:50:24PM +0200, Igor Mammedov wrote:
> > > --- a/hw/arm/virt-acpi-build.c
> > > +++ b/hw/arm/virt-acpi-build.c
> > > @@ -426,11 +426,9 @@ build_srat(GArray *table_data, BIOSLinker
> > > *linker, VirtGuestInfo *guest_info) uint32_t *cpu_node =
> > > g_malloc0(guest_info->smp_cpus * sizeof(uint32_t));
> > > for (i = 0; i < guest_info->smp_cpus; i++) {
> > > - for (j = 0; j < nb_numa_nodes; j++) {
> > > - if (test_bit(i, numa_info[j].node_cpu)) {
> > > + j = numa_get_node_for_cpu(i);
> > > + if (j < nb_numa_nodes) {
> > > cpu_node[i] = j;
> >
> > I think this, and all other occurrences, would read nicer like
> >
> > if (numa_enabled()) {
> > cpu_node[i] = numa_get_node_for_cpu(i);
> > }
> it would be nicer but it could be a guest visible change as in case of
> if cpu is not in numa_info[].node_cpu then old code won't do
> assignment, if done as suggested it will assign whatever value
> numa_get_node_for_cpu(i) returns.
I see. So how about creating
static inline bool numa_node_is_valid(int node)
{
return node >= 0 && node < nb_numa_nodes;
}
and then using that for the condition, instead of the inequality.
It's not a huge improvement, so take it or leave it.
Thanks,
drew
