]> git.ipfire.org Git - thirdparty/glibc.git/commitdiff
posix: Fix and simplify default p{read,write}v implementation
authorAdhemerval Zanella <adhemerval.zanella@linaro.org>
Tue, 2 May 2017 17:39:58 +0000 (14:39 -0300)
committerAdhemerval Zanella <adhemerval.zanella@linaro.org>
Mon, 15 May 2017 19:33:45 +0000 (16:33 -0300)
Currently all architectures but microblaze use wire-up syscall for
p{readv,write}v.  Microblaze still uses the syscall emulation using
sysdep/posix/p{readv,writev}.c and it was reported in some ocasions
[1] [2] that it might have some issues with some linux specific
usage (mainly with O_DIRECT and the alignment requirement).

Although it is not an issue for virtually all the system, this
patch refactors the sysdeps/posix p{read,write}v syscall to avoid
such issue (by using posix_memalign on the buffer used on
p{read,write} call) and by refactoring it common files to avoid
the need check on defines to correct set the alias and internal
symbols.

Checked on microblaze-linux-gnu check with run-built-tests=no and
by using the sysdeps/posix implementation on x86_64-linux-gnu (just
for sanity test where it shown no regression).

* sysdeps/posix/preadv.c: Use sysdeps/posix/preadv_common.c.
* sysdeps/posix/preadv64.c: Likewise.
* sysdeps/unix/sysv/linux/preadv.c: Likewise.
* sysdeps/unix/sysv/linux/preadv64.c: Likewise.
* sysdeps/posix/pwritev.c: Use sysdeps/posix/pwritev_common.c.
* sysdeps/posix/pwritev64.c: Likewise.
* sysdeps/unix/sysv/linux/pwritev.c: Likewise.
* sysdeps/unix/sysv/linux/pwritev64.c: Likewise.
* sysdeps/posix/preadv_common.c: New file.
* sysdeps/posix/pwritev_common.c: Likewise.

[1] http://www.mail-archive.com/qemu-devel@nongnu.org/msg25282.html
[2] https://bugzilla.redhat.com/show_bug.cgi?id=563103#c8

ChangeLog
sysdeps/posix/preadv.c
sysdeps/posix/preadv64.c
sysdeps/posix/preadv_common.c [new file with mode: 0644]
sysdeps/posix/pwritev.c
sysdeps/posix/pwritev64.c
sysdeps/posix/pwritev_common.c [new file with mode: 0644]
sysdeps/unix/sysv/linux/preadv.c
sysdeps/unix/sysv/linux/preadv64.c
sysdeps/unix/sysv/linux/pwritev.c
sysdeps/unix/sysv/linux/pwritev64.c

index 04e88e75332062c73edf76f8de2604acf72b0850..7a0a45a8454fdcc4eeccf0e39f8ebebb01450adf 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,16 @@
+2017-05-15  Adhemerval Zanella  <adhemerval.zanella@linaro.org>
+
+       * sysdeps/posix/preadv.c: Use sysdeps/posix/preadv_common.c.
+       * sysdeps/posix/preadv64.c: Likewise.
+       * sysdeps/unix/sysv/linux/preadv.c: Likewise.
+       * sysdeps/unix/sysv/linux/preadv64.c: Likewise.
+       * sysdeps/posix/pwritev.c: Use sysdeps/posix/pwritev_common.c.
+       * sysdeps/posix/pwritev64.c: Likewise.
+       * sysdeps/unix/sysv/linux/pwritev.c: Likewise.
+       * sysdeps/unix/sysv/linux/pwritev64.c: Likewise.
+       * sysdeps/posix/preadv_common.c: New file.
+       * sysdeps/posix/pwritev_common.c: Likewise.
+
 2017-05-14  Gabriel F. T. Gomes  <gftg@linux.vnet.ibm.com>
 
        * sysdeps/generic/math-type-macros-float128.h
index 7539bf85a94aa50b43f2e0a1d6d880bf0c36ddad..7819a938a1ee1a149277322cccfb04d5bbe2bfe7 100644 (file)
@@ -1,4 +1,5 @@
-/* Copyright (C) 2009-2017 Free Software Foundation, Inc.
+/* Read data into multiple buffers.  Generic version.
+   Copyright (C) 2009-2017 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
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
-#include <errno.h>
-#include <stdlib.h>
-#include <unistd.h>
-#include <string.h>
-#include <limits.h>
-#include <stdbool.h>
-#include <sys/param.h>
-#if __WORDSIZE == 64 && !defined PREADV
-/* Hide the preadv64 declaration.  */
-# define preadv64 __redirect_preadv64
-#endif
-#include <sys/uio.h>
-#include <bits/wordsize.h>
+#include <sys/types.h>
+
+#ifndef __OFF_T_MATCHES_OFF64_T
 
-#ifndef PREADV
 # define PREADV preadv
 # define PREAD __pread
 # define OFF_T off_t
-#endif
-
-
-static void
-ifree (char **ptrp)
-{
-  free (*ptrp);
-}
-
-
-/* Read data from file descriptor FD at the given position OFFSET
-   without change the file pointer, and put the result in the buffers
-   described by VECTOR, which is a vector of COUNT 'struct iovec's.
-   The buffers are filled in the order specified.  Operates just like
-   'pread' (see <unistd.h>) except that data are put in VECTOR instead
-   of a contiguous buffer.  */
-ssize_t
-PREADV (int fd, const struct iovec *vector, int count, OFF_T offset)
-{
-  /* Find the total number of bytes to be read.  */
-  size_t bytes = 0;
-  for (int i = 0; i < count; ++i)
-    {
-      /* Check for ssize_t overflow.  */
-      if (SSIZE_MAX - bytes < vector[i].iov_len)
-       {
-         __set_errno (EINVAL);
-         return -1;
-       }
-      bytes += vector[i].iov_len;
-    }
-
-  /* Allocate a temporary buffer to hold the data.  We should normally
-     use alloca since it's faster and does not require synchronization
-     with other threads.  But we cannot if the amount of memory
-     required is too large.  */
-  char *buffer;
-  char *malloced_buffer __attribute__ ((__cleanup__ (ifree))) = NULL;
-  if (__libc_use_alloca (bytes))
-    buffer = (char *) __alloca (bytes);
-  else
-    {
-      malloced_buffer = buffer = (char *) malloc (bytes);
-      if (buffer == NULL)
-       return -1;
-    }
-
-  /* Read the data.  */
-  ssize_t bytes_read = PREAD (fd, buffer, bytes, offset);
-  if (bytes_read < 0)
-    return -1;
-
-  /* Copy the data from BUFFER into the memory specified by VECTOR.  */
-  bytes = bytes_read;
-  for (int i = 0; i < count; ++i)
-    {
-      size_t copy = MIN (vector[i].iov_len, bytes);
-
-      (void) memcpy ((void *) vector[i].iov_base, (void *) buffer, copy);
+# include <sysdeps/posix/preadv_common.c>
 
-      buffer += copy;
-      bytes -= copy;
-      if (bytes == 0)
-       break;
-    }
+libc_hidden_def (preadv)
 
-  return bytes_read;
-}
-#if __WORDSIZE == 64 && defined preadv64
-# undef preadv64
-strong_alias (preadv, preadv64)
 #endif
index 198622353aca2dd77b9bddbbe2e1d23645267315..374b5bc45cc61faee2be0168f466b0ed889f6692 100644 (file)
@@ -1,9 +1,28 @@
-#include <bits/wordsize.h>
+/* Read data into multiple buffers.  Generic LFS version.
+   Copyright (C) 2009-2017 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
 
-#if __WORDSIZE == 32
-# define PREADV preadv64
-# define PREAD __pread64
-# define OFF_T off64_t
+   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.
 
-# include "preadv.c"
+   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
+   <http://www.gnu.org/licenses/>.  */
+
+#define PREADV preadv64
+#define PREAD __pread64
+#define OFF_T off64_t
+#include <sysdeps/posix/preadv_common.c>
+
+libc_hidden_def (preadv64)
+#ifdef __OFF_T_MATCHES_OFF64_T
+strong_alias (preadv64, preadv)
+libc_hidden_def (preadv)
 #endif
