From 81c6a55c7bc3788c77798e51197851f926ccd221 Mon Sep 17 00:00:00 2001 From: Thomas Munro 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 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 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 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