On Fri, Sep 10, 2021 at 01:41:17PM +0200, [email protected] wrote:
> From: Martin Wilck <[email protected]>
>
> Our ppoll() call needs to wake up when a client request times out.
> This logic can be added by determining the first client that's about
> to time out. The logic in handle_client() will then cause a timeout
> reply to be sent to the client. This is more client-friendly
> as the client timing out without receiving a reply.
>
> Signed-off-by: Martin Wilck <[email protected]>
> ---
> multipathd/uxlsnr.c | 58 +++++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 53 insertions(+), 5 deletions(-)
>
> diff --git a/multipathd/uxlsnr.c b/multipathd/uxlsnr.c
> index 4637954..1bf4126 100644
> --- a/multipathd/uxlsnr.c
> +++ b/multipathd/uxlsnr.c
> @@ -320,6 +320,35 @@ static void handle_inotify(int fd, struct
> watch_descriptors *wds)
> }
>
> static const struct timespec ts_zero = { .tv_sec = 0, };
> +static const struct timespec ts_max = { .tv_sec = LONG_MAX, .tv_nsec =
> 999999999 };
> +
> +/* call with clients lock held */
> +static struct timespec *__get_soonest_timeout(struct timespec *ts)
> +{
> + struct timespec ts_min = ts_max, now;
> + bool any = false;
> + struct client *c;
> +
> + list_for_each_entry(c, &clients, node) {
> + if (timespeccmp(&c->expires, &ts_zero) != 0 &&
> + timespeccmp(&c->expires, &ts_min) < 0) {
> + ts_min = c->expires;
> + any = true;
> + }
> + }
> +
> + if (!any)
> + return NULL;
> +
> + get_monotonic_time(&now);
> + timespecsub(&ts_min, &now, ts);
> + if (timespeccmp(ts, &ts_zero) < 0)
> + *ts = ts_zero;
> +
> + condlog(4, "%s: next client expires in %ld.%03lds", __func__,
> + (long)ts->tv_sec, ts->tv_nsec / 1000000);
> + return ts;
> +}
>
> /* call with clients lock held */
> static bool __need_lock(void)
> @@ -422,6 +451,24 @@ static void set_client_state(struct client *c, int state)
> c->state = state;
> }
>
> +static void check_timeout(struct client *c)
> +{
> + struct timespec now;
> +
> + if (timespeccmp(&c->expires, &ts_zero) == 0)
> + return;
> +
> + get_monotonic_time(&now);
> + if (timespeccmp(&c->expires, &now) > 0)
> + return;
> +
> + condlog(2, "%s: cli[%d]: timed out at %ld.%03ld", __func__,
> + c->fd, (long)c->expires.tv_sec, c->expires.tv_nsec / 1000000);
> +
> + c->error = -ETIMEDOUT;
> + set_client_state(c, CLT_SEND);
> +}
> +
> static void handle_client(struct client *c, struct vectors *vecs, short
> revents)
> {
> ssize_t n;
> @@ -435,6 +482,8 @@ static void handle_client(struct client *c, struct
> vectors *vecs, short revents)
> return;
> }
>
> + check_timeout(c);
> +
> condlog(4, "%s: cli[%d] poll=%x state=%d cmd=\"%s\" repl \"%s\"",
> __func__,
> c->fd, revents, c->state, c->cmd, get_strbuf_str(&c->reply));
>
> @@ -594,6 +643,7 @@ void *uxsock_listen(long ux_sock, void *trigger_data)
> while (1) {
> struct client *c, *tmp;
> int i, n_pfds, poll_count, num_clients;
> + struct timespec __timeout, *timeout;
Maybe it's just too late to be looking at code, but I'm missing why we
need a separate variable that it a pointer to __timeout.
-Ben
>
> /* setup for a poll */
> pthread_mutex_lock(&client_lock);
> @@ -661,10 +711,12 @@ void *uxsock_listen(long ux_sock, void *trigger_data)
> break;
> }
> n_pfds = i;
> + timeout = __get_soonest_timeout(&__timeout);
> +
> pthread_cleanup_pop(1);
>
> /* most of our life is spent in this call */
> - poll_count = ppoll(polls, n_pfds, NULL, &mask);
> + poll_count = ppoll(polls, n_pfds, timeout, &mask);
>
> handle_signals(false);
> if (poll_count == -1) {
> @@ -679,10 +731,6 @@ void *uxsock_listen(long ux_sock, void *trigger_data)
> break;
> }
>
> - if (poll_count == 0) {
> - handle_signals(true);
> - continue;
> - }
> if (polls[POLLFD_IDLE].fd != -1 &&
> polls[POLLFD_IDLE].revents & POLLIN)
> drain_idle_fd(idle_fd);
> --
> 2.33.0
--
dm-devel mailing list
[email protected]
https://listman.redhat.com/mailman/listinfo/dm-devel