diff --git a/sysdeps/posix/preadv_common.c b/sysdeps/posix/preadv_common.c
new file mode 100644 (file)
index 0000000..37efdc0
--- /dev/null
@@ -0,0 +1,83 @@
+/* Read data into multiple buffers.  Base implementation for preadv
+   and preadv64.
+   Copyright (C) 2017 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
+   <http://www.gnu.org/licenses/>.  */
+
+#include <unistd.h>
+#include <sys/uio.h>
+#include <sys/param.h>
+#include <errno.h>
+#include <malloc.h>
+
+#include <ldsodefs.h>
+
+/* Read data from file descriptor FD at the given position OFFSET
+   without change the file pointer, and put the result in the buffers
+   described by VECTOR, which is a vector of COUNT 'struct iovec's.
+   The buffers are filled in the order specified.  Operates just like
+   'pread' (see <unistd.h>) except that data are put in VECTOR instead
+   of a contiguous buffer.  */
+ssize_t
+PREADV (int fd, const struct iovec *vector, int count, OFF_T offset)
+{
+  /* Find the total number of bytes to be read.  */
+  size_t bytes = 0;
+  for (int i = 0; i < count; ++i)
+    {
+      /* Check for ssize_t overflow.  */
+      if (SSIZE_MAX - bytes < vector[i].iov_len)
+       {
+         __set_errno (EINVAL);
+         return -1;
+       }
+      bytes += vector[i].iov_len;
+    }
+
+  /* Allocate a temporary buffer to hold the data.  It could be done with a
+     stack allocation, but due limitations on some system (Linux with
+     O_DIRECT) it aligns the buffer to pagesize.  A possible optimization
+     would be querying if the syscall would impose any alignment constraint,
+     but 1. it is system specific (not meant in generic implementation), and
+     2. it would make the implementation more complex, and 3. it will require
+     another syscall (fcntl).  */
+  void *buffer = NULL;
+  if (__posix_memalign (&buffer, GLRO(dl_pagesize), bytes) != 0)
+    return -1;
+
+  ssize_t bytes_read = PREAD (fd, buffer, bytes, offset);
+  if (bytes_read < 0)
+    goto end;
+
+  /* Copy the data from BUFFER into the memory specified by VECTOR.  */
+  bytes = bytes_read;
+  void *buf = buffer;
+  for (int i = 0; i < count; ++i)
+    {
+      size_t copy = MIN (vector[i].iov_len, bytes);
+
+      memcpy (vector[i].iov_base, buf, copy);
+
+      buf += copy;
+      bytes -= copy;
+      if (bytes == 0)
+       break;
+    }
+
+end:
+  free (buffer);
+  return bytes_read;
+}
index 57e641b8b50d8e74e2aab239724c115d095c5766..f9de092aa5d71ae3268c207fac59086acef50fc6 100644 (file)
@@ -1,4 +1,5 @@
-/* Copyright (C) 2009-2017 Free Software Foundation, Inc.
+/* Write data into multiple buffers.  Generic version.
+   Copyright (C) 2009-2017 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
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
-#include <errno.h>
-#include <stdlib.h>
-#include <unistd.h>
-#include <string.h>
-#include <limits.h>
-#include <stdbool.h>
-#include <sys/param.h>
-#if __WORDSIZE == 64 && !defined PWRITEV
-/* Hide the pwritev64 declaration.  */
-# define pwritev64 __redirect_pwritev64
-#endif
-#include <sys/uio.h>
-#include <bits/wordsize.h>
+#include <sys/types.h>
+
+#ifndef __OFF_T_MATCHES_OFF64_T
 
-#ifndef PWRITEV
 # define PWRITEV pwritev
 # define PWRITE __pwrite
 # define OFF_T off_t
