| Message ID | 1487928416-131310-1-git-send-email-imammedo@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Fri, 24 Feb 2017 10:26:56 +0100 Igor Mammedov <imammedo@redhat.com> wrote: > Threads within a core shouldn't be on different > NUMA nodes, so if user has misconfgured command > line, fail QEMU at start up to force user fix it. > > For now use the first thread on the core as source > of core's node-id. Later when cpu-numa refactoring > lands it will be switched to core's node-id from > possible_cpus[]. > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> ping, David could you review/merge maybe adding to commit message: " patch prevents the same issues as commit 20bb648d fixes, only instead of default mapping it adds check for manually configured NUMA node mapping " > --- > Enforcement is necessary to ensure that it would be > possible to map legacy CPU index based CLI mapping > to core based one and replace numa_info[XXX].node_cpu > with possible_cpus[] as storage for mapping info. > > CC: qemu-ppc@nongnu.org > CC: lvivier@redhat.com > CC: David Gibson <david@gibson.dropbear.id.au> > CC: thuth@redhat.com > CC: mdroth@linux.vnet.ibm.com > CC: agraf@suse.de > --- > hw/ppc/spapr_cpu_core.c | 23 +++++++++++++++-------- > 1 file changed, 15 insertions(+), 8 deletions(-) > > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c > index 55cd045..1499a8b 100644 > --- a/hw/ppc/spapr_cpu_core.c > +++ b/hw/ppc/spapr_cpu_core.c > @@ -50,8 +50,6 @@ static void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu, > Error **errp) > { > CPUPPCState *env = &cpu->env; > - CPUState *cs = CPU(cpu); > - int i; > > /* Set time-base frequency to 512 MHz */ > cpu_ppc_tb_init(env, SPAPR_TIMEBASE_FREQ); > @@ -70,12 +68,6 @@ static void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu, > } > } > > - /* Set NUMA node for the added CPUs */ > - i = numa_get_node_for_cpu(cs->cpu_index); > - if (i < nb_numa_nodes) { > - cs->numa_node = i; > - } > - > xics_cpu_setup(spapr->xics, cpu); > > qemu_register_reset(spapr_cpu_reset, cpu); > @@ -159,11 +151,13 @@ static void spapr_cpu_core_realize(DeviceState *dev, Error **errp) > const char *typename = object_class_get_name(scc->cpu_class); > size_t size = object_type_get_instance_size(typename); > Error *local_err = NULL; > + int core_node_id = numa_get_node_for_cpu(cc->core_id);; > void *obj; > int i, j; > > sc->threads = g_malloc0(size * cc->nr_threads); > for (i = 0; i < cc->nr_threads; i++) { > + int node_id; > char id[32]; > CPUState *cs; > > @@ -172,6 +166,19 @@ static void spapr_cpu_core_realize(DeviceState *dev, Error **errp) > object_initialize(obj, size, typename); > cs = CPU(obj); > cs->cpu_index = cc->core_id + i; > + > + /* Set NUMA node for the added CPUs */ > + node_id = numa_get_node_for_cpu(cs->cpu_index); > + if (node_id != core_node_id) { > + error_setg(&local_err, "Invalid node-id=%d of thread[cpu-index: %d]" > + " on CPU[core-id: %d, node-id: %d], node-id must be the same", > + node_id, cs->cpu_index, cc->core_id, core_node_id); > + goto err; > + } > + if (node_id < nb_numa_nodes) { > + cs->numa_node = node_id; > + } > + > snprintf(id, sizeof(id), "thread[%d]", i); > object_property_add_child(OBJECT(sc), id, obj, &local_err); > if (local_err) {
On Fri, Mar 03, 2017 at 06:41:04PM +0100, Igor Mammedov wrote: > On Fri, 24 Feb 2017 10:26:56 +0100 > Igor Mammedov <imammedo@redhat.com> wrote: > > > Threads within a core shouldn't be on different > > NUMA nodes, so if user has misconfgured command > > line, fail QEMU at start up to force user fix it. > > > > For now use the first thread on the core as source > > of core's node-id. Later when cpu-numa refactoring > > lands it will be switched to core's node-id from > > possible_cpus[]. > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > ping, > > David could you review/merge maybe adding to commit > message: > > " > patch prevents the same issues as commit 20bb648d > fixes, only instead of default mapping it adds > check for manually configured NUMA node mapping > " Ok, merged. I'll try to send that up today, just in time for the hard freeze. > > > --- > > Enforcement is necessary to ensure that it would be > > possible to map legacy CPU index based CLI mapping > > to core based one and replace numa_info[XXX].node_cpu > > with possible_cpus[] as storage for mapping info. > > > > CC: qemu-ppc@nongnu.org > > CC: lvivier@redhat.com > > CC: David Gibson <david@gibson.dropbear.id.au> > > CC: thuth@redhat.com > > CC: mdroth@linux.vnet.ibm.com > > CC: agraf@suse.de > > --- > > hw/ppc/spapr_cpu_core.c | 23 +++++++++++++++-------- > > 1 file changed, 15 insertions(+), 8 deletions(-) > > > > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c > > index 55cd045..1499a8b 100644 > > --- a/hw/ppc/spapr_cpu_core.c > > +++ b/hw/ppc/spapr_cpu_core.c > > @@ -50,8 +50,6 @@ static void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu, > > Error **errp) > > { > > CPUPPCState *env = &cpu->env; > > - CPUState *cs = CPU(cpu); > > - int i; > > > > /* Set time-base frequency to 512 MHz */ > > cpu_ppc_tb_init(env, SPAPR_TIMEBASE_FREQ); > > @@ -70,12 +68,6 @@ static void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu, > > } > > } > > > > - /* Set NUMA node for the added CPUs */ > > - i = numa_get_node_for_cpu(cs->cpu_index); > > - if (i < nb_numa_nodes) { > > - cs->numa_node = i; > > - } > > - > > xics_cpu_setup(spapr->xics, cpu); > > > > qemu_register_reset(spapr_cpu_reset, cpu); > > @@ -159,11 +151,13 @@ static void spapr_cpu_core_realize(DeviceState *dev, Error **errp) > > const char *typename = object_class_get_name(scc->cpu_class); > > size_t size = object_type_get_instance_size(typename); > > Error *local_err = NULL; > > + int core_node_id = numa_get_node_for_cpu(cc->core_id);; > > void *obj; > > int i, j; > > > > sc->threads = g_malloc0(size * cc->nr_threads); > > for (i = 0; i < cc->nr_threads; i++) { > > + int node_id; > > char id[32]; > > CPUState *cs; > > > > @@ -172,6 +166,19 @@ static void spapr_cpu_core_realize(DeviceState *dev, Error **errp) > > object_initialize(obj, size, typename); > > cs = CPU(obj); > > cs->cpu_index = cc->core_id + i; > > + > > + /* Set NUMA node for the added CPUs */ > > + node_id = numa_get_node_for_cpu(cs->cpu_index); > > + if (node_id != core_node_id) { > > + error_setg(&local_err, "Invalid node-id=%d of thread[cpu-index: %d]" > > + " on CPU[core-id: %d, node-id: %d], node-id must be the same", > > + node_id, cs->cpu_index, cc->core_id, core_node_id); > > + goto err; > > + } > > + if (node_id < nb_numa_nodes) { > > + cs->numa_node = node_id; > > + } > > + > > snprintf(id, sizeof(id), "thread[%d]", i); > > object_property_add_child(OBJECT(sc), id, obj, &local_err); > > if (local_err) { >
diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c index 55cd045..1499a8b 100644 --- a/hw/ppc/spapr_cpu_core.c +++ b/hw/ppc/spapr_cpu_core.c @@ -50,8 +50,6 @@ static void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu, Error **errp) { CPUPPCState *env = &cpu->env; - CPUState *cs = CPU(cpu); - int i; /* Set time-base frequency to 512 MHz */ cpu_ppc_tb_init(env, SPAPR_TIMEBASE_FREQ); @@ -70,12 +68,6 @@ static void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu, } } - /* Set NUMA node for the added CPUs */ - i = numa_get_node_for_cpu(cs->cpu_index); - if (i < nb_numa_nodes) { - cs->numa_node = i; - } - xics_cpu_setup(spapr->xics, cpu); qemu_register_reset(spapr_cpu_reset, cpu); @@ -159,11 +151,13 @@ static void spapr_cpu_core_realize(DeviceState *dev, Error **errp) const char *typename = object_class_get_name(scc->cpu_class); size_t size = object_type_get_instance_size(typename); Error *local_err = NULL; + int core_node_id = numa_get_node_for_cpu(cc->core_id);; void *obj; int i, j; sc->threads = g_malloc0(size * cc->nr_threads); for (i = 0; i < cc->nr_threads; i++) { + int node_id; char id[32]; CPUState *cs; @@ -172,6 +166,19 @@ static void spapr_cpu_core_realize(DeviceState *dev, Error **errp) object_initialize(obj, size, typename); cs = CPU(obj); cs->cpu_index = cc->core_id + i; + + /* Set NUMA node for the added CPUs */ + node_id = numa_get_node_for_cpu(cs->cpu_index); + if (node_id != core_node_id) { + error_setg(&local_err, "Invalid node-id=%d of thread[cpu-index: %d]" + " on CPU[core-id: %d, node-id: %d], node-id must be the same", + node_id, cs->cpu_index, cc->core_id, core_node_id); + goto err; + } + if (node_id < nb_numa_nodes) { + cs->numa_node = node_id; + } + snprintf(id, sizeof(id), "thread[%d]", i); object_property_add_child(OBJECT(sc), id, obj, &local_err); if (local_err) {
Threads within a core shouldn't be on different NUMA nodes, so if user has misconfgured command line, fail QEMU at start up to force user fix it. For now use the first thread on the core as source of core's node-id. Later when cpu-numa refactoring lands it will be switched to core's node-id from possible_cpus[]. Signed-off-by: Igor Mammedov <imammedo@redhat.com> --- Enforcement is necessary to ensure that it would be possible to map legacy CPU index based CLI mapping to core based one and replace numa_info[XXX].node_cpu with possible_cpus[] as storage for mapping info. CC: qemu-ppc@nongnu.org CC: lvivier@redhat.com CC: David Gibson <david@gibson.dropbear.id.au> CC: thuth@redhat.com CC: mdroth@linux.vnet.ibm.com CC: agraf@suse.de --- hw/ppc/spapr_cpu_core.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-)