[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] Get thread and socket information in virsh nodeinfo.



On 03/05/2010 12:06 PM, Chris Lalancette wrote:
> The current code for "nodeinfo" is pretty naive
> about socket and thread information.  To determine the
> sockets, it just takes the number of cpus and divides
> by the number of cores.  For the thread count, it always
> sets it to 1.  With more recent Intel machines, however,
> hyperthreading is again an option, meaning that these
> heuristics no longer work and give bogus numbers.

I noticed this has already been committed, but here are some further
ideas for improvement:

> +#define CPU_SYS_PATH "/sys/devices/system/cpu"
...
> +    if (virAsprintf(&path, "%s/cpu%d/topology/thread_siblings", CPU_SYS_PATH,
> +                    cpu) < 0) {

Do more work at compile-time and less at runtime, by using string
concatenation, as in:

virAsprintf(&path, CPU_SYS_PATH "/cpu%d/topology/thread_siblings", cpu)

> +
> +static int parse_socket(int cpu)
> +{

Several tools (such as ctag, or even more simply, 'git grep "^func"')
work better if all function implementations are listed with split lines,
such that the function name starts at the first column:

static int
parse_socket(int cpu)
{

Would it be worth a global cleanup patch that does this throughout
libvirt, rather than the current ad hoc mix in declaration styles?

> +    nodeinfo->nodes = nodeinfo->cores = 1;
>  
>      /* NB: It is impossible to fill our nodes, since cpuinfo
>       * has not knowledge of NUMA nodes */

s/not/no/

-- 
Eric Blake   eblake redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]