[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/08/2010 07:19 PM, Eric Blake wrote:
> On 03/05/2010 12:06 PM, Chris Lalancette wrote:
>> +#define CPU_SYS_PATH "/sys/devices/system/cpu"
>>  
>> +    if (virAsprintf(&path, "%s/cpu%d/topology/thread_siblings", CPU_SYS_PATH,
>> +                    cpu) < 0) {
> 
> Where is the documentation about what this file will contain?  On my
> system, I see:
> 
> $ cat /sys/devices/system/cpu/cpu0/topology/thread_siblings_list
> 0
> $ cat /sys/devices/system/cpu/cpu0/topology/thread_siblings
> 00000001
> $ cat /sys/devices/system/cpu/cpu1/topology/thread_siblings_list
> 1
> $ cat /sys/devices/system/cpu/cpu1/topology/thread_siblings
> 00000002
> 
> That is, I'm guessing that topology/thread_siblings_list is
> human-readable, while topology/thread_siblings is a hex bitmask.  If it
> is indeed a 32-bit mask, then:

Actually, it's a much longer than 32-bit bit mask.  On my RHEL-5 system,
it looks like:

00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000002

(the commas are just to split it up visually; think of that above as
one huge bitmask)

> 
>> +    if (fgets(str, sizeof(str), pathfp) == NULL) {
>> +        virReportSystemError(errno, _("cannot read from %s"), path);
>> +        goto cleanup;
>> +    }
>> +
>> +    i = 0;
>> +    while (str[i] != '\0') {
>> +        if (str[i] != '\n' && str[i] != ',')
>> +            ret += count_one_bits(str[i] - '0');
>> +        i++;
>> +    }
> 
> ...this loop is borked, since it is assuming that str[i] will be a
> digit, and is not looking for a-f.  And why skipping comma?  Shouldn't
> this instead be parsing the entire file contents as a single int, and
> only then calling count_one_bits once on the result?

So skipping the comma is correct here.  You are right, though, that this
is wrong for hex.  For some reason I confused myself and thought the above
was binary, not hex.  I'll send a follow-up patch.

Thanks,
-- 
Chris Lalancette


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