Re: [Qemu-devel] [PATCH 1/2][RFC] qom: introduce cpu QOM hierarchy tree /machine/node/socket/core/thread/cpu.

[Chen Fan <chen.fan.fnst@cn.fujitsu.com>]

On Wed, 2014-02-26 at 15:52 -0300, Eduardo Habkost wrote:
> On Tue, Feb 25, 2014 at 05:07:31PM +0800, Chen Fan wrote:
> [...]
> > +Object *object_get_thread_from_index(int64_t cpu_index)
> 
> Probably the code that is going to call this function already knows what
> will be the node/socket/core/thread IDs for the CPU. And if it doesn't,
> we need to make the code node-/socket-/core-/thread-aware, instead of
> making the code dependent on CPU-index-based logic.
> 
> I believe we should try to make CPU indexes unnecessary, except on
> places where we are already forced to deal with them for compatibility
> reasons.
> 
firstly, thanks for you important feedback.

I suppose that if we try to make CPU indexes unnecessary we should add
"-device cpufoo" support, and we should also need to add a "path" in
command line to let function pc_new_cpu() know the IDs which can be
calculated from the "path". then which one should the commend line be 
"-device cpufoo,sockets=X,cores=Y,threads=Z" or "-device cpufoo,paths=/
node-/socket-/core-/thread-/", or any other better?


> > +{
> > +    gchar *path;
> > +    Object *thread;
> > +    int nodes, cpus_pre_nodes, smp_sockets;
> > +    unsigned node_id, socket_id, core_id, thread_id;
> > +    unsigned core_index, socket_index;
> > +
> > +    nodes = nb_numa_nodes ? nb_numa_nodes : 1;
> > +    cpus_pre_nodes = max_cpus / nodes;
> > +    smp_sockets = cpus_pre_nodes / (smp_cores * smp_threads);
> > +
> > +    core_index = cpu_index / smp_threads;
> > +    thread_id = cpu_index % smp_threads;
> > +    socket_index = core_index / smp_cores;
> > +    core_id = core_index % smp_cores;
> 
> This is duplicating logic that already exists at topo_ids_from_idx().
> 
> > +    node_id = socket_index / smp_sockets;
> 
> This is not how CPUs are distributed along NUMA nodes. They are
> distributed depending on the -numa options. Grep for node_cpumask in the
> code.
> 
> 
> > +    socket_id = socket_index % smp_sockets;
> > +
> > +    path = g_strdup_printf("/machine/node[%d]/socket[%d]/core[%d]/thread[%d]",
> > +                           node_id, socket_id, core_id, thread_id);
> > +    thread = object_resolve_path(path, NULL);
> > +    g_free(path);
> > +
> > +    return thread;
> > +}
> > +
>