This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] sunrpc/clnt_udp.c: always respect timeout(s)
- From: Florian Weimer <fweimer at redhat dot com>
- To: Pavel Raiskup <praiskup at redhat dot com>
- Cc: libc-alpha at sourceware dot org
- Date: Wed, 15 Jun 2016 10:00:34 +0200
- Subject: Re: [PATCH] sunrpc/clnt_udp.c: always respect timeout(s)
- Authentication-results: sourceware.org; auth=none
- References: <1464945979-10503-1-git-send-email-praiskup at redhat dot com>
On 06/03/2016 11:26 AM, Pavel Raiskup wrote:
Previously, when the port we were listening on was really busy,
clntudp_call () ended up processing the UDP queue without properly
defined time-limit (clntudp_call could keep processing forever
without returning back to caller).
The fix incorporates using of clock_gettime () to measure the real
timeout, instead of simply counting the number of poll()'s. We
should not face some huge slowdown because we call clock_gettime
only twice in most cases *and* never in busy loop.
Thanks.
I field a bug to track this:
https://sourceware.org/bugzilla/show_bug.cgi?id=20257
The commit message and ChangeLog need to reference this (as “[BZ #2057]”).
diff --git a/sunrpc/clnt_udp.c b/sunrpc/clnt_udp.c
index 6ffa5f2..2cab4c2 100644
--- a/sunrpc/clnt_udp.c
+++ b/sunrpc/clnt_udp.c
@@ -1,6 +1,8 @@
/*
* clnt_udp.c, Implements a UDP/IP based, client side RPC.
*
+ * Copyright (c) 2016, Free Software Foundation, Inc.
+ *
* Copyright (c) 2010, Oracle America, Inc.
Please do not change the copyright notice in this way. Either add the
full LGPL header, or leave it unchanged.
+static bool_t
+clntudp_call_timeouted (const struct timespec *start, int timeout, int *resend_timeout)
+{
+ struct timespec now;
+ long remains, waited;
+
+ __clock_gettime (CLOCK_MONOTONIC, &now);
I was mistaken about the availability of CLOCK_MONOTONIC. It seems to
be missing on Hurd. CLOCK_REALTIME is probably missing there as well,
so we need fallback to gettimeofday.
We have similar code in resolv/res_send.c. It needs to use
CLOCK_MONOTONIC if available, but currently does not. So this should
really move to generic libc functionality (for separate testing, too).
I can take care of this if you want. Unless you want to learn a bit
about glibc symbol management.
+ /* count everything in milliseconds */
+ waited = (now.tv_sec - start->tv_sec - 1) * 1000
+ + (now.tv_nsec - start->tv_nsec + 1000000000) / 1000000;
+ remains = timeout - waited;
+ if (*resend_timeout > remains)
+ *resend_timeout = remains;
+ return (*resend_timeout <= 0);
+}
This should perform a bit of overflow checking, or use unsigned long
long at least. It probably won't matter in practice because 2**31
milliseconds is a long time, but let's cover this properly.
From an interface perspective, it seems preferable to compute an
explicit deadline, and count the milliseconds towards that, rather than
recomputing the absolute deadline from the millisecond value.
Thanks,
Florian