From patchwork Thu Nov 13 21:53:52 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alexandre Oliva X-Patchwork-Id: 3721 Received: (qmail 5400 invoked by alias); 13 Nov 2014 21:54:11 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 5389 invoked by uid 89); 13 Nov 2014 21:54:10 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.5 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com From: Alexandre Oliva To: libc-alpha@sourceware.org Subject: BZ#15819: introduce internal function to ease poll retry with timeout Date: Thu, 13 Nov 2014 19:53:52 -0200 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.2 (gnu/linux) MIME-Version: 1.0 Here's a formal submission of the patch I posted asking for feedback on how to introduce this sort of wrapper, a few days ago. Regression tested on x86_64-linux-gnu. Ok to install? for ChangeLog BZ #15819 * NEWS: Update. * include/sys/poll.h: Include errno.h and sys/time.h. (__poll_noeintr): New function. * nis/nis_callback.c (internal_nis_do_callback): Use new fn, drop TEMP_FAILURE_RETRY. * nscd/nscd_helper.c (wait_on_socket): Use new fn, drop loop and gettimeofday timeout recomputation. * sunrpc/clnt_udp.c (clntudp_call): Use new fn. * sunrpc/clnt_unix.c (readunix): Likewise. * sunrpc/rtime.c (rtime): Likewise. * sunrpc/svc_run.c (svc_run): Likewise. * sunrpc/svc_tcp.c (readtcp): Likewise. * sunrpc/svc_unix.c (readunix): Likewise. --- NEWS | 8 +++---- include/sys/poll.h | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++- nis/nis_callback.c | 3 +- nscd/nscd_helper.c | 24 +------------------- sunrpc/clnt_udp.c | 5 +--- sunrpc/clnt_unix.c | 22 +++++++----------- sunrpc/rtime.c | 4 +-- sunrpc/svc_run.c | 4 +-- sunrpc/svc_tcp.c | 29 +++++++++--------------- sunrpc/svc_unix.c | 29 +++++++++--------------- 10 files changed, 101 insertions(+), 90 deletions(-) diff --git a/NEWS b/NEWS index 9ed697c..af86a93 100644 --- a/NEWS +++ b/NEWS @@ -9,10 +9,10 @@ Version 2.21 * The following bugs are resolved with this release: - 6652, 12926, 14132, 14138, 14171, 14498, 15215, 15884, 17266, 17344, - 17363, 17370, 17371, 17411, 17460, 17475, 17485, 17501, 17506, 17508, - 17522, 17555, 17570, 17571, 17572, 17573, 17574, 17582, 17583, 17584, - 17585, 17589. + 6652, 12926, 14132, 14138, 14171, 14498, 15215, 15819, 15884, 17266, + 17344, 17363, 17370, 17371, 17411, 17460, 17475, 17485, 17501, 17506, + 17508, 17522, 17555, 17570, 17571, 17572, 17573, 17574, 17582, 17583, + 17584, 17585, 17589. * New locales: tu_IN, bh_IN. diff --git a/include/sys/poll.h b/include/sys/poll.h index a42bc93..bfb5d8a 100644 --- a/include/sys/poll.h +++ b/include/sys/poll.h @@ -6,6 +6,67 @@ extern int __poll (struct pollfd *__fds, unsigned long int __nfds, int __timeout); libc_hidden_proto (__poll) libc_hidden_proto (ppoll) -#endif + +#include +#include + +static inline int +__poll_noeintr (struct pollfd *__fds, unsigned long int __nfds, + int __timeout) +{ + int __ret; + + __retry_poll: + __ret = __poll (__fds, __nfds, __timeout); + + if (__ret == -1 && __glibc_unlikely (errno == EINTR)) + { + /* Handle the case where the poll() call is interrupted by a + signal. We cannot just use TEMP_FAILURE_RETRY since it might + lead to infinite loops. We can't tell how long poll has + already waited, and we can't assume the existence of a + higher-precision clock, but that's ok-ish: the timeout is a + lower bound, we just have to make sure we don't wait + indefinitely. */ + struct timespec __tscur, __tsend; + /* Remember which clock to use. */ + static clockid_t __xclk = CLOCK_MONOTONIC; + clockid_t __clk = __xclk; + + __try_another_clock: + if (__glibc_unlikely (__clock_gettime (__clk, &__tscur) == -1)) + { + if (errno == EINVAL && __clk == CLOCK_MONOTONIC) + { + __xclk = __clk = CLOCK_REALTIME; + goto __try_another_clock; + } + + /* At least CLOCK_REALTIME should always be supported, but + if clock_gettime fails for any other reason, the best we + can do is to retry with a slightly lower timeout, until + we complete without interruption. */ + __timeout--; + goto __retry_poll; + } + + __tsend.tv_sec = __tscur.tv_sec + __timeout / 1000; + __tsend.tv_nsec = __tscur.tv_nsec + __timeout % 1000 * 1000000L + 500000; + + while (1) + { + __ret = __poll (__fds, __nfds, + (__tsend.tv_sec - __tscur.tv_sec) * 1000 + + (__tsend.tv_nsec - __tscur.tv_nsec) / 1000000); + if (__ret != -1 || errno != EINTR) + break; + + (void) __clock_gettime (__clk, &__tscur); + } + } + + return __ret; +} +#endif /* _ISOMAC */ #endif diff --git a/nis/nis_callback.c b/nis/nis_callback.c index e352733..baa8d75 100644 --- a/nis/nis_callback.c +++ b/nis/nis_callback.c @@ -215,8 +215,7 @@ internal_nis_do_callback (struct dir_binding *bptr, netobj *cookie, my_pollfd[i].revents = 0; } - switch (i = TEMP_FAILURE_RETRY (__poll (my_pollfd, svc_max_pollfd, - 25*1000))) + switch (i = __poll_noeintr (my_pollfd, svc_max_pollfd, 25*1000)) { case -1: return NIS_CBERROR; diff --git a/nscd/nscd_helper.c b/nscd/nscd_helper.c index ee3b67f..fe5f096 100644 --- a/nscd/nscd_helper.c +++ b/nscd/nscd_helper.c @@ -53,29 +53,7 @@ wait_on_socket (int sock, long int usectmo) struct pollfd fds[1]; fds[0].fd = sock; fds[0].events = POLLIN | POLLERR | POLLHUP; - int n = __poll (fds, 1, usectmo); - if (n == -1 && __builtin_expect (errno == EINTR, 0)) - { - /* Handle the case where the poll() call is interrupted by a - signal. We cannot just use TEMP_FAILURE_RETRY since it might - lead to infinite loops. */ - struct timeval now; - (void) __gettimeofday (&now, NULL); - long int end = now.tv_sec * 1000 + usectmo + (now.tv_usec + 500) / 1000; - long int timeout = usectmo; - while (1) - { - n = __poll (fds, 1, timeout); - if (n != -1 || errno != EINTR) - break; - - /* Recompute the timeout time. */ - (void) __gettimeofday (&now, NULL); - timeout = end - (now.tv_sec * 1000 + (now.tv_usec + 500) / 1000); - } - } - - return n; + return __poll_noeintr (fds, 1, usectmo); } diff --git a/sunrpc/clnt_udp.c b/sunrpc/clnt_udp.c index 6ffa5f2..f1e6500 100644 --- a/sunrpc/clnt_udp.c +++ b/sunrpc/clnt_udp.c @@ -378,9 +378,8 @@ send_again: anyup = 0; for (;;) { - switch (__poll (&fd, 1, milliseconds)) + switch (__poll_noeintr (&fd, 1, milliseconds)) { - case 0: if (anyup == 0) { @@ -407,8 +406,6 @@ send_again: * updated. */ case -1: - if (errno == EINTR) - continue; cu->cu_error.re_errno = errno; return (cu->cu_error.re_status = RPC_CANTRECV); } diff --git a/sunrpc/clnt_unix.c b/sunrpc/clnt_unix.c index 32d88b9..aff6fa5 100644 --- a/sunrpc/clnt_unix.c +++ b/sunrpc/clnt_unix.c @@ -551,22 +551,16 @@ readunix (char *ctptr, char *buf, int len) fd.fd = ct->ct_sock; fd.events = POLLIN; - while (TRUE) + switch (__poll_noeintr (&fd, 1, milliseconds)) { - switch (__poll (&fd, 1, milliseconds)) - { - case 0: - ct->ct_error.re_status = RPC_TIMEDOUT; - return -1; + case 0: + ct->ct_error.re_status = RPC_TIMEDOUT; + return -1; - case -1: - if (errno == EINTR) - continue; - ct->ct_error.re_status = RPC_CANTRECV; - ct->ct_error.re_errno = errno; - return -1; - } - break; + case -1: + ct->ct_error.re_status = RPC_CANTRECV; + ct->ct_error.re_errno = errno; + return -1; } switch (len = __msgread (ct->ct_sock, buf, len)) { diff --git a/sunrpc/rtime.c b/sunrpc/rtime.c index d224624..79d55d1 100644 --- a/sunrpc/rtime.c +++ b/sunrpc/rtime.c @@ -102,9 +102,7 @@ rtime (struct sockaddr_in *addrp, struct rpc_timeval *timep, milliseconds = (timeout->tv_sec * 1000) + (timeout->tv_usec / 1000); fd.fd = s; fd.events = POLLIN; - do - res = __poll (&fd, 1, milliseconds); - while (res < 0 && errno == EINTR); + res = __poll_noeintr (&fd, 1, milliseconds); if (res <= 0) { if (res == 0) diff --git a/sunrpc/svc_run.c b/sunrpc/svc_run.c index 90dfc94..5e20aae 100644 --- a/sunrpc/svc_run.c +++ b/sunrpc/svc_run.c @@ -83,11 +83,9 @@ svc_run (void) my_pollfd[i].revents = 0; } - switch (i = __poll (my_pollfd, max_pollfd, -1)) + switch (i = __poll_noeintr (my_pollfd, max_pollfd, -1)) { case -1: - if (errno == EINTR) - continue; perror (_("svc_run: - poll failed")); break; case 0: diff --git a/sunrpc/svc_tcp.c b/sunrpc/svc_tcp.c index 913f05f..92886f0 100644 --- a/sunrpc/svc_tcp.c +++ b/sunrpc/svc_tcp.c @@ -317,26 +317,19 @@ readtcp (char *xprtptr, char *buf, int len) int milliseconds = 35 * 1000; struct pollfd pollfd; - do + pollfd.fd = sock; + pollfd.events = POLLIN; + switch (__poll_noeintr (&pollfd, 1, milliseconds)) { - pollfd.fd = sock; - pollfd.events = POLLIN; - switch (__poll (&pollfd, 1, milliseconds)) - { - case -1: - if (errno == EINTR) - continue; - /*FALLTHROUGH*/ - case 0: - goto fatal_err; - default: - if ((pollfd.revents & POLLERR) || (pollfd.revents & POLLHUP) - || (pollfd.revents & POLLNVAL)) - goto fatal_err; - break; - } + case -1: + case 0: + goto fatal_err; + default: + if ((pollfd.revents & POLLERR) || (pollfd.revents & POLLHUP) + || (pollfd.revents & POLLNVAL)) + goto fatal_err; + break; } - while ((pollfd.revents & POLLIN) == 0); if ((len = __read (sock, buf, len)) > 0) return len; diff --git a/sunrpc/svc_unix.c b/sunrpc/svc_unix.c index 963276b2..6d93c5f 100644 --- a/sunrpc/svc_unix.c +++ b/sunrpc/svc_unix.c @@ -419,26 +419,19 @@ readunix (char *xprtptr, char *buf, int len) int milliseconds = 35 * 1000; struct pollfd pollfd; - do + pollfd.fd = sock; + pollfd.events = POLLIN; + switch (__poll_noeintr (&pollfd, 1, milliseconds)) { - pollfd.fd = sock; - pollfd.events = POLLIN; - switch (__poll (&pollfd, 1, milliseconds)) - { - case -1: - if (errno == EINTR) - continue; - /*FALLTHROUGH*/ - case 0: - goto fatal_err; - default: - if ((pollfd.revents & POLLERR) || (pollfd.revents & POLLHUP) - || (pollfd.revents & POLLNVAL)) - goto fatal_err; - break; - } + case -1: + case 0: + goto fatal_err; + default: + if ((pollfd.revents & POLLERR) || (pollfd.revents & POLLHUP) + || (pollfd.revents & POLLNVAL)) + goto fatal_err; + break; } - while ((pollfd.revents & POLLIN) == 0); if ((len = __msgread (sock, buf, len)) > 0) return len;