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