[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [Qemu-devel] [PATCH v2 12/24] numa: add numa_[has_]node_i
From: |
Igor Mammedov |
Subject: |
Re: [Qemu-ppc] [Qemu-devel] [PATCH v2 12/24] numa: add numa_[has_]node_id() wrappers |
Date: |
Fri, 5 May 2017 22:04:16 +0200 |
On Fri, 5 May 2017 14:12:26 -0300
Eduardo Habkost <address@hidden> wrote:
> On Fri, May 05, 2017 at 11:06:02AM +0200, Andrew Jones wrote:
> > On Fri, May 05, 2017 at 10:09:18AM +0200, Igor Mammedov wrote:
> > > On Fri, 5 May 2017 11:45:22 +1000
> > > David Gibson <address@hidden> 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 <address@hidden>
> > > >
> > > > Reviewed-by: David Gibson <address@hidden>
> > > >
> > > > 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.
>
> If expression length is a problem, we can just use an extra
> variable:
>
> CPUArchId *slot = &machine->possible_cpus->cpus[cs->cpu_index];
> if (slot->props.has_node_id && slot->props.node_id == FOO) ...
>
>
> > 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.
>
> I don't mind that much, but I still prefer the version without
> the wrappers.
considering 3vs1 for wrapper less variant, I'll return it back.
but respin will have to wait till Tuesday due to holiday over here.
- [Qemu-ppc] [PATCH v2 14/24] spapr: get numa node mapping from possible_cpus instead of numa_get_node_for_cpu(), (continued)
[Qemu-ppc] [PATCH v2 16/24] QMP: include CpuInstanceProperties into query_cpus output output, Igor Mammedov, 2017/05/03
[Qemu-ppc] [PATCH v2 17/24] tests: numa: add case for QMP command query-cpus, Igor Mammedov, 2017/05/03
[Qemu-ppc] [PATCH v2 18/24] numa: remove no longer used numa_get_node_for_cpu(), Igor Mammedov, 2017/05/03
[Qemu-ppc] [PATCH v2 20/24] machine: call machine init from wrapper, Igor Mammedov, 2017/05/03
[Qemu-ppc] [PATCH v2 19/24] numa: remove no longer need numa_post_machine_init(), Igor Mammedov, 2017/05/03