[2/4] linux: Use sched_getaffinity for __get_nprocs (BZ #27645)

Message ID 20210329182520.323665-2-adhemerval.zanella@linaro.org
State Committed
Headers
Series [1/4] Remove architecture specific sched_cpucount optimizations |

Commit Message

Adhemerval Zanella March 29, 2021, 6:25 p.m. UTC
  Both the sysfs and procfs parsing (through GET_NPROCS_PARSER) are
removed in favor the syscall.  The initial scratch buffer should
fit to most of the common usage (1024 bytes with maps to 8192 CPUs).

Checked on x86_64-linux-gnu and aarch64-linux-gnu.
---
 include/bits/cpu-set.h                        |   8 +
 posix/sched_cpucount.c                        |   1 +
 sysdeps/unix/sysv/linux/alpha/getsysstats.c   |  19 --
 sysdeps/unix/sysv/linux/getsysstats.c         | 217 ++----------------
 sysdeps/unix/sysv/linux/m68k/getsysstats.c    |  37 ---
 .../unix/sysv/linux/microblaze/getsysstats.c  |  34 ---
 sysdeps/unix/sysv/linux/mips/getsysstats.c    |  36 ---
 sysdeps/unix/sysv/linux/sparc/getsysstats.c   |  17 --
 8 files changed, 31 insertions(+), 338 deletions(-)
 delete mode 100644 sysdeps/unix/sysv/linux/m68k/getsysstats.c
 delete mode 100644 sysdeps/unix/sysv/linux/microblaze/getsysstats.c
 delete mode 100644 sysdeps/unix/sysv/linux/mips/getsysstats.c
  

Comments

Florian Weimer April 27, 2021, 3:38 p.m. UTC | #1
* Adhemerval Zanella via Libc-alpha:

> Both the sysfs and procfs parsing (through GET_NPROCS_PARSER) are
> removed in favor the syscall.  The initial scratch buffer should
> fit to most of the common usage (1024 bytes with maps to 8192 CPUs).

The code changes look okay to me.

I don't know if it is safe to ever return 1, though, because some
container hosts return just one bit in the affinity mask, but will still
schedule threads on different CPUs, so the single-processor
synchronization that is normally implied by a CPU count of 1 isn't
guaranteed.

This isn't an issue with the old approach because containers hosts
generally do not virtualize those /sys and /proc files.

I suppose it's okay to make this change and maybe add a NEWS entry that
recommends using at least 2 CPUs for containers due to this change.

Thanks,
Florian
  

Patch

diff --git a/include/bits/cpu-set.h b/include/bits/cpu-set.h
index 388f03cfbd..05aa0a8cf9 100644
--- a/include/bits/cpu-set.h
+++ b/include/bits/cpu-set.h
@@ -1 +1,9 @@ 
+#ifndef _BITS_CPU_SET_H
 #include <posix/bits/cpu-set.h>
+
+#ifndef _ISOMAC
+int __sched_cpucount (size_t __setsize, const cpu_set_t *__setp);
+libc_hidden_proto (__sched_cpucount)
+#endif
+
+#endif /* _BITS_CPU_SET_H  */
diff --git a/posix/sched_cpucount.c b/posix/sched_cpucount.c
index 529286e777..8d853dcc2f 100644
--- a/posix/sched_cpucount.c
+++ b/posix/sched_cpucount.c
@@ -30,3 +30,4 @@  __sched_cpucount (size_t setsize, const cpu_set_t *setp)
     }
   return s;
 }
+libc_hidden_def (__sched_cpucount)
diff --git a/sysdeps/unix/sysv/linux/alpha/getsysstats.c b/sysdeps/unix/sysv/linux/alpha/getsysstats.c
index bdf794703a..098575faef 100644
--- a/sysdeps/unix/sysv/linux/alpha/getsysstats.c
+++ b/sysdeps/unix/sysv/linux/alpha/getsysstats.c
@@ -18,25 +18,6 @@ 
    <https://www.gnu.org/licenses/>.  */
 
 
