>From 9d003d5d6cc4d7ff165cf12cd554dc551a3d63bc Mon Sep 17 00:00:00 2001 From: Noel Power Date: Mon, 22 Feb 2016 19:18:10 +0000 Subject: [PATCH 2/2] Reorganise glib glue code use TEVENT_TRACE_[BEFORE|AFTER]_LOOP_ONCE events Each loop iteration we need to evaluate any available glib event sources, this patch modifies the previous code in the following ways a) simplify calling sequence, in normal loop processing no immediate events are used, the exception is when glib_loop quit is detected in the 'glib_prepare' phase. In that case we need the event loop to cycle to avoid blocking on a timeout. b) simplify the optimisation for 0 timeout handling, detect this scenario in the prepare phase and drain events there. This results in the 'glib_process' phase have also being simplified, previously it confusingly did a) the 0 timeout optimisation b) normal 'poll, check & dispatch' c) calling glib_prepare now 'glib_process' does just the normal 'poll, check & dispatch', nothing else c) every loop iteration results in glib_prepare/glib_process regardless of whether a tevent event fires --- source3/lib/tevent_glib_glue.c | 218 ++++++++++++++++++++++++++--------------- 1 file changed, 140 insertions(+), 78 deletions(-) diff --git a/source3/lib/tevent_glib_glue.c b/source3/lib/tevent_glib_glue.c index 3e2ed67..1c1fab9 100644 --- a/source3/lib/tevent_glib_glue.c +++ b/source3/lib/tevent_glib_glue.c @@ -45,6 +45,7 @@ struct tevent_glib_glue { struct tevent_context *ev; GMainContext *gmain_ctx; bool quit; + bool skip_glue_trace_event; struct tevent_timer *retry_timer; gint gtimeout; @@ -58,8 +59,10 @@ struct tevent_glib_glue { size_t num_maps; struct tevent_timer *timer; struct tevent_immediate *im; - bool scheduled_im; + bool draining_events; struct pollfd *pollfds; + tevent_trace_callback_t prev_trace_cb; + void *prev_trace_data; }; static bool tevent_glib_prepare(struct tevent_glib_glue *glue); @@ -69,6 +72,11 @@ static void tevent_glib_fd_handler(struct tevent_context *ev, struct tevent_fd *fde, uint16_t flags, void *private_data); +static void tevent_im_tickle_loop_handler(struct tevent_context *ev, + struct tevent_immediate *im, + void *private_data); +static void tevent_glib_glue_trace_callback(enum tevent_trace_point point, + void *private_data); typedef int (*gfds_cmp_cb)(const void *fd1, const void *fd2); typedef bool (*gfds_found_cb)(struct tevent_glib_glue *glue, @@ -270,8 +278,12 @@ static void tevent_glib_fd_handler(struct tevent_context *ev, private_data, struct tevent_glib_glue); tevent_glib_process(glue); - - return; + /* + * we have already called tevent_glib_process, we wish to inhibit + * the next TEVENT_TRACE_AFTER_LOOP_ONCE event from calling process + * a second time (out of order) + */ + glue->skip_glue_trace_event = true; } static void tevent_glib_timer_handler(struct tevent_context *ev, @@ -284,21 +296,118 @@ static void tevent_glib_timer_handler(struct tevent_context *ev, glue->timer = NULL; tevent_glib_process(glue); - + /* + * we have already called tevent_glib_process, we wish to inhibit + * the next TEVENT_TRACE_AFTER_LOOP_ONCE event from calling process + * a second time (out of order) + */ + glue->skip_glue_trace_event = true; return; } -static void tevent_glib_im_handler(struct tevent_context *ev, - struct tevent_immediate *im, - void *private_data) +static void tevent_glib_glue_trace_callback(enum tevent_trace_point point, + void *private_data) { struct tevent_glib_glue *glue = talloc_get_type_abort( private_data, struct tevent_glib_glue); + bool call_trace_handler; + /* + * don't call prepare/process if explicity told to skip or if + * we are still waiting to acquire the glib context + */ + call_trace_handler = (!glue->skip_glue_trace_event && !glue->retry_timer); + + switch (point) { + case TEVENT_TRACE_BEFORE_LOOP_ONCE: + case TEVENT_TRACE_AFTER_LOOP_ONCE: + if (call_trace_handler) { + if (point == TEVENT_TRACE_BEFORE_LOOP_ONCE) { + tevent_glib_prepare(glue); + } else { + tevent_glib_process(glue); + } + } + glue->skip_glue_trace_event = false; + break; + case TEVENT_TRACE_BEFORE_WAIT: + case TEVENT_TRACE_AFTER_WAIT: + default: + break; + } - glue->scheduled_im = false; - tevent_glib_process(glue); - return; + /* chain previous handler */ + if (glue->prev_trace_cb) { + glue->prev_trace_cb(point, glue->prev_trace_data); + } +} + +/* noop to force loop processing */ +static void tevent_im_tickle_loop_handler(struct tevent_context *ev, + struct tevent_immediate *im, + void *private_data) +{ + TALLOC_FREE(im); +} + +/* + * This function preforms an optimisation for the following case: + * + * If g_main_context_query() returns a timeout value of 0 this + * implies that there maybe more glib event sources ready. + * Here we loop through the prepare, check & dispatch glib phases while + * the returned timeout is 0, this avoids scheduling an immediate event and + * going through tevent_loop_once(). +*/ +static void drain_glib_events( struct tevent_glib_glue *glue) +{ + bool sources_ready, ok; + do { + sources_ready = g_main_context_check(glue->gmain_ctx, + glue->gpriority, + glue->gpollfds, + glue->num_gpollfds); + if (sources_ready) { + g_main_context_dispatch(glue->gmain_ctx); + } + + g_main_context_release(glue->gmain_ctx); + + if (glue->quit) { + /* Set via tevent_glib_glue_quit() */ + struct tevent_immediate *im = + tevent_create_immediate(NULL); + /* + * as we are called from glib_prepare if we fall out + * here more than likely there is a timeout that poll + * will block for, an immediate event will force the + * event loop to cycle and terminate if no more events. + */ + tevent_schedule_immediate(im, glue->ev, + tevent_im_tickle_loop_handler, glue); + DBG_DEBUG("quitting....\n"); + return; + } + + + /* + * Give other glib threads a chance to grab the context, + * tevent_glib_prepare() will then re-acquire it + */ + + ok = tevent_glib_prepare(glue); + if (!ok) { + samba_tevent_glib_glue_quit(glue); + return; + } + + if (glue->gtimeout != 0) { + break; + } + + } while (true); + + glue->draining_events = false; } static bool save_current_fdset(struct tevent_glib_glue *glue) @@ -382,22 +491,9 @@ static bool tevent_glib_update_events(struct tevent_glib_glue *glue) } TALLOC_FREE(glue->timer); - if ((glue->gtimeout == 0) && (!glue->scheduled_im)) { - /* - * Schedule an immediate event. We use a immediate event and not - * an immediate timer event, because the former can be reused. - * - * We may be called in a loop in tevent_glib_process() and only - * want to schedule this once, so we remember the fact. - * - * Doing this here means we occasionally schedule an unneeded - * immediate event, but it avoids leaking abstraction into upper - * layers. - */ - tevent_schedule_immediate(glue->im, glue->ev, - tevent_glib_im_handler, - glue); - glue->scheduled_im = true; + if ((glue->gtimeout == 0) && (!glue->draining_events)) { + glue->draining_events = true; + drain_glib_events(glue); } else if (glue->gtimeout > 0) { uint64_t microsec = glue->gtimeout * 1000; struct timeval tv = tevent_timeval_current_ofs(microsec / 1000000, @@ -432,7 +528,6 @@ static bool tevent_glib_prepare(struct tevent_glib_glue *glue) { bool ok; gboolean gok, source_ready; - gok = g_main_context_acquire(glue->gmain_ctx); if (!gok) { DBG_ERR("couldn't acquire g_main_context\n"); @@ -544,7 +639,7 @@ static bool tevent_glib_process(struct tevent_glib_glue *glue) { bool ok; int num_ready; - + bool sources_ready; ok = gpoll_to_poll_fds(glue); if (!ok) { DBG_ERR("gpoll_to_poll_fds failed\n"); @@ -563,62 +658,19 @@ static bool tevent_glib_process(struct tevent_glib_glue *glue) DBG_DEBUG("tevent_glib_process: num_ready: %d\n", num_ready); - do { - bool sources_ready; - - sources_ready = g_main_context_check(glue->gmain_ctx, - glue->gpriority, - glue->gpollfds, - glue->num_gpollfds); - if (!sources_ready) { - break; - } - + sources_ready = g_main_context_check(glue->gmain_ctx, + glue->gpriority, + glue->gpollfds, + glue->num_gpollfds); + if (sources_ready) { g_main_context_dispatch(glue->gmain_ctx); - - if (glue->quit) { - /* Set via tevent_glib_glue_quit() */ - g_main_context_release(glue->gmain_ctx); - return true; - } - - /* - * This is an optimisation for the following case: - * - * If g_main_context_query() returns a timeout value of 0 this - * implicates that there may be more glib event sources ready. - * This avoids sheduling an immediate event and going through - * tevent_loop_once(). - */ - if (glue->gtimeout != 0) { - break; - } - - /* - * Give other glib threads a chance to grab the context, - * tevent_glib_prepare() will then re-acquire it - */ - g_main_context_release(glue->gmain_ctx); - - ok = tevent_glib_prepare(glue); - if (!ok) { - samba_tevent_glib_glue_quit(glue); - return false; - } - } while (true); + } /* * Give other glib threads a chance to grab the context, * tevent_glib_prepare() will then re-acquire it */ g_main_context_release(glue->gmain_ctx); - - ok = tevent_glib_prepare(glue); - if (!ok) { - samba_tevent_glib_glue_quit(glue); - return false; - } - return true; } @@ -631,6 +683,7 @@ static void tevent_glib_glue_cleanup(struct tevent_glib_glue *glue) TALLOC_FREE(glue->fd_map[i].fd_event); } + tevent_set_trace_callback(glue->ev, glue->prev_trace_cb, glue->prev_trace_data); TALLOC_FREE(glue->fd_map); TALLOC_FREE(glue->gpollfds); TALLOC_FREE(glue->prev_gpollfds); @@ -668,12 +721,21 @@ struct tevent_glib_glue *samba_tevent_glib_glue_create(TALLOC_CTX *mem_ctx, glue->im = tevent_create_immediate(glue); + tevent_get_trace_callback(glue->ev, &glue->prev_trace_cb, &glue->prev_trace_data); + tevent_set_trace_callback(ev, tevent_glib_glue_trace_callback, + glue); + ok = tevent_glib_prepare(glue); if (!ok) { TALLOC_FREE(glue); return NULL; } - + /* + * we have already called tevent_glib_prepare, we wish to inhibit + * the next TEVENT_TRACE_BEFORE_LOOP_ONCE event from calling prepare + * a second time (out of order) + */ + glue->skip_glue_trace_event = true; return glue; } -- 2.1.4