Re: [Qemu-devel] [PATCH v3 3/5] numa: reduce code duplication by adding helper numa_get_node_for_cpu()

[Andrew Jones]

On Mon, Jul 04, 2016 at 09:06:16AM +0200, Igor Mammedov wrote:
> On Fri, 1 Jul 2016 14:30:12 +0200
> Andrew Jones <address@hidden> 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