[PATCH 0/2] Use procutils on getsysstats and malloc-hugepages

[PATCH 0/2] Use procutils on getsysstats and malloc-hugepages

[Adhemerval Zanella]

It allows consolidate the Linux code that parses procfs/sysfs.

Adhemerval Zanella (2):
  linux: Use procutils_read_file on getsysstats
  linux: Use procutils_read_file on malloc-hugepages

 sysdeps/aarch64/Makefile                   |   5 +-
 sysdeps/unix/sysv/linux/Makefile           |   6 +
 sysdeps/unix/sysv/linux/getsysstats.c      | 177 ++++++---------------
 sysdeps/unix/sysv/linux/malloc-hugepages.c |  68 ++++----
 sysdeps/unix/sysv/linux/pidfd_getpid.c     |   2 +-
 sysdeps/unix/sysv/linux/procutils.c        |   4 +-
 sysdeps/unix/sysv/linux/procutils.h        |   4 +-
 7 files changed, 95 insertions(+), 171 deletions(-)

-- 
2.34.1

[PATCH 1/2] linux: Use procutils_read_file on getsysstats

[Adhemerval Zanella]

Checked on x86_64-linux-gnu.
---
 sysdeps/unix/sysv/linux/getsysstats.c  | 177 +++++++------------------
 sysdeps/unix/sysv/linux/pidfd_getpid.c |   2 +-
 sysdeps/unix/sysv/linux/procutils.c    |   4 +-
 sysdeps/unix/sysv/linux/procutils.h    |   4 +-
 4 files changed, 55 insertions(+), 132 deletions(-)

diff --git a/sysdeps/unix/sysv/linux/getsysstats.c b/sysdeps/unix/sysv/linux/getsysstats.c
index b0b6c154ac..1adf74cfc6 100644
--- a/sysdeps/unix/sysv/linux/getsysstats.c
+++ b/sysdeps/unix/sysv/linux/getsysstats.c
@@ -16,18 +16,11 @@
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
-#include <array_length.h>
-#include <assert.h>
 #include <ctype.h>
-#include <errno.h>
-#include <ldsodefs.h>
-#include <limits.h>
-#include <not-cancel.h>
-#include <stdio.h>
-#include <stdio_ext.h>
-#include <sys/mman.h>
+#include <procutils.h>
 #include <sys/sysinfo.h>
 #include <sysdep.h>
+#include <unistd.h>
 
 int
 __get_nprocs_sched (void)
@@ -53,140 +46,66 @@ __get_nprocs_sched (void)
   return 0;
 }
 
-static char *
-next_line (int fd, char *const buffer, char **cp, char **re,
-           char *const buffer_end)
+static int
+parse_proc_stat (const char *l, void *arg)
 {
-  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);
+  /* 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)
+    return -1;
 
-          res = *cp;
-        }
+  if (isdigit (l[3]))
+    *(int *) arg += 1;
 
-      if (nl == NULL)
-        nl = *re - 1;
-    }
-
-  *cp = nl + 1;
-  assert (*cp <= *re);
-
-  return res == *re ? NULL : res;
+  return 1;
 }
 
 static int
 get_nproc_stat (void)
 {
-  enum { buffer_size = 1024 };
-  char buffer[buffer_size];
-  char *buffer_end = buffer + buffer_size;
-  char *cp = buffer_end;
-  char *re = buffer_end;
   int result = 0;
-
-  const int flags = O_RDONLY | O_CLOEXEC;
-  int fd = __open_nocancel ("/proc/stat", flags);
-  if (fd != -1)
-    {
-      char *l;
-      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);
-    }
-
+  /* If procfs can not be accessed, 'result' won't be updated.  */
+  __procutils_read_file ("/proc/stat", parse_proc_stat, &result);
   return result;
 }
 
 static int
-read_sysfs_file (const char *fname)
+parse_sysfs_file (const char *l, void *arg)
 {
-  enum { buffer_size = 1024 };
-  char buffer[buffer_size];
-  char *buffer_end = buffer + buffer_size;
-  char *cp = buffer_end;
-  char *re = buffer_end;
+  int *result = arg;
 
-  const int flags = O_RDONLY | O_CLOEXEC;
-  /* This file contains comma-separated ranges.  */
-  int fd = __open_nocancel (fname, flags);
-  char *l;
-  int result = 0;
-  if (fd != -1)
+  do
     {
-      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;
-		  }
-	      }
-
-	    if (m >= n)
-	      result += m - n + 1;
-
-	    l = endp;
-	    if (l < re && *l == ',')
-	      ++l;
-	  }
-	while (l < re && *l != '\n');
-
-      __close_nocancel_nostatus (fd);
+      char *endp;
+      unsigned long int n = strtoul (l, &endp, 10);
+      if (l == endp)
+	{
+	  *result = 0;
+	  return -1;
+	}
+
+      unsigned long int m = n;
+      if (*endp == '-')
+	{
+	  l = endp + 1;
+	  m = strtoul (l, &endp, 10);
+	  if (l == endp)
+	    {
+	      *result = 0;
+	      return -1;
+	    }
+	}
+
+      if (m >= n)
+	*result += m - n + 1;
+
+      l = endp;
+      if (*l != '\0' && *l == ',')
+	++l;
     }
