public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: Adhemerval Zanella <adhemerval.zanella@linaro.org>
To: libc-alpha@sourceware.org
Cc: Nicolas Saenz Julienne <nsaenzju@redhat.com>,
	Florian Weimer <fweimer@redhat.com>
Subject: [PATCH 3/3] linux: Revert the use of sched_getaffinity on get_nproc (BZ #28310)
Date: Mon,  6 Sep 2021 15:11:33 -0300	[thread overview]
Message-ID: <20210906181133.1401140-4-adhemerval.zanella@linaro.org> (raw)
In-Reply-To: <20210906181133.1401140-1-adhemerval.zanella@linaro.org>

The use of sched_getaffinity on get_nproc and
sysconf (_SC_NPROCESSORS_ONLN) done in 903bc7dcc2acafc40 (BZ #27645)
breaks the top command in common hypervisor configurations and also
other monitoring tools.

The main issue using sched_getaffinity changed the symbols semantic
from system-wide scope of online CPUs to per-process one (which can
be changed with kernel cpusets or book parameters in VM).

This patch reverts mostly of the 903bc7dcc2acafc40, with the
exceptions:

  * No more cached values and atomic updates, since they are inherent
    racy.

  * No /proc/cpuinfo fallback, since /proc/stat is already used and
    it would require to revert more arch-specific code.

  * The alloca is replace with a static buffer of 1024 bytes.

So the implementation first consult the sysfs, and fallbacks to procfs.

Checked on x86_64-linux-gnu.
---
 sysdeps/unix/sysv/linux/getsysstats.c | 140 +++++++++++++++++++++++++-
 1 file changed, 135 insertions(+), 5 deletions(-)

diff --git a/sysdeps/unix/sysv/linux/getsysstats.c b/sysdeps/unix/sysv/linux/getsysstats.c
index b9710f8319..a1f84c01e2 100644
--- a/sysdeps/unix/sysv/linux/getsysstats.c
+++ b/sysdeps/unix/sysv/linux/getsysstats.c
@@ -17,6 +17,8 @@
    <https://www.gnu.org/licenses/>.  */
 
 #include <array_length.h>
+#include <assert.h>
+#include <ctype.h>
 #include <dirent.h>
 #include <errno.h>
 #include <ldsodefs.h>
@@ -29,7 +31,7 @@
 #include <sysdep.h>
 
 int
-__get_nprocs (void)
+__get_nprocs_sched (void)
 {
   /* This cannot use malloc because it is used on malloc initialization.  */
   enum { max_num_cpus = 32768 };
@@ -48,14 +50,142 @@ __get_nprocs (void)
      atomics are needed). */
   return 2;
 }
-libc_hidden_def (__get_nprocs)
-weak_alias (__get_nprocs, get_nprocs)
+
+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;
+}
+
 
 int
-__get_nprocs_sched (void)
+__get_nprocs (void)
 {
-  return __get_nprocs ();
+  enum { buffer_size = 1024 };
+  char buffer[buffer_size];
+  char *buffer_end = buffer + buffer_size;
+  char *cp = buffer_end;
+  char *re = buffer_end;
+
+  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)
+    {
+      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)
+	return result;
+    }
+
+  cp = buffer_end;
+  re = buffer_end;
+
+  /* Default to an SMP system in case we cannot obtain an accurate
+     number.  */
+  result = 2;
+
+  /* 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;
+
+      __close_nocancel_nostatus (fd);
+    }
+
+  return result;
 }
+libc_hidden_def (__get_nprocs)
+weak_alias (__get_nprocs, get_nprocs)
 
 
 /* On some architectures it is possible to distinguish between configured
-- 
2.30.2


  parent reply	other threads:[~2021-09-06 18:11 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-06 18:11 [PATCH 0/3] Revert the use of sched_getaffinity on get_nproc Adhemerval Zanella
2021-09-06 18:11 ` [PATCH 1/3] misc: Add __get_nprocs_sched Adhemerval Zanella
2021-09-06 18:11 ` [PATCH 2/3] linux: Simplify get_nprocs Adhemerval Zanella
2021-09-07  9:30   ` Florian Weimer
2021-09-07 12:05     ` Adhemerval Zanella
2021-09-07  9:44   ` Florian Weimer
2021-09-06 18:11 ` Adhemerval Zanella [this message]
2021-09-07  9:45   ` [PATCH 3/3] linux: Revert the use of sched_getaffinity on get_nproc (BZ #28310) Florian Weimer
2021-09-07 12:12     ` Adhemerval Zanella

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210906181133.1401140-4-adhemerval.zanella@linaro.org \
    --to=adhemerval.zanella@linaro.org \
    --cc=fweimer@redhat.com \
    --cc=libc-alpha@sourceware.org \
    --cc=nsaenzju@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).