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

[Eduardo Habkost]

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.

> +{
> +    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;
> +}
> +

-- 
Eduardo