[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [Qemu-devel] [PATCH v3 06/18] numa: mirror cpu to node ma
From: |
Eduardo Habkost |
Subject: |
Re: [Qemu-ppc] [Qemu-devel] [PATCH v3 06/18] numa: mirror cpu to node mapping in MachineState::possible_cpus |
Date: |
Tue, 30 May 2017 11:04:01 -0300 |
User-agent: |
Mutt/1.8.0 (2017-02-23) |
On Tue, May 30, 2017 at 02:13:29PM +0200, Igor Mammedov wrote:
> On Fri, 26 May 2017 12:46:25 -0300
> Eduardo Habkost <address@hidden> wrote:
>
> > On Wed, May 10, 2017 at 01:29:50PM +0200, Igor Mammedov wrote:
> > [...]
> > > diff --git a/hw/core/machine.c b/hw/core/machine.c
> > > index 2482c63..420c8c4 100644
> > > --- a/hw/core/machine.c
> > > +++ b/hw/core/machine.c
> > > @@ -389,6 +389,102 @@ HotpluggableCPUList
> > > *machine_query_hotpluggable_cpus(MachineState *machine)
> > [...]
> > > +void machine_set_cpu_numa_node(MachineState *machine,
> > > + const CpuInstanceProperties *props, Error
> > > **errp)
> > > +{
> > [...]
> > > + /* force board to initialize possible_cpus if it hasn't been done
> > > yet */
> > > + mc->possible_cpu_arch_ids(machine);
> > [...]
> > > diff --git a/numa.c b/numa.c
> > > index 7182481..7db5dde 100644
> > > --- a/numa.c
> > > +++ b/numa.c
> > > @@ -170,6 +170,7 @@ static void parse_numa_node(MachineState *ms,
> > > NumaNodeOptions *node,
> > > exit(1);
> > > }
> > > for (cpus = node->cpus; cpus; cpus = cpus->next) {
> > > + CpuInstanceProperties props;
> > > if (cpus->value >= max_cpus) {
> > > error_setg(errp,
> > > "CPU index (%" PRIu16 ")"
> > > @@ -178,6 +179,10 @@ static void parse_numa_node(MachineState *ms,
> > > NumaNodeOptions *node,
> > > return;
> > > }
> > > bitmap_set(numa_info[nodenr].node_cpu, cpus->value, 1);
> > > + props = mc->cpu_index_to_instance_props(ms, cpus->value);
> > > + props.node_id = nodenr;
> > > + props.has_node_id = true;
> > > + machine_set_cpu_numa_node(ms, &props, &error_fatal);
> >
> > This triggers a call to possible_cpu_arch_ids() before
> > nb_numa_nodes is set to the actual number of NUMA nodes in the
> > machine, breaking the "node_id = ... % nb_numa_nodes"
> > initialization logic in pc, virt, and spapr.
> Looking at it more, in current code this problem theoretically
> affects only partial mapping case. And even there it's masked by
> fixup to 0 node for non explicitly mapped CPUs and the bug which
> *) "[PATCH v2 3/5] numa: make sure that all cpus in has has_node_id set if
> numa is enabled"
> fixes. So invalid default values for non mapped cpus are stored
> in possible_cpus but not used nor visible elsewhere.
>
> [*] as side effect fixes invalid defaults in possible_cpus as well
> by fixup to 0 node that's moved from pre_plug handler.
>
> In future we are going to remove partial mapping with fixup to 0
> and fetch default mappings from possible_cpus where these wrong
> defaults won't exists (or matter) as CPUs are going to either
> 1) be all explicitly mapped (so all node_id values are overwritten)
> or
> 2) all default mapped (where defaults will be correct since no
> -numa cpu=/-numa node,cpus= were called), but that's fragile
> so I'd add calculate defaults at machine_run_board_init()
> time anyway.
Your analysis seems correct: it looks like just a case of fragile
code, not a bug that can be triggered with the current code.
--
Eduardo
- [Qemu-ppc] [PATCH v3 01/18] numa: move source of default CPUs to NUMA node mapping into boards, (continued)
- [Qemu-ppc] [PATCH v3 01/18] numa: move source of default CPUs to NUMA node mapping into boards, Igor Mammedov, 2017/05/10
- [Qemu-ppc] [PATCH v3 04/18] virt-arm: add node-id property to CPU, Igor Mammedov, 2017/05/10
- [Qemu-ppc] [PATCH v3 07/18] numa: do default mapping based on possible_cpus instead of node_cpu bitmaps, Igor Mammedov, 2017/05/10
- [Qemu-ppc] [PATCH v3 06/18] numa: mirror cpu to node mapping in MachineState::possible_cpus, Igor Mammedov, 2017/05/10
- Re: [Qemu-ppc] [Qemu-devel] [PATCH v3 06/18] numa: mirror cpu to node mapping in MachineState::possible_cpus, Igor Mammedov, 2017/05/30
- Re: [Qemu-ppc] [Qemu-devel] [PATCH v3 06/18] numa: mirror cpu to node mapping in MachineState::possible_cpus,
Eduardo Habkost <=
[Qemu-ppc] [PATCH v3 08/18] pc: get numa node mapping from possible_cpus instead of numa_get_node_for_cpu(), Igor Mammedov, 2017/05/10
[Qemu-ppc] [PATCH v3 09/18] spapr: get numa node mapping from possible_cpus instead of numa_get_node_for_cpu(), Igor Mammedov, 2017/05/10
[Qemu-ppc] [PATCH v3 10/18] virt-arm: get numa node mapping from possible_cpus instead of numa_get_node_for_cpu(), Igor Mammedov, 2017/05/10
[Qemu-ppc] [PATCH v3 11/18] QMP: include CpuInstanceProperties into query_cpus output output, Igor Mammedov, 2017/05/10
[Qemu-ppc] [PATCH v3 13/18] numa: remove no longer need numa_post_machine_init(), Igor Mammedov, 2017/05/10
[Qemu-ppc] [PATCH v3 12/18] tests: numa: add case for QMP command query-cpus, Igor Mammedov, 2017/05/10
[Qemu-ppc] [PATCH v3 14/18] machine: call machine init from wrapper, Igor Mammedov, 2017/05/10
[Qemu-ppc] [PATCH v3 15/18] numa: use possible_cpus for not mapped CPUs check, Igor Mammedov, 2017/05/10
[Qemu-ppc] [PATCH v3 16/18] numa: remove node_cpu bitmaps as they are no longer used, Igor Mammedov, 2017/05/10