[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: |
Eduardo Habkost |
Subject: |
Re: [Qemu-ppc] [Qemu-devel] [PATCH v2 12/24] numa: add numa_[has_]node_id() wrappers |
Date: |
Fri, 5 May 2017 14:12:26 -0300 |
User-agent: |
Mutt/1.8.0 (2017-02-23) |
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.
--
Eduardo
- Re: [Qemu-ppc] [Qemu-devel] [PATCH v2 13/24] pc: 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