Re: [musl] [PATCH v3] sysconf: add _SC_NPROCESSORS_CONF support

[Rich Felker <dalias@libc.org>]

On Sat, Jul 10, 2021 at 12:29:34PM -0500, Samuel Holland wrote:
> Hi all,
> 
> While this topic is being discussed, I'd like to bring up another possible
> method for consideration. It turns out that some files in procfs have contents
> depending on the number of {possible,online} CPUs; we can parse them to get the
> count we need.
> 
> The benefits are:
>  - procfs is more likely to be mounted than sysfs, and it is already
>    documented as being required by musl.
>  - Some of the procfs interfaces have been around longer than the
>    sysfs interface.
>  - The parsing logic is somewhat simpler than for the files in
>    /sys/devices/system/cpu, because we are just counting the number
>    of occurrences of some string.
> 
> The downsides are:
>  - It depends on the stability of the procfs file formats.
>  - The kernel uses more CPU to generate the contents of these files.

My understanding is that the procfs files have stronger stability
mandate than sysfs if anything (originally, only procfs was stable and
sysfs was not). So I think it's perfectly acceptable to use and prefer
procfs. And indeed it's more likely to be mounted; some
container/sandbox setups I use bind-mount (or mount a new) procfs but
do not expose any sysfs.

> I looked through the procfs code for uses of for_each_possible_cpu and
> for_each_online_cpu, and the best candidates I found are:
>  - /proc/softirqs for _SC_NPROCESSORS_CONF. It was added in 2009
>    (commit d3d64df21d3d, v2.6.31). Its first line contains a column
>    for_each_possible_cpu.

This should be okay; nonstandard functionality like this does not need
to work back to ancient kernels. Maybe there's a way we could use the
kernel cpuset size (is that observable?) as a fallback -- or, if
kernels that old didn't have variable cpuset size, just default to
whatever the fixed size (1024?) was back then.

>  - /proc/stat for _SC_NPROCESSORS_ONLN. It has been around since
>    before git. It contains a line for_each_online_cpu.
> 
> Here's an example implementation:
> 
> diff --git a/src/conf/sysconf.c b/src/conf/sysconf.c
> index 3baaed32..8f787b81 100644
> --- a/src/conf/sysconf.c
> +++ b/src/conf/sysconf.c
> @@ -193,10 +193,27 @@ long sysconf(int name)
>  		return SEM_VALUE_MAX;
>  	case JT_DELAYTIMER_MAX & 255:
>  		return DELAYTIMER_MAX;
> -	case JT_NPROCESSORS_CONF & 255:
> -	case JT_NPROCESSORS_ONLN & 255: ;
> +	case JT_NPROCESSORS_CONF & 255: ;
> +		FILE *f = fopen("/proc/softirqs", "rbe");
> +		int i, cnt = 0;
> +		if (f) {
> +			while (fscanf(f, " CPU%u", &i) > 0) ++cnt;
> +			fclose(f);
> +		}
> +		if (cnt)
> +			return cnt;
> +		/* fallthrough */

This is nice and simple, but the fscanf machinery is rather large and
undesirable to pull in as a linking consequence of sysconf. Likewise,
it should not be pulling in malloc/free. __fopen_rb_ca could be used
if we want to use stdio though, along with a trivial state machine to
count occurrances of "CPU" in first line.

> +	case JT_NPROCESSORS_ONLN & 255:
> +		f = fopen("/proc/stat", "rbe");
> +		cnt = 0;
> +		if (f) {
> +			fscanf(f, "cpu %*[^c]");
> +			while (fscanf(f, "cpu%u %*[^c]", &i) > 0) ++cnt;
> +			fclose(f);
> +		}
> +		if (cnt)
> +			return cnt;
>  		unsigned char set[128] = {1};
> -		int i, cnt;
>  		__syscall(SYS_sched_getaffinity, 0, sizeof set, set);
>  		for (i=cnt=0; i<sizeof set; i++)
>  			for (; set[i]; set[i]&=set[i]-1, cnt++);

Same principles here. fgets is a lot more suitable than fscanf here
anyway.

Rich