From ad98edd00accdf5529da77eb79b72cf9fc025a7d Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Thu, 12 Sep 2024 17:47:13 +0200 Subject: [PATCH] BUG/MINOR: polling: fix time reporting when using busy polling Since commit beb859abce ("MINOR: polling: add an option to support busy polling") the time and status passed to clock_update_local_date() were incorrect. Indeed, what is considered is the before_poll date related to the configured timeout which does not correspond to what is passed to the poller. That's not correct because before_poll+the syscall's timeout will be crossed by the current date 100 ms after the start of the poller. In practice it didn't happen when the poller was limited to 1s timeout but at one minute it happens all the time. That's particularly visible when running a multi-threaded setup with busy polling and only half of the threads working (bind ... thread even). In this case, the fixup code of clock_update_local_date() is executed for each round of busy polling. The issue was made really visible starting with recent commit e8b1ad4c2b ("BUG/MEDIUM: clock: also update the date offset on time jumps") because upon a jump, the shared offset is reset, while it should not be in this specific case. What needs to be done instead is to pass the configured timeout of the poller (and not of the syscall), and always pass "interrupted" set so as to claim we got an event (which is sort of true as it just means the poller returned instantly). In this case we can still detect backwards/forward jumps and will use a correct boundary for the maximum date that covers the whole loop. This can be backported to all versions since the issue was introduced with busy-polling in 1.9-dev8. --- src/ev_epoll.c | 2 +- src/ev_evports.c | 2 +- src/ev_kqueue.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ev_epoll.c b/src/ev_epoll.c index 352620d0a3db1..9e7050c730821 100644 --- a/src/ev_epoll.c +++ b/src/ev_epoll.c @@ -230,7 +230,7 @@ static void _do_poll(struct poller *p, int exp, int wake) int timeout = (global.tune.options & GTUNE_BUSY_POLLING) ? 0 : wait_time; status = epoll_wait(epoll_fd[tid], epoll_events, global.tune.maxpollevents, timeout); - clock_update_local_date(timeout, status); + clock_update_local_date(wait_time, (global.tune.options & GTUNE_BUSY_POLLING) ? 1 : status); if (status) { activity[tid].poll_io++; diff --git a/src/ev_evports.c b/src/ev_evports.c index ee357bc1f4d2c..da2c11060d397 100644 --- a/src/ev_evports.c +++ b/src/ev_evports.c @@ -225,7 +225,7 @@ static void _do_poll(struct poller *p, int exp, int wake) break; } } - clock_update_local_date(timeout, nevlist); + clock_update_local_date(wait_time, (global.tune.options & GTUNE_BUSY_POLLING) ? 1 : nevlist); if (nevlist || interrupted) break; diff --git a/src/ev_kqueue.c b/src/ev_kqueue.c index f123e7be25320..271e95b672c0e 100644 --- a/src/ev_kqueue.c +++ b/src/ev_kqueue.c @@ -183,7 +183,7 @@ static void _do_poll(struct poller *p, int exp, int wake) kev, // struct kevent *eventlist fd, // int nevents &timeout_ts); // const struct timespec *timeout - clock_update_local_date(timeout, status); + clock_update_local_date(wait_time, (global.tune.options & GTUNE_BUSY_POLLING) ? 1 : status); if (status) { activity[tid].poll_io++;