From 1d03c0b2811b53a20eeff781918ef99d4c9b9df9 Mon Sep 17 00:00:00 2001 From: Thomas Munro Date: Mon, 6 Feb 2023 13:51:19 +1300 Subject: [PATCH v5 1/3] Use accept4() to accept connections, where available. The postmaster previously accepted connections and then made extra system calls in child processes to make them non-blocking and (as of a recent commit) close-on-exit. On all target Unixes except macOS and AIX, that can be done atomically with flags to the newer accept4() variant (expected in the next POSIX revision, but around in the wild for many years now), saving a couple of system calls. The exceptions are Windows, where we didn't have to worry about that problem anyway, EXEC_BACKEND builds on Unix (used by developers only for slightly-more-like-Windows testing), and the straggler Unixes that haven't implemented it yet (at the time of writing: macOS and AIX). Suggested-by: Andres Freund Discussion: https://postgr.es/m/CA%2BhUKGKb6FsAdQWcRL35KJsftv%2B9zXqQbzwkfRf1i0J2e57%2BhQ%40mail.gmail.com --- configure | 12 ++++++++++++ configure.ac | 1 + meson.build | 1 + src/backend/libpq/pqcomm.c | 34 +++++++++++++++++++++++++++------- src/include/pg_config.h.in | 4 ++++ 5 files changed, 45 insertions(+), 7 deletions(-) diff --git a/configure b/configure index e35769ea73..d06f64cdbb 100755 --- a/configure +++ b/configure @@ -16219,6 +16219,18 @@ _ACEOF # We can't use AC_REPLACE_FUNCS to replace these functions, because it # won't handle deployment target restrictions on macOS +ac_fn_c_check_decl "$LINENO" "accept4" "ac_cv_have_decl_accept4" "#include +" +if test "x$ac_cv_have_decl_accept4" = xyes; then : + ac_have_decl=1 +else + ac_have_decl=0 +fi + +cat >>confdefs.h <<_ACEOF +#define HAVE_DECL_ACCEPT4 $ac_have_decl +_ACEOF + ac_fn_c_check_decl "$LINENO" "preadv" "ac_cv_have_decl_preadv" "#include " if test "x$ac_cv_have_decl_preadv" = xyes; then : diff --git a/configure.ac b/configure.ac index af23c15cb2..070a0b33db 100644 --- a/configure.ac +++ b/configure.ac @@ -1843,6 +1843,7 @@ AC_CHECK_DECLS([strlcat, strlcpy, strnlen]) # We can't use AC_REPLACE_FUNCS to replace these functions, because it # won't handle deployment target restrictions on macOS +AC_CHECK_DECLS([accept4], [], [], [#include ]) AC_CHECK_DECLS([preadv], [], [AC_LIBOBJ(preadv)], [#include ]) AC_CHECK_DECLS([pwritev], [], [AC_LIBOBJ(pwritev)], [#include ]) diff --git a/meson.build b/meson.build index 26be83afb6..c91fb05133 100644 --- a/meson.build +++ b/meson.build @@ -2097,6 +2097,7 @@ decl_checks = [ # checking for library symbols wouldn't handle deployment target # restrictions on macOS decl_checks += [ + ['accept4', 'sys/socket.h'], ['preadv', 'sys/uio.h'], ['pwritev', 'sys/uio.h'], ] diff --git a/src/backend/libpq/pqcomm.c b/src/backend/libpq/pqcomm.c index da5bb5fc5d..8b97d1a7e3 100644 --- a/src/backend/libpq/pqcomm.c +++ b/src/backend/libpq/pqcomm.c @@ -184,6 +184,14 @@ pq_init(void) /* set up process-exit hook to close the socket */ on_proc_exit(socket_close, 0); +#ifndef WIN32 + /* + * On most systems, the socket has already been made non-blocking and + * close-on-exec by StreamConnection(). On systems that don't have + * accept4() yet, and EXEC_BACKEND builds that need the socket to survive + * exec*(), we do that separately here in the child process. + */ +#if !HAVE_DECL_ACCEPT4 || defined(EXEC_BACKEND) /* * In backends (as soon as forked) we operate the underlying socket in * nonblocking mode and use latches to implement blocking semantics if @@ -194,17 +202,14 @@ pq_init(void) * the client, which might require changing the mode again, leading to * infinite recursion. */ -#ifndef WIN32 if (!pg_set_noblock(MyProcPort->sock)) ereport(COMMERROR, (errmsg("could not set socket to nonblocking mode: %m"))); -#endif - -#ifndef WIN32 /* Don't give the socket to any subprograms we execute. */ if (fcntl(MyProcPort->sock, F_SETFD, FD_CLOEXEC) < 0) elog(FATAL, "fcntl(F_SETFD) failed on socket: %m"); +#endif #endif FeBeWaitSet = CreateWaitEventSet(TopMemoryContext, FeBeWaitSetNEvents); @@ -699,9 +704,24 @@ StreamConnection(pgsocket server_fd, Port *port) { /* accept connection and fill in the client (remote) address */ port->raddr.salen = sizeof(port->raddr.addr); - if ((port->sock = accept(server_fd, - (struct sockaddr *) &port->raddr.addr, - &port->raddr.salen)) == PGINVALID_SOCKET) + +#if HAVE_DECL_ACCEPT4 && !defined(EXEC_BACKEND) + /* + * If we have accept4(), we can avoid a couple of fcntl() calls in + * pq_init(). We can't use this optimization in EXEC_BACKEND builds, + * though, because they need the socket to survive exec(). + */ + port->sock = accept4(server_fd, + (struct sockaddr *) &port->raddr.addr, + &port->raddr.salen, + SOCK_CLOEXEC | SOCK_NONBLOCK); +#else + port->sock = accept(server_fd, + (struct sockaddr *) &port->raddr.addr, + &port->raddr.salen); +#endif + + if (port->sock == PGINVALID_SOCKET) { ereport(LOG, (errcode_for_socket_access(), diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in index 20c82f5979..e21d0f05f5 100644 --- a/src/include/pg_config.h.in +++ b/src/include/pg_config.h.in @@ -91,6 +91,10 @@ /* Define to 1 if you have the `CRYPTO_lock' function. */ #undef HAVE_CRYPTO_LOCK +/* Define to 1 if you have the declaration of `accept4', and to 0 if you + don't. */ +#undef HAVE_DECL_ACCEPT4 + /* Define to 1 if you have the declaration of `fdatasync', and to 0 if you don't. */ #undef HAVE_DECL_FDATASYNC -- 2.39.1