From 66197ca25d13f2725ba6d741868557fade1cc8c8 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Mon, 2 Nov 2015 08:04:10 -0800 Subject: [PATCH] Fix nap condition for pollset wakeup If: - one thread issues a kick forcing pollset re-evaluation - concurrently with a second thread forcing a specific poller to be awoken And: - both threads kicks are processed as a single wakeup Then: - since we enqueue nothing to the exec_ctx in this situation, we responded to the wakeup by doing another poll until the timeout, ignoring urgent work up the stack Fix this by flagging that a specific worker was designated to be awoken (since this is a good signal that we really really need to wake up), and use that to still re-evaluate the poll set, but with an immediate deadline so that we fall out of the poll loop as soon as possible. --- src/core/iomgr/pollset_posix.c | 5 ++++- src/core/iomgr/pollset_posix.h | 1 + 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/core/iomgr/pollset_posix.c b/src/core/iomgr/pollset_posix.c index bce1ce97144..6f478ccacb9 100644 --- a/src/core/iomgr/pollset_posix.c +++ b/src/core/iomgr/pollset_posix.c @@ -121,12 +121,14 @@ void grpc_pollset_kick_ext(grpc_pollset *p, if ((flags & GRPC_POLLSET_REEVALUATE_POLLING_ON_WAKEUP) != 0) { specific_worker->reevaluate_polling_on_wakeup = 1; } + specific_worker->kicked_specifically = 1; grpc_wakeup_fd_wakeup(&specific_worker->wakeup_fd); } else if ((flags & GRPC_POLLSET_CAN_KICK_SELF) != 0) { GPR_TIMER_MARK("kick_yoself", 0); if ((flags & GRPC_POLLSET_REEVALUATE_POLLING_ON_WAKEUP) != 0) { specific_worker->reevaluate_polling_on_wakeup = 1; } + specific_worker->kicked_specifically = 1; grpc_wakeup_fd_wakeup(&specific_worker->wakeup_fd); } } else if (gpr_tls_get(&g_current_thread_poller) != (gpr_intptr)p) { @@ -242,6 +244,7 @@ void grpc_pollset_work(grpc_exec_ctx *exec_ctx, grpc_pollset *pollset, /* this must happen before we (potentially) drop pollset->mu */ worker->next = worker->prev = NULL; worker->reevaluate_polling_on_wakeup = 0; + worker->kicked_specifically = 0; /* TODO(ctiller): pool these */ grpc_wakeup_fd_init(&worker->wakeup_fd); /* If there's work waiting for the pollset to be idle, and the @@ -308,7 +311,7 @@ void grpc_pollset_work(grpc_exec_ctx *exec_ctx, grpc_pollset *pollset, if (worker->reevaluate_polling_on_wakeup) { worker->reevaluate_polling_on_wakeup = 0; pollset->kicked_without_pollers = 0; - if (queued_work) { + if (queued_work || worker->kicked_specifically) { /* If there's queued work on the list, then set the deadline to be immediate so we get back out of the polling loop quickly */ deadline = gpr_inf_past(GPR_CLOCK_MONOTONIC); diff --git a/src/core/iomgr/pollset_posix.h b/src/core/iomgr/pollset_posix.h index 34f76db2afa..95ebeab1c26 100644 --- a/src/core/iomgr/pollset_posix.h +++ b/src/core/iomgr/pollset_posix.h @@ -51,6 +51,7 @@ struct grpc_fd; typedef struct grpc_pollset_worker { grpc_wakeup_fd wakeup_fd; int reevaluate_polling_on_wakeup; + int kicked_specifically; struct grpc_pollset_worker *next; struct grpc_pollset_worker *prev; } grpc_pollset_worker;