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