v3-0011-Simplify-replacement-code-for-preadv-and-pwritev.patch

From 81c6a55c7bc3788c77798e51197851f926ccd221 Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.munro@gmail.com>
Date: Sat, 23 Jul 2022 23:24:14 +1200
Subject: [PATCH v3 11/13] Simplify replacement code for preadv and pwritev.

preadv() and pwritev() are not standardized by POSIX.  Most targeted
Unix systems have had them for more than a decade, since they are
obvious combinations of standard p- and -v functions.

In 15, we had two replacement implementations: one based on lseek() + -v
function if available, and the other based on a loop over p- function.
They aren't used for much yet, but are heavily used in a current
proposal.

Supporting two ways of falling back, at the cost of having a
pg_preadv/pg_pwritev that could never be used in a multi-threaded
program accessing the same file descriptor from two threads without
unpleasant locking does not sound like a good trade.

Therefore, drop the lseek()-based variant, and also the pg_ prefix, now
that the file position portability hazard is gone.  Previously, both
fallbacks had the file position portability hazard, because our
pread()/pwrite() replacement had the same hazard, but that problem has
been fixed for pread()/pwrite() by an earlier commit.  Now the way is
clear to expunge the file position portability hazard of the
lseek()-based variants too.

At the time of writing, the following systems in our build farm lack
native preadv/pwritev and thus use fallback code:

 * Solaris (but not illumos)
 * macOS before release 11.0
 * Windows with Cygwin
 * Windows native

With this commit, all of the above systems will now use the *same*
fallback code, the one that loops over pread()/pwrite() (which is
translated to equivalent calls in Windows).  Previously, all but Windows
native would use the readv()/writev()-based fallback that this commit
removes.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CA+hUKGJ3LHeP9w5Fgzdr4G8AnEtJ=z=p6hGDEm4qYGEUX5B6fQ@mail.gmail.com
---
 configure                     |  2 +-
 configure.ac                  |  2 --
 src/backend/storage/file/fd.c |  4 ++--
 src/include/pg_config.h.in    |  6 ------
 src/include/port/pg_iovec.h   | 12 ++++--------
 src/port/preadv.c             | 13 +------------
 src/port/pwritev.c            | 13 +------------
 src/tools/msvc/Solution.pm    |  2 --
 8 files changed, 9 insertions(+), 45 deletions(-)

diff --git a/configure b/configure
index 4d81993515..21f4520f81 100755
--- a/configure
+++ b/configure
@@ -16039,7 +16039,7 @@ fi
 LIBS_including_readline="$LIBS"
 LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'`
 
-for ac_func in backtrace_symbols copyfile fdatasync getifaddrs getpeerucred inet_pton kqueue mbstowcs_l memset_s posix_fallocate ppoll pthread_is_threaded_np readv setproctitle setproctitle_fast strchrnul strsignal syncfs sync_file_range uselocale wcstombs_l writev
+for ac_func in backtrace_symbols copyfile fdatasync getifaddrs getpeerucred inet_pton kqueue mbstowcs_l memset_s posix_fallocate ppoll pthread_is_threaded_np setproctitle setproctitle_fast strchrnul strsignal syncfs sync_file_range uselocale wcstombs_l
 do :
   as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
 ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var"
diff --git a/configure.ac b/configure.ac
index d36d983a57..a7ab660179 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1802,7 +1802,6 @@ AC_CHECK_FUNCS(m4_normalize([
 	posix_fallocate
 	ppoll
 	pthread_is_threaded_np
-	readv
 	setproctitle
 	setproctitle_fast
 	strchrnul
@@ -1811,7 +1810,6 @@ AC_CHECK_FUNCS(m4_normalize([
 	sync_file_range
 	uselocale
 	wcstombs_l
-	writev
 ]))
 
 # These typically are compiler builtins, for which AC_CHECK_FUNCS fails.
diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c
index 3d1a973354..5a2eb01238 100644
--- a/src/backend/storage/file/fd.c
+++ b/src/backend/storage/file/fd.c
@@ -3762,7 +3762,7 @@ data_sync_elevel(int elevel)
 }
 
 /*
- * A convenience wrapper for pg_pwritev() that retries on partial write.  If an
+ * A convenience wrapper for pwritev() that retries on partial write.  If an
  * error is returned, it is unspecified how much has been written.
  */
 ssize_t
@@ -3782,7 +3782,7 @@ pg_pwritev_with_retry(int fd, const struct iovec *iov, int iovcnt, off_t offset)
 	for (;;)
 	{
 		/* Write as much as we can. */
-		part = pg_pwritev(fd, iov, iovcnt, offset);
+		part = pwritev(fd, iov, iovcnt, offset);
 		if (part < 0)
 			return -1;
 
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index 29c5d60210..6505a84211 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -409,9 +409,6 @@
 /* Define to 1 if you have the <readline/readline.h> header file. */
 #undef HAVE_READLINE_READLINE_H
 
-/* Define to 1 if you have the `readv' function. */
-#undef HAVE_READV
-
 /* Define to 1 if you have the `rl_completion_matches' function. */
 #undef HAVE_RL_COMPLETION_MATCHES
 
@@ -640,9 +637,6 @@
 /* Define to 1 if you have the <winldap.h> header file. */
 #undef HAVE_WINLDAP_H
 
-/* Define to 1 if you have the `writev' function. */
-#undef HAVE_WRITEV
-
 /* Define to 1 if you have the `X509_get_signature_nid' function. */
 #undef HAVE_X509_GET_SIGNATURE_NID
 
diff --git a/src/include/port/pg_iovec.h b/src/include/port/pg_iovec.h
index f0b1a71bcb..f0a50c0e01 100644
--- a/src/include/port/pg_iovec.h
+++ b/src/include/port/pg_iovec.h
@@ -39,16 +39,12 @@ struct iovec
 /* Define a reasonable maximum that is safe to use on the stack. */
 #define PG_IOV_MAX Min(IOV_MAX, 32)
 
-#if HAVE_DECL_PREADV
-#define pg_preadv preadv
-#else
-extern ssize_t pg_preadv(int fd, const struct iovec *iov, int iovcnt, off_t offset);
+#if !HAVE_DECL_PREADV
+extern ssize_t preadv(int fd, const struct iovec *iov, int iovcnt, off_t offset);
 #endif
 
-#if HAVE_DECL_PWRITEV
-#define pg_pwritev pwritev
-#else
-extern ssize_t pg_pwritev(int fd, const struct iovec *iov, int iovcnt, off_t offset);
+#if !HAVE_DECL_PWRITEV
+extern ssize_t pwritev(int fd, const struct iovec *iov, int iovcnt, off_t offset);
 #endif
 
 #endif							/* PG_IOVEC_H */
diff --git a/src/port/preadv.c b/src/port/preadv.c
index aa7537503f..0aaf99222f 100644
--- a/src/port/preadv.c
+++ b/src/port/preadv.c
@@ -8,9 +8,6 @@
  * IDENTIFICATION
  *	  src/port/preadv.c
  *
- * Note that this implementation changes the current file position, unlike
- * the POSIX-like function, so we use the name pg_preadv().
- *
  *-------------------------------------------------------------------------
  */
 
@@ -26,15 +23,8 @@
 #include "port/pg_iovec.h"
 
 ssize_t
-pg_preadv(int fd, const struct iovec *iov, int iovcnt, off_t offset)
+preadv(int fd, const struct iovec *iov, int iovcnt, off_t offset)
 {
-#ifdef HAVE_READV
-	if (iovcnt == 1)
-		return pread(fd, iov[0].iov_base, iov[0].iov_len, offset);
-	if (lseek(fd, offset, SEEK_SET) < 0)
-		return -1;
-	return readv(fd, iov, iovcnt);
-#else
 	ssize_t		sum = 0;
 	ssize_t		part;
 
@@ -54,5 +44,4 @@ pg_preadv(int fd, const struct iovec *iov, int iovcnt, off_t offset)
 			return sum;
 	}
 	return sum;
-#endif
 }
diff --git a/src/port/pwritev.c b/src/port/pwritev.c
index cb7421381e..a0fa4b8edd 100644
--- a/src/port/pwritev.c
+++ b/src/port/pwritev.c
@@ -8,9 +8,6 @@
  * IDENTIFICATION
  *	  src/port/pwritev.c
  *
- * Note that this implementation changes the current file position, unlike
- * the POSIX-like function, so we use the name pg_pwritev().
- *
  *-------------------------------------------------------------------------
  */
 
@@ -26,15 +23,8 @@
 #include "port/pg_iovec.h"
 
 ssize_t
-pg_pwritev(int fd, const struct iovec *iov, int iovcnt, off_t offset)
+pwritev(int fd, const struct iovec *iov, int iovcnt, off_t offset)
 {
-#ifdef HAVE_WRITEV
-	if (iovcnt == 1)
-		return pwrite(fd, iov[0].iov_base, iov[0].iov_len, offset);
-	if (lseek(fd, offset, SEEK_SET) < 0)
-		return -1;
-	return writev(fd, iov, iovcnt);
-#else
 	ssize_t		sum = 0;
 	ssize_t		part;
 
@@ -54,5 +44,4 @@ pg_pwritev(int fd, const struct iovec *iov, int iovcnt, off_t offset)
 			return sum;
 	}
 	return sum;
-#endif
 }
diff --git a/src/tools/msvc/Solution.pm b/src/tools/msvc/Solution.pm
index 7a02198621..bd8b755b96 100644
--- a/src/tools/msvc/Solution.pm
+++ b/src/tools/msvc/Solution.pm
@@ -331,7 +331,6 @@ sub GenerateFiles
 		HAVE_READLINE_H             => undef,
 		HAVE_READLINE_HISTORY_H     => undef,
 		HAVE_READLINE_READLINE_H    => undef,
-		HAVE_READV                  => undef,
 		HAVE_RL_COMPLETION_MATCHES  => undef,
 		HAVE_RL_COMPLETION_SUPPRESS_QUOTE        => undef,
 		HAVE_RL_FILENAME_COMPLETION_FUNCTION     => undef,
@@ -407,7 +406,6 @@ sub GenerateFiles
 		HAVE_WINLDAP_H                           => undef,
 		HAVE_WCSTOMBS_L                          => 1,
 		HAVE_VISIBILITY_ATTRIBUTE                => undef,
-		HAVE_WRITEV                              => undef,
 		HAVE_X509_GET_SIGNATURE_NID              => 1,
 		HAVE_X86_64_POPCNTQ                      => undef,
 		HAVE__BOOL                               => undef,
-- 
2.37.1