-/* We need to define a special parser for /proc/cpuinfo.  */
-#define GET_NPROCS_PARSER(FD, BUFFER, CP, RE, BUFFER_END, RESULT)	   \
-  do									   \
-    {									   \
-      /* Find the line that contains the information about the number of   \
-	 active cpus.  We don't have to fear extremely long lines since	   \
-	 the kernel will not generate them.  8192 bytes are really enough. \
-	 If there is no "CPUs ..." line then we are on a UP system.  */	   \
-      char *l;								   \
-      (RESULT) = 1;							   \
-      while ((l = next_line (FD, BUFFER, &CP, &RE, BUFFER_END)) != NULL)  \
-	if ((sscanf (BUFFER, "cpus active : %d", &(RESULT)) == 1)	   \
-	    || (sscanf (BUFFER, "CPUs probed %*d active %d",		   \
-			&(RESULT)) == 1))  				   \
-	  break;							   \
-    }									   \
-  while (0)
-
-
 /* On the Alpha we can distinguish between the number of configured and
    active cpus.  */
 #define GET_NPROCS_CONF_PARSER(FP, BUFFER, RESULT)			   \
diff --git a/sysdeps/unix/sysv/linux/getsysstats.c b/sysdeps/unix/sysv/linux/getsysstats.c
index 2c7f40998a..f8a4a31d1b 100644
--- a/sysdeps/unix/sysv/linux/getsysstats.c
+++ b/sysdeps/unix/sysv/linux/getsysstats.c
@@ -17,215 +17,42 @@ 
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
-#include <alloca.h>
-#include <assert.h>
-#include <ctype.h>
 #include <dirent.h>
-#include <errno.h>
-#include <fcntl.h>
-#include <mntent.h>
-#include <paths.h>
+#include <not-cancel.h>
+#include <scratch_buffer.h>
 #include <stdio.h>
 #include <stdio_ext.h>
-#include <stdlib.h>
-#include <string.h>
-#include <unistd.h>
 #include <sys/sysinfo.h>
