From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60948) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1d6ZBn-0004Vm-CD for qemu-devel@nongnu.org; Fri, 05 May 2017 05:06:16 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1d6ZBm-0007AU-9V for qemu-devel@nongnu.org; Fri, 05 May 2017 05:06:15 -0400 Date: Fri, 5 May 2017 11:06:02 +0200 From: Andrew Jones Message-ID: <20170505090602.c2u7ugdh65rdjrs6@hawk.localdomain> References: <1493816238-33120-1-git-send-email-imammedo@redhat.com> <1493816238-33120-13-git-send-email-imammedo@redhat.com> <20170505014522.GK14413@umbus.fritz.box> <20170505100918.181acc8b@nial.brq.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170505100918.181acc8b@nial.brq.redhat.com> Subject: Re: [Qemu-devel] [PATCH v2 12/24] numa: add numa_[has_]node_id() wrappers List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Mammedov Cc: David Gibson , Peter Maydell , Eduardo Habkost , qemu-devel@nongnu.org, qemu-arm@nongnu.org, qemu-ppc@nongnu.org, Shannon Zhao , Paolo Bonzini On Fri, May 05, 2017 at 10:09:18AM +0200, Igor Mammedov wrote: > On Fri, 5 May 2017 11:45:22 +1000 > David Gibson wrote: > > > On Wed, May 03, 2017 at 02:57:06PM +0200, Igor Mammedov wrote: > > > wrappers should make access to [has]node_id fields more readable > > > > > > Signed-off-by: Igor Mammedov > > > > Reviewed-by: David Gibson > > > > Correct, though I'm not sure it actually simplifies things that much. > > Maybe more in future patches, though. > that's what Drew insisted on, and even though I prefer other way around > I won't stall series arguing about styling issues, > so here this patch goes. My argument in the last review of this series was that references like machine->possible_cpus->cpus[cs->cpu_index].props.has_node_id and machine->possible_cpus->cpus[cs->cpu_index].props.node_id are quite long, and only differ by 'has_', making it tough to easily recognize. But, if nobody, but me, sees value in changing them to numa_has_node_id(machine->possible_cpus, cs->cpu_index) and numa_node_id(machine->possible_cpus, cs->cpu_index) then I won't insist. Thanks, drew > > > > > > --- > > > follow up patches will use this wrappers > > > v2: > > > - add wrappers (Drew) > > > --- > > > include/sysemu/numa.h | 10 ++++++++++ > > > numa.c | 2 +- > > > 2 files changed, 11 insertions(+), 1 deletion(-) > > > > > > diff --git a/include/sysemu/numa.h b/include/sysemu/numa.h > > > index 46ea6c7..98d01e6 100644 > > > --- a/include/sysemu/numa.h > > > +++ b/include/sysemu/numa.h > > > @@ -35,4 +35,14 @@ uint32_t numa_get_node(ram_addr_t addr, Error **errp); > > > /* on success returns node index in numa_info, > > > * on failure returns nb_numa_nodes */ > > > int numa_get_node_for_cpu(int idx); > > > + > > > +static inline bool numa_has_node_id(const CPUArchIdList *possible_cpus, int idx) > > > +{ > > > + return possible_cpus->cpus[idx].props.has_node_id; > > > +} > > > + > > > +static inline int numa_node_id(const CPUArchIdList *possible_cpus, int idx) > > > +{ > > > + return possible_cpus->cpus[idx].props.node_id; > > > +} > > > #endif > > > diff --git a/numa.c b/numa.c > > > index c7e3e0a..872ee0d 100644 > > > --- a/numa.c > > > +++ b/numa.c > > > @@ -394,7 +394,7 @@ void parse_numa_opts(MachineState *ms) > > > > > > possible_cpus = mc->possible_cpu_arch_ids(ms); > > > for (i = 0; i < possible_cpus->len; i++) { > > > - if (possible_cpus->cpus[i].props.has_node_id) { > > > + if (numa_has_node_id(possible_cpus, i)) { > > > break; > > > } > > > } > > > >