This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: BZ#15819: introduce internal function to ease poll retry with timeout
- From: Alexandre Oliva <aoliva at redhat dot com>
- To: Roland McGrath <roland at hack dot frob dot com>
- Cc: libc-alpha at sourceware dot org
- Date: Fri, 14 Nov 2014 00:17:52 -0200
- Subject: Re: BZ#15819: introduce internal function to ease poll retry with timeout
- Authentication-results: sourceware.org; auth=none
- References: <orioiiahsf dot fsf at free dot home> <20141113221831 dot D15A62C3B18 at topped-with-meat dot com>
On Nov 13, 2014, Roland McGrath <roland@hack.frob.com> wrote:
> prefer new local headers
*check*
> The __ treatment is never necessary for local-scope names in internal
> source files (including headers). It only matters for global names, or
> uses in installed headers.
I figured names outside the reserved namespace could be used in our
tests, and then they'd interfere. I guess the reasoning is that, since
we control the tests too, we can just fix any such conflicts in the
tests, rigth?
> Drop the unnecessary prefixes to make the code easier to read.
*check*
This revised patch fixes both of these issues, and it poisons __poll and
poll at the end of include/poll-noeintr.h.
Retested on x86_64-linux-gnu. Ok to install?
for ChangeLog
[BZ #15819]
* NEWS: Update.
* nis/nis_callback.c (internal_nis_do_callback): Call
__poll_noeintr, drop TEMP_FAILURE_RETRY.
* nscd/nscd_helper.c (wait_on_socket): Call __poll_noeintr,
drop loop and gettimeofday timeout recomputation.
* sunrpc/clnt_udp.c (clntudp_call): Call __poll_noeintr.
* 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.
* include/poll-noeintr.h: New header, included instead of
sys/poll.h in all of the above.
---
NEWS | 8 ++--
include/poll-noeintr.h | 86 ++++++++++++++++++++++++++++++++++++++++++++++++
nis/nis_callback.c | 5 +--
nscd/nscd_helper.c | 26 +--------------
sunrpc/clnt_udp.c | 7 +---
sunrpc/clnt_unix.c | 24 +++++--------
sunrpc/rtime.c | 6 +--
sunrpc/svc_run.c | 6 +--
sunrpc/svc_tcp.c | 31 +++++++----------
sunrpc/svc_unix.c | 31 +++++++----------
10 files changed, 133 insertions(+), 97 deletions(-)
create mode 100644 include/poll-noeintr.h
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/poll-noeintr.h b/include/poll-noeintr.h
new file mode 100644
index 0000000..d94deb4
--- /dev/null
+++ b/include/poll-noeintr.h
@@ -0,0 +1,86 @@
+/* Wrapper for __poll that restarts on EINTR
+ Copyright (C) 2014 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#ifndef _POLL_EINTR_H
+#define _POLL_EINTR_H
+
+#include <sys/poll.h>
+#include <errno.h>
+#include <time.h>
+
+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;
+}
+
+#pragma poison __poll
+#pragma poison poll
+
+#endif /* _POLL_EINTR_H */
diff --git a/nis/nis_callback.c b/nis/nis_callback.c
index e352733..b7492fb 100644
--- a/nis/nis_callback.c
+++ b/nis/nis_callback.c
@@ -26,7 +26,7 @@
#include <string.h>
#include <memory.h>
#include <syslog.h>
-#include <sys/poll.h>
+#include <poll-noeintr.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <arpa/inet.h>
@@ -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..301be42 100644
--- a/nscd/nscd_helper.c
+++ b/nscd/nscd_helper.c
@@ -27,7 +27,7 @@
#include <unistd.h>
#include <stdint.h>
#include <sys/mman.h>
-#include <sys/poll.h>
+#include <poll-noeintr.h>
#include <sys/socket.h>
#include <sys/stat.h>
#include <sys/time.h>
@@ -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..38b11e1 100644
--- a/sunrpc/clnt_udp.c
+++ b/sunrpc/clnt_udp.c
@@ -37,7 +37,7 @@
#include <rpc/rpc.h>
#include <rpc/xdr.h>
#include <rpc/clnt.h>
-#include <sys/poll.h>
+#include <poll-noeintr.h>
#include <sys/socket.h>
#include <sys/ioctl.h>
#include <netdb.h>
@@ -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..40eb269 100644
--- a/sunrpc/clnt_unix.c
+++ b/sunrpc/clnt_unix.c
@@ -51,7 +51,7 @@
#include <libintl.h>
#include <rpc/rpc.h>
#include <sys/uio.h>
-#include <sys/poll.h>
+#include <poll-noeintr.h>
#include <sys/socket.h>
#include <rpc/pmap_clnt.h>
#include <wchar.h>
@@ -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..862493b 100644
--- a/sunrpc/rtime.c
+++ b/sunrpc/rtime.c
@@ -43,7 +43,7 @@
#include <rpc/rpc.h>
#include <rpc/clnt.h>
#include <sys/types.h>
-#include <sys/poll.h>
+#include <poll-noeintr.h>
#include <sys/socket.h>
#include <sys/time.h>
#include <rpc/auth_des.h>
@@ -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..1f54d84 100644
--- a/sunrpc/svc_run.c
+++ b/sunrpc/svc_run.c
@@ -34,7 +34,7 @@
#include <errno.h>
#include <unistd.h>
#include <libintl.h>
-#include <sys/poll.h>
+#include <poll-noeintr.h>
#include <rpc/rpc.h>
/* This function can be used as a signal handler to terminate the
@@ -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..2ab98dc 100644
--- a/sunrpc/svc_tcp.c
+++ b/sunrpc/svc_tcp.c
@@ -58,7 +58,7 @@
#include <libintl.h>
#include <rpc/rpc.h>
#include <sys/socket.h>
-#include <sys/poll.h>
+#include <poll-noeintr.h>
#include <errno.h>
#include <stdlib.h>
@@ -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..cbc9891 100644
--- a/sunrpc/svc_unix.c
+++ b/sunrpc/svc_unix.c
@@ -59,7 +59,7 @@
#include <rpc/svc.h>
#include <sys/socket.h>
#include <sys/uio.h>
-#include <sys/poll.h>
+#include <poll-noeintr.h>
#include <errno.h>
#include <stdlib.h>
#include <libintl.h>
@@ -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;
--
Alexandre Oliva, freedom fighter http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/ FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer