Re: [PATCH] core-count: A new program to count the number of cpu cores

[Pádraig Brady]

Thanks for continuing with this.
I'm not sure we agreed on the name but I like nproc at least :)

Giuseppe Scrivano wrote:

> address@hidden --available
> address@hidden --available
> +Print the number of processors available to the current process.  It
> +may be less than the number of installed processors.
> +If this information is not accessible, then nproc returs the number of

s/returs/returns/

> +installed processors.  By default --available is used.
> +
> address@hidden --installed
> address@hidden --installed
> +Print the number of installed processors.

I went digging for what --available and --installed correspond to..
I thought first that they were respectively:

$ getconf -a | grep NPROC
_NPROCESSORS_CONF                  1
_NPROCESSORS_ONLN                  1

num_processors() already uses _NPROCESSORS_ONLN (online processors)
so I then wondered how this be different to that returned by
pthread_getaffinity_np() ?

A quick google for cpuset shows:
http://www.kernel.org/doc/man-pages/online/pages/man7/cpuset.7.html

Also this is what sysconf seems to query for the variables above:
$ strace -e open getconf _NPROCESSORS_ONLN
open("/proc/stat"
$ strace -e open getconf _NPROCESSORS_CONF
open("/sys/devices/system/cpu"

So looking at the /proc/stat code:
http://lxr.linux.no/#linux+v2.6.31/fs/proc/stat.c
Shows it calls for_each_online_cpu()
Which according to the following is each CPU available to scheduler:
http://lxr.linux.no/#linux+v2.6.31/include/linux/cpumask.h#L451
However that's system wide and a particular process
could be in a smaller cpuset.

pthread_getaffinity_np instead calls sched_getaffinity which
can return a smaller set as seen here:
http://lxr.linux.no/#linux+v2.6.31/kernel/sched.c#L6484

So in summary, this is a cool addition Giuseppe!

I do wonder though whether it would be better
to have num_processors() try to return this by default?
I.E. can you think of a use case where someone would
want to use the --installed option? BTW "installed"
differs from the terms discussed here, and perhaps
"online" would be better (if we did want to expose it at all).

Also I'm wondering why you used the pthread interface to this?
I didn't notice pthread_getaffinity_np() in POSIX for example
(is that what the _np represents?), so why not call sched_getaffinity
directly without needing to link with the pthread library.
>From experience the sched_getaffinity() call has been a moving target:
http://www.pixelbeat.org/programming/gcc/c_c++_notes.html#affinity
but it has been stable for a long time and one could just check
for the current stable interface. Also linked from the above is:
the interesting  http://www.open-mpi.org/projects/hwloc/

> +static u_long
> +nproc_available ()
> +{
> +  u_long nproc = 0;

unsigned long is standard and fine.

> +++ b/tests/nproc/cpuinfo
> @@ -0,0 +1,34 @@
> +#!/bin/sh
> +# Check that the number of cores reported by nproc is equal to the number
> +# of cores listed in /proc/cpuinfo, when it is available

s/cores/processors/

> +cores=$(nproc)

nproc=$(nproc --installed)

Also perhaps we should be comparing to /proc/stat just in case
/proc/cpuinfo was not showing the online processors:
grep '^cpu[0-9]' /proc/stat  | wc -l

cheers,
Pádraig.