-
-#include <atomic.h>
-#include <not-cancel.h>
-
-
-/* How we can determine the number of available processors depends on
-   the configuration.  There is currently (as of version 2.0.21) no
-   system call to determine the number.  It is planned for the 2.1.x
-   series to add this, though.
-
-   One possibility to implement it for systems using Linux 2.0 is to
-   examine the pseudo file /proc/cpuinfo.  Here we have one entry for
-   each processor.
-
-   But not all systems have support for the /proc filesystem.  If it
-   is not available we simply return 1 since there is no way.  */
-
-
-/* Other architectures use different formats for /proc/cpuinfo.  This
-   provides a hook for alternative parsers.  */
-#ifndef GET_NPROCS_PARSER
-# define GET_NPROCS_PARSER(FD, BUFFER, CP, RE, BUFFER_END, RESULT) \
-  do									\
-    {									\
-      (RESULT) = 0;							\
-      /* Read all lines and count the lines starting with the string	\
-	 "processor".  We don't have to fear extremely long lines since	\
-	 the kernel will not generate them.  8192 bytes are really	\
-	 enough.  */							\
-      char *l;								\
-      while ((l = next_line (FD, BUFFER, &CP, &RE, BUFFER_END)) != NULL) \
-	if (strncmp (l, "processor", 9) == 0)				\
-	  ++(RESULT);							\
-    }									\
-  while (0)
-#endif
-
-
-static char *
-next_line (int fd, char *const buffer, char **cp, char **re,
-	   char *const buffer_end)
-{
-  char *res = *cp;
-  char *nl = memchr (*cp, '\n', *re - *cp);
-  if (nl == NULL)
-    {
-      if (*cp != buffer)
-	{
-	  if (*re == buffer_end)
-	    {
-	      memmove (buffer, *cp, *re - *cp);
-	      *re = buffer + (*re - *cp);
-	      *cp = buffer;
-
-	      ssize_t n = __read_nocancel (fd, *re, buffer_end - *re);
-	      if (n < 0)
-		return NULL;
-
-	      *re += n;
-
-	      nl = memchr (*cp, '\n', *re - *cp);
-	      while (nl == NULL && *re == buffer_end)
-		{
-		  /* Truncate too long lines.  */
-		  *re = buffer + 3 * (buffer_end - buffer) / 4;
-		  n = __read_nocancel (fd, *re, buffer_end - *re);
-		  if (n < 0)
-		    return NULL;
-
-		  nl = memchr (*re, '\n', n);
-		  **re = '\n';
-		  *re += n;
-		}
-	    }
-	  else
-	    nl = memchr (*cp, '\n', *re - *cp);
-
-	  res = *cp;
-	}
-
-      if (nl == NULL)
-	nl = *re - 1;
-    }
-
-  *cp = nl + 1;
-  assert (*cp <= *re);
-
-  return res == *re ? NULL : res;
-}
-
+#include <sysdep.h>
 
 int
 __get_nprocs (void)
 {
-  static int cached_result = -1;
-  static time_t timestamp;
-
-  time_t now = time_now ();
-  time_t prev = timestamp;
-  atomic_read_barrier ();
-  if (now == prev && cached_result > -1)
-    return cached_result;
-
-  /* XXX Here will come a test for the new system call.  */
-
-  const size_t buffer_size = __libc_use_alloca (8192) ? 8192 : 512;
-  char *buffer = alloca (buffer_size);
-  char *buffer_end = buffer + buffer_size;
-  char *cp = buffer_end;
-  char *re = buffer_end;
+  struct scratch_buffer set;
+  scratch_buffer_init (&set);
 
-  const int flags = O_RDONLY | O_CLOEXEC;
-  /* This file contains comma-separated ranges.  */
-  int fd = __open_nocancel ("/sys/devices/system/cpu/online", flags);
-  char *l;
-  int result = 0;
-  if (fd != -1)
+  int r;
+  while (true)
     {
-      l = next_line (fd, buffer, &cp, &re, buffer_end);
-      if (l != NULL)
-	do
-	  {
-	    char *endp;
-	    unsigned long int n = strtoul (l, &endp, 10);
-	    if (l == endp)
-	      {
-		result = 0;
-		break;
-	      }
-
-	    unsigned long int m = n;
-	    if (*endp == '-')
-	      {
-		l = endp + 1;
-		m = strtoul (l, &endp, 10);
-		if (l == endp)
-		  {
-		    result = 0;
-		    break;
-		  }
-	      }
-
-	    result += m - n + 1;
-
-	    l = endp;
-	    if (l < re && *l == ',')
-	      ++l;
-	  }
-	while (l < re && *l != '\n');
-
-      __close_nocancel_nostatus (fd);
-
-      if (result > 0)
-	goto out;
+      /* The possible error are EFAULT for an invalid buffer or ESRCH for
+	 invalid pid, none could happen.  */
+      r = INTERNAL_SYSCALL_CALL (sched_getaffinity, 0, set.length,
+				 set.data);
+      if (r > 0)
+	break;
+
+      if (!scratch_buffer_grow (&set))
+	/* Default to an SMP system in case we cannot obtain an accurate
+	   number.  */
+	return 2;
     }
 
-  cp = buffer_end;
-  re = buffer_end;
-
-  /* Default to an SMP system in case we cannot obtain an accurate
-     number.  */
-  result = 2;
+  /* The scratch_buffer is aligned to max_align_t.  */
+  r = __sched_cpucount (r, (const cpu_set_t *) set.data);
 
-  /* The /proc/stat format is more uniform, use it by default.  */
-  fd = __open_nocancel ("/proc/stat", flags);
-  if (fd != -1)
-    {
-      result = 0;
-
-      while ((l = next_line (fd, buffer, &cp, &re, buffer_end)) != NULL)
-	/* The current format of /proc/stat has all the cpu* entries
-	   at the front.  We assume here that stays this way.  */
-	if (strncmp (l, "cpu", 3) != 0)
-	  break;
-	else if (isdigit (l[3]))
-	  ++result;
+  scratch_buffer_free (&set);
 
-      __close_nocancel_nostatus (fd);
-    }
-  else
-    {
-      fd = __open_nocancel ("/proc/cpuinfo", flags);
-      if (fd != -1)
-	{
-	  GET_NPROCS_PARSER (fd, buffer, cp, re, buffer_end, result);
-	  __close_nocancel_nostatus (fd);
-	}
-    }
-
- out:
-  cached_result = result;
-  atomic_write_barrier ();
-  timestamp = now;
-
-  return result;
+  return r;
 }
 libc_hidden_def (__get_nprocs)
 weak_alias (__get_nprocs, get_nprocs)