-#endif
-
-
-static void
-ifree (char **ptrp)
-{
-  free (*ptrp);
-}
-
-
-/* Write data pointed by the buffers described by IOVEC, which is a
-   vector of COUNT 'struct iovec's, to file descriptor FD at the given
-   position OFFSET without change the file pointer.  The data is
-   written in the order specified.  Operates just like 'write' (see
-   <unistd.h>) except that the data are taken from IOVEC instead of a
-   contiguous buffer.  */
-ssize_t
-PWRITEV (int fd, const struct iovec *vector, int count, OFF_T offset)
-{
-  /* Find the total number of bytes to be read.  */
-  size_t bytes = 0;
-  for (int i = 0; i < count; ++i)
-    {
-      /* Check for ssize_t overflow.  */
-      if (SSIZE_MAX - bytes < vector[i].iov_len)
-       {
-         __set_errno (EINVAL);
-         return -1;
-       }
-      bytes += vector[i].iov_len;
-    }
-
-  /* Allocate a temporary buffer to hold the data.  We should normally
-     use alloca since it's faster and does not require synchronization
-     with other threads.  But we cannot if the amount of memory
-     required is too large.  */
-  char *buffer;
-  char *malloced_buffer __attribute__ ((__cleanup__ (ifree))) = NULL;
-  if (__libc_use_alloca (bytes))
-    buffer = (char *) __alloca (bytes);
-  else
-    {
-      malloced_buffer = buffer = (char *) malloc (bytes);
-      if (buffer == NULL)
-       return -1;
-    }
+# include <sysdeps/posix/pwritev_common.c>
 
-  /* Copy the data from BUFFER into the memory specified by VECTOR.  */
-  char *ptr = buffer;
-  for (int i = 0; i < count; ++i)
-    ptr = __mempcpy ((void *) ptr, (void *) vector[i].iov_base,
-                    vector[i].iov_len);
+libc_hidden_def (pwritev)
 
-  /* Write the data.  */
-  return PWRITE (fd, buffer, bytes, offset);
-}
-#if __WORDSIZE == 64 && defined pwritev64
-# undef pwritev64
-strong_alias (pwritev, pwritev64)
 #endif
index 4948d2efee8ab7441f26ab92d99f3dc66320de67..c5392f78366f186f98d3a349d8338855c96589db 100644 (file)
@@ -1,9 +1,28 @@
-#include <bits/wordsize.h>
+/* Write data into multiple buffers.  Generic LFS version.
+   Copyright (C) 2009-2017 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
 
-#if __WORDSIZE == 32
-# define PWRITEV pwritev64
-# define PWRITE __pwrite64
-# define OFF_T off64_t
+   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.
 
-# include "pwritev.c"
+   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
+   <http://www.gnu.org/licenses/>.  */
+
+#define PWRITEV pwritev64
+#define PWRITE __pwrite64
+#define OFF_T off64_t
+#include <sysdeps/posix/pwritev_common.c>
+
+libc_hidden_def (pwritev64)
+#ifdef __OFF_T_MATCHES_OFF64_T
+strong_alias (pwritev64, pwritev)
+libc_hidden_def (pwritev)
 #endif
diff --git a/sysdeps/posix/pwritev_common.c b/sysdeps/posix/pwritev_common.c
new file mode 100644 (file)
index 0000000..0383065
--- /dev/null
@@ -0,0 +1,72 @@
+/* Write data into multiple buffers.  Base implementation for pwritev
+   and pwritev64.
+   Copyright (C) 2017 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
+   <http://www.gnu.org/licenses/>.  */
+
+#include <unistd.h>
+#include <sys/uio.h>
+#include <sys/param.h>
+#include <errno.h>
+#include <malloc.h>
+
+#include <ldsodefs.h>
+
+/* Write data pointed by the buffers described by IOVEC, which is a
+   vector of COUNT 'struct iovec's, to file descriptor FD at the given
+   position OFFSET without change the file pointer.  The data is
+   written in the order specified.  Operates just like 'write' (see
+   <unistd.h>) except that the data are taken from IOVEC instead of a
+   contiguous buffer.  */
+ssize_t
+PWRITEV (int fd, const struct iovec *vector, int count, OFF_T offset)
+{
+  /* Find the total number of bytes to be read.  */
+  size_t bytes = 0;
+  for (int i = 0; i < count; ++i)
+    {
+      /* Check for ssize_t overflow.  */
+      if (SSIZE_MAX - bytes < vector[i].iov_len)
+       {
+         __set_errno (EINVAL);
+         return -1;
+       }
+      bytes += vector[i].iov_len;
+    }
+
+  /* Allocate a temporary buffer to hold the data.  It could be done with a
+     stack allocation, but due limitations on some system (Linux with
+     O_DIRECT) it aligns the buffer to pagesize.  A possible optimization
+     would be querying if the syscall would impose any alignment constraint,
+     but 1. it is system specific (not meant in generic implementation), and
+     2. it would make the implementation more complex, and 3. it will require
+     another syscall (fcntl).  */
+  void *buffer = NULL;
+  if (__posix_memalign (&buffer, GLRO(dl_pagesize), bytes) != 0)
+    return -1;
+
+  /* Copy the data from BUFFER into the memory specified by VECTOR.  */
+  char *ptr = buffer;
+  for (int i = 0; i < count; ++i)
+    ptr = __mempcpy ((void *) ptr, (void *) vector[i].iov_base,
+                    vector[i].iov_len);
+
+  ssize_t ret = PWRITE (fd, buffer, bytes, offset);
+
+  free (buffer);
+
+  return ret;
+}
index ccfe7636d629b54546ab45bc2ffe265894692d78..7d971cc7b31940b5493542f93907e13830999bab 100644 (file)
@@ -48,6 +48,6 @@ preadv (int fd, const struct iovec *vector, int count, off_t offset)
 #  define PREADV static internal_function __atomic_preadv_replacement
 #  define PREAD __pread
 #  define OFF_T off_t
-#  include <sysdeps/posix/preadv.c>
+#  include <sysdeps/posix/preadv_common.c>
 # endif /* __ASSUME_PREADV  */
 #endif
index 979db95571c95d6d95e6f65522ab5991f5b9c35c..66daa74ded0d3b651aab119de6655b4afdd370ab 100644 (file)
@@ -46,7 +46,7 @@ preadv64 (int fd, const struct iovec *vector, int count, off64_t offset)
 # define PREADV static internal_function __atomic_preadv64_replacement
 # define PREAD __pread64
 # define OFF_T off64_t
-# include <sysdeps/posix/preadv.c>
+# include <sysdeps/posix/preadv_common.c>
 #endif
 
 #ifdef __OFF_T_MATCHES_OFF64_T
index 278994376098b063ebf348e7241a1b274830411e..ce02996a0b9a84b1eb01d727d709995ffeb28a91 100644 (file)
@@ -48,6 +48,6 @@ pwritev (int fd, const struct iovec *vector, int count, off_t offset)
 #  define PWRITEV static internal_function __atomic_pwritev_replacement
 #  define PWRITE __pwrite
 #  define OFF_T off_t
-#  include <sysdeps/posix/pwritev.c>
+#  include <sysdeps/posix/pwritev_common.c>
 # endif /* __ASSUME_PREADV  */
 #endif
index 1e3a36ca58bc41ec303c9742923d7b6b3235933b..45fb90b0d70a6862d7eebac8ebe6d7a0f603e22a 100644 (file)
@@ -46,7 +46,7 @@ pwritev64 (int fd, const struct iovec *vector, int count, off64_t offset)
 # define PWRITEV static internal_function __atomic_pwritev64_replacement
 # define PWRITE __pwrite64
 # define OFF_T off64_t
-# include <sysdeps/posix/pwritev.c>
+# include <sysdeps/posix/pwritev_common.c>
 #endif
 
 #ifdef __OFF_T_MATCHES_OFF64_T