From 62faf7d37bd8b6be6657b1e3b61f92eac5b84653 Mon Sep 17 00:00:00 2001 From: Krishnan Parthasarathi Date: Thu, 11 Oct 2018 15:16:41 +0530 Subject: socket: use accept4/paccept for nonblocking socket This reduces the no. of syscalls on Linux systems from 2, accept(2) and fcntl(2) for setting O_NONBLOCK, to a single accept4(2). On NetBSD, we have paccept(2) that does the same, if we leave signal masking aside. Added sys_accept which accepts an extra flags argument than accept(2). This would opportunistically use accept4/paccept as available. It would fallback to accept(2) and fcntl(2) otherwise. While at this, the patch sets FD_CLOEXEC flag on the accepted socket fd. BUG: 1236272 Change-Id: I41e43fd3e36d6dabb07e578a1cea7f45b7b4e37f fixes: bz#1236272 Signed-off-by: Krishnan Parthasarathi --- configure.ac | 10 ++++++ libglusterfs/src/libglusterfs.sym | 1 + libglusterfs/src/syscall.c | 60 +++++++++++++++++++++++++++++++++++ libglusterfs/src/syscall.h | 4 +++ rpc/rpc-transport/socket/src/socket.c | 22 ++++--------- 5 files changed, 81 insertions(+), 16 deletions(-) diff --git a/configure.ac b/configure.ac index 9925ad918b9..afba73e18cf 100644 --- a/configure.ac +++ b/configure.ac @@ -1140,6 +1140,16 @@ if test "x${ac_cv_have_decl_SEEK_HOLE}" = "xyes"; then fi CFLAGS=${OLD_CFLAGS} +AC_CHECK_FUNC([accept4], [have_accept4=yes]) +if test "x${have_accept4}" = "xyes"; then + AC_DEFINE(HAVE_ACCEPT4, 1, [define if accept4 exists]) +fi + +AC_CHECK_FUNC([paccept], [have_paccept=yes]) +if test "x${have_paccept}" = "xyes"; then +AC_DEFINE(HAVE_PACCEPT, 1, [define if paccept exists]) +fi + # Check the distribution where you are compiling glusterfs on GF_DISTRIBUTION= diff --git a/libglusterfs/src/libglusterfs.sym b/libglusterfs/src/libglusterfs.sym index 18c9fff34a2..27eb1175b56 100644 --- a/libglusterfs/src/libglusterfs.sym +++ b/libglusterfs/src/libglusterfs.sym @@ -1052,6 +1052,7 @@ sys_utimensat sys_write sys_writev sys_socket +sys_accept tbf_init tbf_throttle timespec_now diff --git a/libglusterfs/src/syscall.c b/libglusterfs/src/syscall.c index dd504f734fc..b28f2b9d17d 100644 --- a/libglusterfs/src/syscall.c +++ b/libglusterfs/src/syscall.c @@ -741,3 +741,63 @@ sys_socket(int domain, int type, int protocol) fcntl(fd, F_SETFD, FD_CLOEXEC); return fd; } + +#if (defined(HAVE_ACCEPT4) || defined(HAVE_PACCEPT)) +static inline int +prep_accept_flags(int flags) +{ + if (flags & O_NONBLOCK) { + flags &= ~O_NONBLOCK; + flags |= SOCK_NONBLOCK; + } + + flags |= SOCK_CLOEXEC; + + return flags; +} +#endif + +int +sys_accept(int sock, struct sockaddr *sockaddr, socklen_t *socklen, int flags) +{ + int newsock = -1; + +#ifdef HAVE_ACCEPT4 + + flags = prep_accept_flags(flags); + newsock = accept4(sock, sockaddr, socklen, flags); + +#elif HAVE_PACCEPT + flags = prep_accept_flags(flags); + newsock = paccept(sock, sockaddr, socklen, NULL, flags); + +#else + int op_errno = 0; + int curflag = 0; + int ret = 0; + + newsock = accept(sock, sockaddr, socklen); + if (newsock != -1) { + curflag = fcntl(newsock, F_GETFL); + if (fcntl(newsock, F_SETFL, curflag | flags) == -1) { + op_errno = errno; + goto err; + } + + curflag = fcntl(newsock, F_GETFD); + if (fcntl(newsock, F_SETFD, curflag | FD_CLOEXEC) == -1) { + op_errno = errno; + goto err; + } + } + +err: + if (op_errno) { + close(newsock); + errno = op_errno; + return -1; + } + +#endif + return newsock; +} diff --git a/libglusterfs/src/syscall.h b/libglusterfs/src/syscall.h index 9bad49edb7c..faaf694b22c 100644 --- a/libglusterfs/src/syscall.h +++ b/libglusterfs/src/syscall.h @@ -16,6 +16,7 @@ #include #include #include +#include /* GF follows the Linux XATTR definition, which differs in Darwin. */ #define GF_XATTR_CREATE 0x1 /* set value, fail if attr already exists */ @@ -224,4 +225,7 @@ sys_pwrite(int fd, const void *buf, size_t count, off_t offset); int sys_socket(int domain, int type, int protocol); +int +sys_accept(int sock, struct sockaddr *sockaddr, socklen_t *socklen, int flags); + #endif /* __SYSCALL_H__ */ diff --git a/rpc/rpc-transport/socket/src/socket.c b/rpc/rpc-transport/socket/src/socket.c index 753722cdb61..9d926e6e078 100644 --- a/rpc/rpc-transport/socket/src/socket.c +++ b/rpc/rpc-transport/socket/src/socket.c @@ -2992,7 +2992,12 @@ socket_server_event_handler(int fd, int idx, int gen, void *data, int poll_in, priv->idx = idx; if (poll_in) { - new_sock = accept(priv->sock, SA(&new_sockaddr), &addrlen); + int aflags = 0; + + if (!priv->bio) + aflags = O_NONBLOCK; + + new_sock = sys_accept(priv->sock, SA(&new_sockaddr), &addrlen, aflags); if (ctx) event_handled(ctx->event_pool, fd, idx, gen); @@ -3101,21 +3106,6 @@ socket_server_event_handler(int fd, int idx, int gen, void *data, int poll_in, new_priv->connected = 1; new_priv->is_server = _gf_true; - /* set O_NONBLOCK for plain text as well as ssl connections */ - if (!priv->bio) { - gf_log(this->name, GF_LOG_TRACE, "### use non-blocking IO"); - ret = __socket_nonblock(new_sock); - - if (ret == -1) { - gf_log(this->name, GF_LOG_WARNING, "NBIO on %d failed (%s)", - new_sock, strerror(errno)); - - sys_close(new_sock); - GF_FREE(new_trans->name); - GF_FREE(new_trans); - goto out; - } - } /* * This is the first ref on the newly accepted * transport. -- cgit