Skip to content
Snippets Groups Projects
Commit 66197ca2 authored by Craig Tiller's avatar Craig Tiller
Browse files

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.
parent 89ea0c78
No related branches found
No related tags found
No related merge requests found
...@@ -121,12 +121,14 @@ void grpc_pollset_kick_ext(grpc_pollset *p, ...@@ -121,12 +121,14 @@ void grpc_pollset_kick_ext(grpc_pollset *p,
if ((flags & GRPC_POLLSET_REEVALUATE_POLLING_ON_WAKEUP) != 0) { if ((flags & GRPC_POLLSET_REEVALUATE_POLLING_ON_WAKEUP) != 0) {
specific_worker->reevaluate_polling_on_wakeup = 1; specific_worker->reevaluate_polling_on_wakeup = 1;
} }
specific_worker->kicked_specifically = 1;
grpc_wakeup_fd_wakeup(&specific_worker->wakeup_fd); grpc_wakeup_fd_wakeup(&specific_worker->wakeup_fd);
} else if ((flags & GRPC_POLLSET_CAN_KICK_SELF) != 0) { } else if ((flags & GRPC_POLLSET_CAN_KICK_SELF) != 0) {
GPR_TIMER_MARK("kick_yoself", 0); GPR_TIMER_MARK("kick_yoself", 0);
if ((flags & GRPC_POLLSET_REEVALUATE_POLLING_ON_WAKEUP) != 0) { if ((flags & GRPC_POLLSET_REEVALUATE_POLLING_ON_WAKEUP) != 0) {
specific_worker->reevaluate_polling_on_wakeup = 1; specific_worker->reevaluate_polling_on_wakeup = 1;
} }
specific_worker->kicked_specifically = 1;
grpc_wakeup_fd_wakeup(&specific_worker->wakeup_fd); grpc_wakeup_fd_wakeup(&specific_worker->wakeup_fd);
} }
} else if (gpr_tls_get(&g_current_thread_poller) != (gpr_intptr)p) { } 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, ...@@ -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 */ /* this must happen before we (potentially) drop pollset->mu */
worker->next = worker->prev = NULL; worker->next = worker->prev = NULL;
worker->reevaluate_polling_on_wakeup = 0; worker->reevaluate_polling_on_wakeup = 0;
worker->kicked_specifically = 0;
/* TODO(ctiller): pool these */ /* TODO(ctiller): pool these */
grpc_wakeup_fd_init(&worker->wakeup_fd); grpc_wakeup_fd_init(&worker->wakeup_fd);
/* If there's work waiting for the pollset to be idle, and the /* 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, ...@@ -308,7 +311,7 @@ void grpc_pollset_work(grpc_exec_ctx *exec_ctx, grpc_pollset *pollset,
if (worker->reevaluate_polling_on_wakeup) { if (worker->reevaluate_polling_on_wakeup) {
worker->reevaluate_polling_on_wakeup = 0; worker->reevaluate_polling_on_wakeup = 0;
pollset->kicked_without_pollers = 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 /* 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 */ immediate so we get back out of the polling loop quickly */
deadline = gpr_inf_past(GPR_CLOCK_MONOTONIC); deadline = gpr_inf_past(GPR_CLOCK_MONOTONIC);
......
...@@ -51,6 +51,7 @@ struct grpc_fd; ...@@ -51,6 +51,7 @@ struct grpc_fd;
typedef struct grpc_pollset_worker { typedef struct grpc_pollset_worker {
grpc_wakeup_fd wakeup_fd; grpc_wakeup_fd wakeup_fd;
int reevaluate_polling_on_wakeup; int reevaluate_polling_on_wakeup;
int kicked_specifically;
struct grpc_pollset_worker *next; struct grpc_pollset_worker *next;
struct grpc_pollset_worker *prev; struct grpc_pollset_worker *prev;
} grpc_pollset_worker; } grpc_pollset_worker;
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment