[glibc/azanella/posix_spawn-optimizations] mips: Do not malloc on getdents64 fallback

[glibc/azanella/posix_spawn-optimizations] mips: Do not malloc on getdents64 fallback

[Adhemerval Zanella]

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=345abd193337aa5139ae50cccd3b3b069a22ee5e

commit 345abd193337aa5139ae50cccd3b3b069a22ee5e
Author: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Date:   Wed Jul 24 13:48:12 2019 -0300

    mips: Do not malloc on getdents64 fallback
    
    This patch changes how the fallback getdents64 implementation calls
    non-LFS getdents by replacing the scratch_buffer with static buffer
    plus a loop on getdents calls.  This avoids the potential malloc
    call on scratch_buffer_set_array_size for large input buffer size
    at the cost of more getdents syscalls.
    
    It also adds a small optimization for older kernels, where the first
    ENOSYS failure for getdents64 disable subsequent calls.
    
    Check the dirent tests on a mips64-linux-gnu with getdents64 code
    disabled.
    
    	* sysdeps/unix/sysv/linux/mips/mips64/getdents64.c (__getdents64):
    	Add small optimization for older kernel to avoid issuing
    	__NR_getdents64 on each call and replace scratch_buffer usage with
    	a static allocated buffer.

Diff:
---
 sysdeps/unix/sysv/linux/mips/mips64/getdents64.c | 122 ++++++++++-------------
 1 file changed, 54 insertions(+), 68 deletions(-)

diff --git a/sysdeps/unix/sysv/linux/mips/mips64/getdents64.c b/sysdeps/unix/sysv/linux/mips/mips64/getdents64.c
index 8bf3abb..3b5afd9 100644
--- a/sysdeps/unix/sysv/linux/mips/mips64/getdents64.c
+++ b/sysdeps/unix/sysv/linux/mips/mips64/getdents64.c
@@ -22,98 +22,84 @@
 #include <assert.h>
 #include <sys/param.h>
 #include <unistd.h>
-#include <scratch_buffer.h>
 #include <limits.h>
 
 ssize_t
-__getdents64 (int fd, void *buf0, size_t nbytes)
+__getdents64 (int fd, void *buf, size_t nbytes)
 {
-  char *buf = buf0;
-
   /* The system call takes an unsigned int argument, and some length
      checks in the kernel use an int type.  */
   if (nbytes > INT_MAX)
     nbytes = INT_MAX;
 
 #ifdef __NR_getdents64
-  ssize_t ret = INLINE_SYSCALL_CALL (getdents64, fd, buf, nbytes);
-  if (ret != -1)
-    return ret;
+  static bool getdents64_supportted = true;
+  if (atomic_load_relaxed (&getdents64_supportted))
+    {
+      ssize_t ret = INLINE_SYSCALL_CALL (getdents64, fd, buf, nbytes);
+      if (ret >= 0 || errno != ENOSYS)
+	return ret;
+
+      atomic_store_relaxed (&getdents64_supportted, false);
+    }
 #endif
 
   /* Unfortunately getdents64 was only wire-up for MIPS n64 on Linux 3.10.
-     If syscall is not available it need to fallback to non-LFS one.  */
+     If the syscall is not available it need to fallback to the non-LFS one.
+     Also to avoid an unbounded allocation through VLA/alloca or malloc (which
+     would make the syscall non async-signal-safe) it uses a limited buffer.
+     This is sub-optimal for large NBYTES, however this is a fallback
+     mechanism to emulate a syscall that kernel should provide.   */
 
+  enum { KBUF_SIZE = 1024 };
   struct kernel_dirent
-    {
-      unsigned long d_ino;
-      unsigned long d_off;
-      unsigned short int d_reclen;
-      char d_name[256];
-    };
-
-  const size_t size_diff = (offsetof (struct dirent64, d_name)
-			   - offsetof (struct kernel_dirent, d_name));
-
-  size_t red_nbytes = MIN (nbytes
-			   - ((nbytes / (offsetof (struct dirent64, d_name)
-					 + 14)) * size_diff),
-			   nbytes - size_diff);
-
-  struct scratch_buffer tmpbuf;
-  scratch_buffer_init (&tmpbuf);
-  if (!scratch_buffer_set_array_size (&tmpbuf, red_nbytes, sizeof (uint8_t)))
-    INLINE_SYSCALL_ERROR_RETURN_VALUE (ENOMEM);
-
-  struct kernel_dirent *skdp, *kdp;
-  skdp = kdp = tmpbuf.data;
-
-  ssize_t retval = INLINE_SYSCALL_CALL (getdents, fd, kdp, red_nbytes);
-  if (retval == -1)
-    {
-      scratch_buffer_free (&tmpbuf);
-      return -1;
-    }
+  {
+    unsigned long d_ino;
+    unsigned long d_off;
+    unsigned short int d_reclen;
+    char d_name[1];
+  } kbuf[KBUF_SIZE / sizeof (struct kernel_dirent)];
+  size_t kbuf_size = nbytes < KBUF_SIZE ? nbytes : KBUF_SIZE;
 
-  off64_t last_offset = -1;
   struct dirent64 *dp = (struct dirent64 *) buf;
-  while ((char *) kdp < (char *) skdp + retval)
+
+  size_t nb = 0;
+  off64_t last_offset = -1;
+
+  ssize_t r;
+  while ((r = INLINE_SYSCALL_CALL (getdents, fd, kbuf, kbuf_size)) > 0)
     {
-      const size_t alignment = _Alignof (struct dirent64);
-      /* Since kdp->d_reclen is already aligned for the kernel structure
-	 this may compute a value that is bigger than necessary.  */
-      size_t new_reclen = ((kdp->d_reclen + size_diff + alignment - 1)
-			   & ~(alignment - 1));
-      if ((char *) dp + new_reclen > buf + nbytes)
-        {
-	  /* Our heuristic failed.  We read too many entries.  Reset
-	     the stream.  */
-	  assert (last_offset != -1);
-	  __lseek64 (fd, last_offset, SEEK_SET);
-
-	  if ((char *) dp == buf)
+      struct kernel_dirent *skdp, *kdp;
+      skdp = kdp = kbuf;
+
+      while ((char *) kdp < (char *) skdp + r)
+	{
+	  const size_t alignment = _Alignof (struct dirent64);
+	  size_t new_reclen = ((kdp->d_reclen + alignment - 1)
+			      & ~(alignment - 1));
+	  if (nb + new_reclen > nbytes)
 	    {
-	      scratch_buffer_free (&tmpbuf);
-	      return INLINE_SYSCALL_ERROR_RETURN_VALUE (EINVAL);
+		/* The new entry will overflow the input buffer, rewind to
+		   last obtained entry and return.  */
+	       __lseek64 (fd, last_offset, SEEK_SET);
+	       goto out;
 	    }
+	  nb += new_reclen;
 
-	  break;
-	}
-
-      last_offset = kdp->d_off;
-      dp->d_ino = kdp->d_ino;
-      dp->d_off = kdp->d_off;
-      dp->d_reclen = new_reclen;
-      dp->d_type = *((char *) kdp + kdp->d_reclen - 1);
-      memcpy (dp->d_name, kdp->d_name,
-	      kdp->d_reclen - offsetof (struct kernel_dirent, d_name));
+	  dp->d_ino = kdp->d_ino;
+	  dp->d_off = last_offset = kdp->d_off;
+	  dp->d_reclen = new_reclen;
+	  dp->d_type = *((char *) kdp + kdp->d_reclen - 1);
+	  memcpy (dp->d_name, kdp->d_name,
+		  kdp->d_reclen - offsetof (struct kernel_dirent, d_name));
 
-      dp = (struct dirent64 *) ((char *) dp + new_reclen);
-      kdp = (struct kernel_dirent *) (((char *) kdp) + kdp->d_reclen);
+	  dp = (struct dirent64 *) ((char *) dp + new_reclen);
+	  kdp = (struct kernel_dirent *) (((char *) kdp) + kdp->d_reclen);
+	}
     }
 
-  scratch_buffer_free (&tmpbuf);
-  return (char *) dp - buf;
+out:
+  return (char *) dp - (char *) buf;
 }
 libc_hidden_def (__getdents64)
 weak_alias (__getdents64, getdents64)

[glibc/azanella/posix_spawn-optimizations] mips: Do not malloc on getdents64 fallback

[Adhemerval Zanella]

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=339200e22e165b4d18bb9ddd42d6020536175542

commit 339200e22e165b4d18bb9ddd42d6020536175542
Author: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Date:   Wed Jul 24 13:48:12 2019 -0300

    mips: Do not malloc on getdents64 fallback
    
    This patch changes how the fallback getdents64 implementation calls
    non-LFS getdents by replacing the scratch_buffer with static buffer
    plus a loop on getdents calls.  This avoids the potential malloc
    call on scratch_buffer_set_array_size for large input buffer size
    at the cost of more getdents syscalls.
    
    It also adds a small optimization for older kernels, where the first
    ENOSYS failure for getdents64 disable subsequent calls.
    
    Check the dirent tests on a mips64-linux-gnu with getdents64 code
    disable.
    
    	* sysdeps/unix/sysv/linux/mips/mips64/getdents64.c (__getdents64):
    	Add small optimization for older kernel to avoid issuing
    	__NR_getdents64 on each call and replace scratch_buffer usage with
    	a static allocated buffer.

Diff:
---
 sysdeps/unix/sysv/linux/mips/mips64/getdents64.c | 125 ++++++++++-------------
 1 file changed, 52 insertions(+), 73 deletions(-)

diff --git a/sysdeps/unix/sysv/linux/mips/mips64/getdents64.c b/sysdeps/unix/sysv/linux/mips/mips64/getdents64.c
index 8bf3abb..82d7e04 100644
--- a/sysdeps/unix/sysv/linux/mips/mips64/getdents64.c
+++ b/sysdeps/unix/sysv/linux/mips/mips64/getdents64.c
@@ -22,98 +22,77 @@
 #include <assert.h>
 #include <sys/param.h>
 #include <unistd.h>
-#include <scratch_buffer.h>
 #include <limits.h>
 
 ssize_t
-__getdents64 (int fd, void *buf0, size_t nbytes)
+__getdents64 (int fd, void *buf, size_t nbytes)
 {
-  char *buf = buf0;
-
   /* The system call takes an unsigned int argument, and some length
      checks in the kernel use an int type.  */
   if (nbytes > INT_MAX)
     nbytes = INT_MAX;
 
 #ifdef __NR_getdents64
-  ssize_t ret = INLINE_SYSCALL_CALL (getdents64, fd, buf, nbytes);
-  if (ret != -1)
-    return ret;
-#endif
-
-  /* Unfortunately getdents64 was only wire-up for MIPS n64 on Linux 3.10.
-     If syscall is not available it need to fallback to non-LFS one.  */
-
-  struct kernel_dirent
+  static bool getdents64_supportted = true;
+  if (atomic_load_relaxed (&getdents64_supportted))
     {
-      unsigned long d_ino;
-      unsigned long d_off;
-      unsigned short int d_reclen;
-      char d_name[256];
-    };
+      ssize_t ret = INLINE_SYSCALL_CALL (getdents64, fd, buf, nbytes);
+      if (ret >= 0 || errno != ENOSYS)
+	return ret;
 
-  const size_t size_diff = (offsetof (struct dirent64, d_name)
-			   - offsetof (struct kernel_dirent, d_name));
-
-  size_t red_nbytes = MIN (nbytes
-			   - ((nbytes / (offsetof (struct dirent64, d_name)
-					 + 14)) * size_diff),
-			   nbytes - size_diff);
-
-  struct scratch_buffer tmpbuf;
-  scratch_buffer_init (&tmpbuf);
-  if (!scratch_buffer_set_array_size (&tmpbuf, red_nbytes, sizeof (uint8_t)))
-    INLINE_SYSCALL_ERROR_RETURN_VALUE (ENOMEM);
-
-  struct kernel_dirent *skdp, *kdp;
-  skdp = kdp = tmpbuf.data;
-
-  ssize_t retval = INLINE_SYSCALL_CALL (getdents, fd, kdp, red_nbytes);
-  if (retval == -1)
-    {
-      scratch_buffer_free (&tmpbuf);
-      return -1;
+      atomic_store_relaxed (&getdents64_supportted, false);
     }
+#endif
+
+  /* Unfortunately getdents64 was only wire-up for MIPS n64 on Linux 3.10.
+     If the syscall is not available it need to fallback to the non-LFS one.
+     Also to avoid an unbounded allocation through VLA/alloca or malloc (which
+     would make the syscall non async-signal-safe) it uses a limited buffer.
+     This is sub-optimal for large NBYTES, however this is a fallback
+     mechanism to emulate a syscall that kernel should provide.   */
+
+  enum { KBUF_SIZE = 1024 };
+  unsigned char kbuf[KBUF_SIZE];
+  /* Set the buffer size to input to avoid reading more entries than asked
+     for.  */
+  size_t kbuf_size = nbytes < KBUF_SIZE ? nbytes : KBUF_SIZE;
 
-  off64_t last_offset = -1;
   struct dirent64 *dp = (struct dirent64 *) buf;
-  while ((char *) kdp < (char *) skdp + retval)
+
+  ssize_t retval;
+  while ((retval = INLINE_SYSCALL_CALL (getdents, fd, kbuf, kbuf_size)) > 0)
     {
-      const size_t alignment = _Alignof (struct dirent64);
-      /* Since kdp->d_reclen is already aligned for the kernel structure
-	 this may compute a value that is bigger than necessary.  */
-      size_t new_reclen = ((kdp->d_reclen + size_diff + alignment - 1)
-			   & ~(alignment - 1));
-      if ((char *) dp + new_reclen > buf + nbytes)
-        {
-	  /* Our heuristic failed.  We read too many entries.  Reset
-	     the stream.  */
-	  assert (last_offset != -1);
-	  __lseek64 (fd, last_offset, SEEK_SET);
-
-	  if ((char *) dp == buf)
-	    {
-	      scratch_buffer_free (&tmpbuf);
-	      return INLINE_SYSCALL_ERROR_RETURN_VALUE (EINVAL);
-	    }
-
-	  break;
+      struct kernel_dirent
+      {
+	unsigned long d_ino;
+	unsigned long d_off;
+	unsigned short int d_reclen;
+	char d_name[1];
+      };
+      struct kernel_dirent *skdp, *kdp;
+      skdp = kdp = (void*) kbuf;
+
+      while ((char *) kdp < (char *) skdp + retval)
+	{
+	  const size_t alignment = _Alignof (struct dirent64);
+	  size_t new_reclen = ((kdp->d_reclen + alignment - 1)
+			      & ~(alignment - 1));
+	  dp->d_ino = kdp->d_ino;
+	  dp->d_off = kdp->d_off;
+	  dp->d_reclen = new_reclen;
+	  dp->d_type = *((char *) kdp + kdp->d_reclen - 1);
+	  memcpy (dp->d_name, kdp->d_name,
+		  kdp->d_reclen - offsetof (struct kernel_dirent, d_name));
+
+	  dp = (struct dirent64 *) ((char *) dp + new_reclen);
+	  kdp = (struct kernel_dirent *) (((char *) kdp) + kdp->d_reclen);
 	}
-
-      last_offset = kdp->d_off;
-      dp->d_ino = kdp->d_ino;
-      dp->d_off = kdp->d_off;
-      dp->d_reclen = new_reclen;
-      dp->d_type = *((char *) kdp + kdp->d_reclen - 1);
-      memcpy (dp->d_name, kdp->d_name,
-	      kdp->d_reclen - offsetof (struct kernel_dirent, d_name));
-
-      dp = (struct dirent64 *) ((char *) dp + new_reclen);
-      kdp = (struct kernel_dirent *) (((char *) kdp) + kdp->d_reclen);
     }
 
-  scratch_buffer_free (&tmpbuf);
-  return (char *) dp - buf;
+  if (retval == -1)
+    return -1;
+
+  return (char *) dp - (char *) buf;
 }
 libc_hidden_def (__getdents64)
 weak_alias (__getdents64, getdents64)