+  while (*l != '\0' && *l != '\n');
 
-  return result;
+  return 1;
 }
 
 static int
@@ -213,7 +132,9 @@ get_nprocs_fallback (void)
 int
 __get_nprocs (void)
 {
-  int result = read_sysfs_file ("/sys/devices/system/cpu/online");
+  int result = 0;
+  __procutils_read_file ("/sys/devices/system/cpu/online", parse_sysfs_file,
+			 &result);
   if (result != 0)
     return result;
 
@@ -228,7 +149,9 @@ weak_alias (__get_nprocs, get_nprocs)
 int
 __get_nprocs_conf (void)
 {
-  int result = read_sysfs_file ("/sys/devices/system/cpu/possible");
+  int result = 0;
+  __procutils_read_file ("/sys/devices/system/cpu/possible", parse_sysfs_file,
+			 &result);
   if (result != 0)
     return result;
 
diff --git a/sysdeps/unix/sysv/linux/pidfd_getpid.c b/sysdeps/unix/sysv/linux/pidfd_getpid.c
index d03936e97f..f857b1236b 100644
--- a/sysdeps/unix/sysv/linux/pidfd_getpid.c
+++ b/sysdeps/unix/sysv/linux/pidfd_getpid.c
@@ -105,7 +105,7 @@ pidfd_getpid (int fd)
   *_fitoa_word (fd, p, 10, 0) = '\0';
 
   struct parse_fdinfo_t fdinfo = { .found = false, .pid = -1 };
-  if (!procutils_read_file (fdinfoname, parse_fdinfo, &fdinfo))
+  if (!__procutils_read_file (fdinfoname, parse_fdinfo, &fdinfo))
     /* The fdinfo contains an invalid 'Pid:' value.  */
     return INLINE_SYSCALL_ERROR_RETURN_VALUE (EBADF);
 
diff --git a/sysdeps/unix/sysv/linux/procutils.c b/sysdeps/unix/sysv/linux/procutils.c
index 30fc3a063a..1a2dbe941c 100644
--- a/sysdeps/unix/sysv/linux/procutils.c
+++ b/sysdeps/unix/sysv/linux/procutils.c
@@ -71,8 +71,8 @@ next_line (char **r, int fd, char *const buffer, char **cp, char **re,
 }
 
 bool
-procutils_read_file (const char *filename, procutils_closure_t closure,
-		     void *arg)
+__procutils_read_file (const char *filename, procutils_closure_t closure,
+		       void *arg)
 {
   enum { buffer_size = PROCUTILS_MAX_LINE_LEN };
   char buffer[buffer_size];
diff --git a/sysdeps/unix/sysv/linux/procutils.h b/sysdeps/unix/sysv/linux/procutils.h
index b3c2f6fb55..89ca4fcec4 100644
--- a/sysdeps/unix/sysv/linux/procutils.h
+++ b/sysdeps/unix/sysv/linux/procutils.h
@@ -37,7 +37,7 @@ typedef int (*procutils_closure_t) (const char *line, void *arg);
 
    It returns true in case the file is fully read or false if CLOSURE
    returns a value diferent than 0.  */
-bool procutils_read_file (const char *filename, procutils_closure_t closure,
-			  void *arg) attribute_hidden;
+bool __procutils_read_file (const char *filename, procutils_closure_t closure,
+			    void *arg) attribute_hidden;
 
 #endif
-- 
2.34.1

[PATCH 2/2] linux: Use procutils_read_file on malloc-hugepages

[Adhemerval Zanella]

Checked on x86_64-linux-gnu.
---
 sysdeps/aarch64/Makefile                   |  5 +-
 sysdeps/unix/sysv/linux/Makefile           |  6 ++
 sysdeps/unix/sysv/linux/malloc-hugepages.c | 68 ++++++++++------------
 3 files changed, 40 insertions(+), 39 deletions(-)

diff --git a/sysdeps/aarch64/Makefile b/sysdeps/aarch64/Makefile
index 6a9559e5f5..6c80ee8479 100644
--- a/sysdeps/aarch64/Makefile
+++ b/sysdeps/aarch64/Makefile
@@ -67,5 +67,8 @@ sysdep_routines += __mtag_tag_zero_region \
 endif
 
 ifeq ($(subdir),malloc)
-sysdep_malloc_debug_routines = __mtag_tag_zero_region __mtag_tag_region
+sysdep_malloc_debug_routines += \
+  __mtag_tag_zero_region \
+  __mtag_tag_region \
+  # sysdep_malloc_debug_routines
 endif
diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
index 063719bae6..d4ab43c8f2 100644
--- a/sysdeps/unix/sysv/linux/Makefile
+++ b/sysdeps/unix/sysv/linux/Makefile
@@ -637,6 +637,12 @@ CFLAGS-mq_send.c += -fexceptions
 CFLAGS-mq_receive.c += -fexceptions
 endif
 
+ifeq ($(subdir),malloc)
+sysdep_malloc_debug_routines += \
+  procutils \
+  # sysdep_malloc_debug_routines
+endif
+
 ifeq ($(subdir),nscd)
 sysdep-CFLAGS += -DHAVE_EPOLL -DHAVE_INOTIFY -DHAVE_NETLINK
 CFLAGS-gai.c += -DNEED_NETLINK
diff --git a/sysdeps/unix/sysv/linux/malloc-hugepages.c b/sysdeps/unix/sysv/linux/malloc-hugepages.c
index 2f316474c1..cec034e07f 100644
--- a/sysdeps/unix/sysv/linux/malloc-hugepages.c
+++ b/sysdeps/unix/sysv/linux/malloc-hugepages.c
@@ -16,10 +16,12 @@
    License along with the GNU C Library; see the file COPYING.LIB.  If
    not, see <https://www.gnu.org/licenses/>.  */
 
-#include <intprops.h>
 #include <dirent.h>
+#include <intprops.h>
 #include <malloc-hugepages.h>
+#include <string.h>
 #include <not-cancel.h>
+#include <procutils.h>
 #include <sys/mman.h>
 
 unsigned long int
@@ -78,51 +80,41 @@ __malloc_thp_mode (void)
   return malloc_thp_mode_not_supported;
 }
 
-static size_t
-malloc_default_hugepage_size (void)
+static int
+proc_meminfo_parse (const char *s, void *arg)
 {
-  int fd = __open64_nocancel ("/proc/meminfo", O_RDONLY);
-  if (fd == -1)
+  enum { HUGEPAGESIZELEN = sizeof ("Hugepagesize:") - 1 };
+  if (strncmp (s, "Hugepagesize:", HUGEPAGESIZELEN) != 0)
     return 0;
 
-  size_t hpsize = 0;
+  /* The default huge page size is in the form:
+     Hugepagesize:       NUMBER kB  */
+  s += HUGEPAGESIZELEN;
 
-  char buf[512];
-  off64_t off = 0;
-  while (1)
+  size_t hpsize = 0;
+  for (int i = 0; (s[i] >= '0' && s[i] <= '9') || s[i] == ' '; i++)
     {
-      ssize_t r = __pread64_nocancel (fd, buf, sizeof (buf) - 1, off);
-      if (r < 0)
-	break;
-      buf[r] = '\0';
-
-      /* If the tag is not found, read the last line again.  */
-      const char *s = strstr (buf, "Hugepagesize:");
-      if (s == NULL)
-	{
-	  char *nl = strrchr (buf, '\n');
-	  if (nl == NULL)
-	    break;
-	  off += (nl + 1) - buf;
-	  continue;
-	}
-
-      /* The default huge page size is in the form:
-	 Hugepagesize:       NUMBER kB  */
-      s += sizeof ("Hugepagesize: ") - 1;
-      for (int i = 0; (s[i] >= '0' && s[i] <= '9') || s[i] == ' '; i++)
-	{
-	  if (s[i] == ' ')
-	    continue;
-	  hpsize *= 10;
-	  hpsize += s[i] - '0';
-	}
-      hpsize *= 1024;
-      break;
+      if (s[i] == ' ')
+	continue;
+
+      if (INT_MULTIPLY_WRAPV (hpsize, 10, &hpsize)
+	  || INT_ADD_WRAPV (hpsize, s[i] - '0', &hpsize))
+	return -1;
     }
+  if (INT_MULTIPLY_WRAPV (hpsize, 1024, &hpsize))
+    return -1;
 
-  __close_nocancel (fd);
+  *(size_t*) arg = hpsize;
+  return 1;
+}
 
+static size_t
+malloc_default_hugepage_size (void)
+{
+  size_t hpsize = 0;
+  /* If procfs is not accessible of if huge page field is not present,
+     the 'hpsize' argument is not changed.  */
+  __procutils_read_file ("/proc/meminfo", proc_meminfo_parse, &hpsize);
   return hpsize;
 }
 
-- 
2.34.1

Re: [PATCH 2/2] linux: Use procutils_read_file on malloc-hugepages

[Andreas Schwab]

On Sep 12 2023, Adhemerval Zanella wrote:

> +  *(size_t*) arg = hpsize;

Spacing.

> +  /* If procfs is not accessible of if huge page field is not present,

s/of/or/

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."