diff --git a/sysdeps/unix/sysv/linux/m68k/getsysstats.c b/sysdeps/unix/sysv/linux/m68k/getsysstats.c
deleted file mode 100644
index 6915f6d721..0000000000
--- a/sysdeps/unix/sysv/linux/m68k/getsysstats.c
+++ /dev/null
@@ -1,37 +0,0 @@ 
-/* Determine various system internal values, Linux/m68k version.
-   Copyright (C) 2003-2021 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-   Contributed by Andreas Schwab <schwab@suse.de>
-
-   The GNU C Library is free software; you can redistribute it and/or
-   modify it under the terms of the GNU Lesser General Public
-   License as published by the Free Software Foundation; either
-   version 2.1 of the License, or (at your option) any later version.
-
-   The GNU C Library is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-   Lesser General Public License for more details.
-
-   You should have received a copy of the GNU Lesser General Public
-   License along with the GNU C Library.  If not, see
-   <https://www.gnu.org/licenses/>.  */
-
-
-/* We need to define a special parser for /proc/cpuinfo.  */
-#define GET_NPROCS_PARSER(FD, BUFFER, CP, RE, BUFFER_END, RESULT)	  \
-  do									  \
-    {									  \
-      (RESULT) = 0;							  \
-      /* Read all lines and count the lines starting with the string	  \
-	 "CPU:".  We don't have to fear extremely long lines since	  \
-	 the kernel will not generate them.  8192 bytes are really	  \
-	 enough.  */							  \
-      char *l;								  \
-      while ((l = next_line (FD, BUFFER, &CP, &RE, BUFFER_END)) != NULL)  \
-	if (strncmp (l, "CPU:", 4) == 0)	      	     		  \
-	  ++(RESULT);							  \
-    }									  \
-  while (0)
-
-#include <sysdeps/unix/sysv/linux/getsysstats.c>
diff --git a/sysdeps/unix/sysv/linux/microblaze/getsysstats.c b/sysdeps/unix/sysv/linux/microblaze/getsysstats.c
deleted file mode 100644
index 46403e74a8..0000000000
--- a/sysdeps/unix/sysv/linux/microblaze/getsysstats.c
+++ /dev/null
@@ -1,34 +0,0 @@ 
-/* Copyright (C) 1997-2021 Free Software Foundation, Inc.
-
-   This file is part of the GNU C Library.
-
-   The GNU C Library is free software; you can redistribute it and/or
-   modify it under the terms of the GNU Lesser General Public License as
-   published by the Free Software Foundation; either version 2.1 of the
-   License, or (at your option) any later version.
-
-   The GNU C Library is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-   Lesser General Public License for more details.
-
-   You should have received a copy of the GNU Lesser General Public
-   License along with the GNU C Library; if not, see
-   <https://www.gnu.org/licenses/>.  */
-
-/* We need to define a special parser for /proc/cpuinfo.  */
-#define GET_NPROCS_PARSER(FD, BUFFER, CP, RE, BUFFER_END, RESULT)              \
-  do                                                                           \
-    {                                                                          \
-      (RESULT) = 0;                                                            \
-      /* Read all lines and count the lines starting with the string           \
-         "CPU-Family:".  We don't have to fear extremely long lines since      \
-         the kernel will not generate them.  8192 bytes are really enough.  */ \
-      char *l;                                                                 \
-      while ((l = next_line (FD, BUFFER, &CP, &RE, BUFFER_END)) != NULL)       \
-      if (strncmp (l, "CPU-Family:", 11) == 0)                                 \
-        ++(RESULT);                                                            \
-    }                                                                          \
-  while (0)
-
-#include <sysdeps/unix/sysv/linux/getsysstats.c>
diff --git a/sysdeps/unix/sysv/linux/mips/getsysstats.c b/sysdeps/unix/sysv/linux/mips/getsysstats.c
deleted file mode 100644
index 34f352e963..0000000000
--- a/sysdeps/unix/sysv/linux/mips/getsysstats.c
+++ /dev/null
@@ -1,36 +0,0 @@ 
-/* Determine various system internal values, Linux/MIPS version.
-   Copyright (C) 2001-2021 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-
-   The GNU C Library is free software; you can redistribute it and/or
-   modify it under the terms of the GNU Lesser General Public
-   License as published by the Free Software Foundation; either
-   version 2.1 of the License, or (at your option) any later version.
-
-   The GNU C Library is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-   Lesser General Public License for more details.
-
-   You should have received a copy of the GNU Lesser General Public
-   License along with the GNU C Library.  If not, see
-   <https://www.gnu.org/licenses/>.  */
-
-
-/* We need to define a special parser for /proc/cpuinfo.  */
-#define GET_NPROCS_PARSER(FD, BUFFER, CP, RE, BUFFER_END, RESULT)	  \
-  do									  \
-    {									  \
-      (RESULT) = 0;							  \
-      /* Read all lines and count the lines starting with the string	  \
-	 "cpu model".  We don't have to fear extremely long lines since	  \
-	 the kernel will not generate them.  8192 bytes are really	  \
-	 enough.  */							  \
-      char *l;								  \
-      while ((l = next_line (FD, BUFFER, &CP, &RE, BUFFER_END)) != NULL)  \
-	if (strncmp (l, "cpu model", 9) == 0)				  \
-	  ++(RESULT);							  \
-    }									  \
-  while (0)
-
-#include <sysdeps/unix/sysv/linux/getsysstats.c>
diff --git a/sysdeps/unix/sysv/linux/sparc/getsysstats.c b/sysdeps/unix/sysv/linux/sparc/getsysstats.c
index 8e1760802f..a8adba8321 100644
--- a/sysdeps/unix/sysv/linux/sparc/getsysstats.c
+++ b/sysdeps/unix/sysv/linux/sparc/getsysstats.c
@@ -19,23 +19,6 @@ 
    <https://www.gnu.org/licenses/>.  */
 
 
-/* We need to define a special parser for /proc/cpuinfo.  */
-#define GET_NPROCS_PARSER(FD, BUFFER, CP, RE, BUFFER_END, RESULT)	  \
-  do									  \
-    {									  \
-      (RESULT) = 0;							  \
-      /* Find the line that contains the information about the number of  \
-	 active cpus.  We don't have to fear extremely long lines since	  \
-	 the kernel will not generate them.  8192 bytes are really	  \
-	 enough.  */							  \
-      char *l;								  \
-      while ((l = next_line (FD, BUFFER, &CP, &RE, BUFFER_END)) != NULL)  \
-	if (sscanf (l, "ncpus active : %d", &(RESULT)) == 1)		  \
-	  break;							  \
-    }									  \
-  while (0)
-
-
 /* On the Sparc we can distinguish between the number of configured and
    active cpus.  */
 #define GET_NPROCS_CONF_PARSER(FP, BUFFER, RESULT)			 \