[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] spapr: ensure that all threads within core are
From: |
David Gibson |
Subject: |
Re: [Qemu-devel] [PATCH] spapr: ensure that all threads within core are on the same NUMA node |
Date: |
Mon, 6 Mar 2017 10:34:40 +1100 |
User-agent: |
Mutt/1.7.1 (2016-10-04) |
On Fri, Mar 03, 2017 at 06:41:04PM +0100, Igor Mammedov wrote:
> On Fri, 24 Feb 2017 10:26:56 +0100
> Igor Mammedov <address@hidden> 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 <address@hidden>
> 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: address@hidden
> > CC: address@hidden
> > CC: David Gibson <address@hidden>
> > CC: address@hidden
> > CC: address@hidden
> > CC: address@hidden
> > ---
> > 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) {
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature