Re: [PATCH] Fix num_processors_ignoring_omp.

[Bruno Haible]

Hi Nikita,

> From: Nikita Ermakov <address@hidden>
> 
> - Update comments to correspond closed glibc bug #21542.
> 
> - In case of failed _SC_NPROCESSORS_CONF use the maximum between
>   nprocs_current and nprocs to meet the requirements of
>   num_processors (NPROC_ALL) >= num_processors (NPROC_CURRENT).

Thanks for the report. To summarize, sysconf (_SC_NPROCESSORS_CONF)
may now return 2 instead of 1, in situations where the number of
processors cannot be determined because /proc and /sys are not mounted.
The rationale given in
  <https://sourceware.org/bugzilla/show_bug.cgi?id=21542>
is that there are "applications which use the return value to recognize
uniprocessor systems and apply optimizations based on that".

> diff --git a/lib/nproc.c b/lib/nproc.c
> index 77b876027..a550711b4 100644
> --- a/lib/nproc.c
> +++ b/lib/nproc.c
> @@ -217,17 +217,14 @@ num_processors_ignoring_omp (enum nproc_query query)
>       the /sys and /proc file systems (see
>       glibc/sysdeps/unix/sysv/linux/getsysstats.c).
>       In some situations these file systems are not mounted, and the sysconf
> -     call returns 1, which does not reflect the reality.  */
> +     call returns 1 or 2, which does not reflect the reality.  */
> +  unsigned long nprocs_current = num_processors_via_affinity_mask ();
>  
>    if (query == NPROC_CURRENT)
>      {
>        /* Try the modern affinity mask system call.  */
> -      {
> -        unsigned long nprocs = num_processors_via_affinity_mask ();
> -
> -        if (nprocs > 0)
> -          return nprocs;
> -      }
> +      if (nprocs_current > 0)
> +        return nprocs_current;
>  
>  #if defined _SC_NPROCESSORS_ONLN
>        { /* This works on glibc, Mac OS X 10.5, FreeBSD, AIX, OSF/1, Solaris,
> @@ -249,15 +246,10 @@ num_processors_ignoring_omp (enum nproc_query query)
>          /* On Linux systems with glibc, this information comes from the /sys 
> and
>             /proc file systems (see 
> glibc/sysdeps/unix/sysv/linux/getsysstats.c).
>             In some situations these file systems are not mounted, and the
> -           sysconf call returns 1.  But we wish to guarantee that
> +           sysconf call returns 1 or 2.  But we wish to guarantee that
>             num_processors (NPROC_ALL) >= num_processors (NPROC_CURRENT).  */
> -        if (nprocs == 1)
> -          {
> -            unsigned long nprocs_current = num_processors_via_affinity_mask 
> ();
> -
> -            if (nprocs_current > 0)
> -              nprocs = nprocs_current;
> -          }
> +        if (nprocs_current > nprocs)
> +          nprocs = nprocs_current;
>  # endif
>  
>          if (nprocs > 0)

Your patch goes in the right direction. But it causes additional,
unnecessary calls to num_processors_via_affinity_mask () in two situations:
  - on systems other than glibc/Linux systems,
  - on glibc/Linux systems on which sysconf (_SC_NPROCESSORS_CONF)
    returned a value > 2.

I'm therefore applying this slightly different patch:


2019-06-10  Bruno Haible  <address@hidden>

        nproc: Ensure nproc(NPROC_ALL) ≥ nproc(NPROC_CURRENT) with glibc ≥ 2.26.
        Reported by Nikita Ermakov <address@hidden> in
        <https://lists.gnu.org/archive/html/bug-gnulib/2019-06/msg00003.html>.
        * lib/nproc.c (num_processors_ignoring_omp): Treat a return value of
        sysconf (_SC_NPROCESSORS_CONF) == 2 like a return value == 1.

diff --git a/lib/nproc.c b/lib/nproc.c
index 77b8760..b11690b 100644
--- a/lib/nproc.c
+++ b/lib/nproc.c
@@ -216,8 +216,9 @@ num_processors_ignoring_omp (enum nproc_query query)
      Note! On Linux systems with glibc, the first and second number come from
      the /sys and /proc file systems (see
      glibc/sysdeps/unix/sysv/linux/getsysstats.c).
-     In some situations these file systems are not mounted, and the sysconf
-     call returns 1, which does not reflect the reality.  */
+     In some situations these file systems are not mounted, and the sysconf 
call
+     returns 1 or 2 (<https://sourceware.org/bugzilla/show_bug.cgi?id=21542>),
+     which does not reflect the reality.  */
 
   if (query == NPROC_CURRENT)
     {
@@ -249,13 +250,13 @@ num_processors_ignoring_omp (enum nproc_query query)
         /* On Linux systems with glibc, this information comes from the /sys 
and
            /proc file systems (see 
glibc/sysdeps/unix/sysv/linux/getsysstats.c).
            In some situations these file systems are not mounted, and the
-           sysconf call returns 1.  But we wish to guarantee that
+           sysconf call returns 1 or 2.  But we wish to guarantee that
            num_processors (NPROC_ALL) >= num_processors (NPROC_CURRENT).  */
-        if (nprocs == 1)
+        if (nprocs == 1 || nprocs == 2)
           {
             unsigned long nprocs_current = num_processors_via_affinity_mask ();
 
-            if (nprocs_current > 0)
+            if (/* nprocs_current > 0 && */ nprocs_current > nprocs)
               nprocs = nprocs_current;
           }
 # endif