diff options
Diffstat (limited to 'libglusterfs/src/timer.c')
| -rw-r--r-- | libglusterfs/src/timer.c | 149 |
1 files changed, 67 insertions, 82 deletions
diff --git a/libglusterfs/src/timer.c b/libglusterfs/src/timer.c index 88a28a9bd16..66c861b04cd 100644 --- a/libglusterfs/src/timer.c +++ b/libglusterfs/src/timer.c @@ -8,12 +8,12 @@ cases as published by the Free Software Foundation. */ -#include "timer.h" -#include "logging.h" -#include "common-utils.h" -#include "globals.h" -#include "timespec.h" -#include "libglusterfs-messages.h" +#include "glusterfs/timer.h" +#include "glusterfs/logging.h" +#include "glusterfs/common-utils.h" +#include "glusterfs/globals.h" +#include "glusterfs/timespec.h" +#include "glusterfs/libglusterfs-messages.h" /* fwd decl */ static gf_timer_registry_t * @@ -53,7 +53,7 @@ gf_timer_call_after(glusterfs_ctx_t *ctx, struct timespec delta, event->callbk = callbk; event->data = data; event->xl = THIS; - LOCK(®->lock); + pthread_mutex_lock(®->lock); { list_for_each_entry_reverse(trav, ®->active, list) { @@ -61,8 +61,11 @@ gf_timer_call_after(glusterfs_ctx_t *ctx, struct timespec delta, break; } list_add(&event->list, &trav->list); + if (&trav->list == ®->active) { + pthread_cond_signal(®->cond); + } } - UNLOCK(®->lock); + pthread_mutex_unlock(®->lock); return event; } @@ -75,13 +78,13 @@ gf_timer_call_cancel(glusterfs_ctx_t *ctx, gf_timer_t *event) if (ctx == NULL || event == NULL) { gf_msg_callingfn("timer", GF_LOG_ERROR, EINVAL, LG_MSG_INVALID_ARG, "invalid argument"); - return 0; + return -1; } if (ctx->cleanup_started) { gf_msg_callingfn("timer", GF_LOG_INFO, 0, LG_MSG_CTX_CLEANUP_STARTED, "ctx cleanup started"); - return 0; + return -1; } LOCK(&ctx->lock); @@ -93,13 +96,12 @@ gf_timer_call_cancel(glusterfs_ctx_t *ctx, gf_timer_t *event) if (!reg) { /* This can happen when cleanup may have just started and * gf_timer_registry_destroy() sets ctx->timer to NULL. - * Just bail out as success as gf_timer_proc() takes - * care of cleaning up the events. + * gf_timer_proc() takes care of cleaning up the events. */ - return 0; + return -1; } - LOCK(®->lock); + pthread_mutex_lock(®->lock); { fired = event->fired; if (fired) @@ -107,7 +109,7 @@ gf_timer_call_cancel(glusterfs_ctx_t *ctx, gf_timer_t *event) list_del(&event->list); } unlock: - UNLOCK(®->lock); + pthread_mutex_unlock(®->lock); if (!fired) { GF_FREE(event); @@ -120,64 +122,29 @@ static void * gf_timer_proc(void *data) { gf_timer_registry_t *reg = data; - struct timespec sleepts; gf_timer_t *event = NULL; gf_timer_t *tmp = NULL; xlator_t *old_THIS = NULL; + pthread_mutex_lock(®->lock); + while (!reg->fin) { - uint64_t now; - struct timespec now_ts; - - timespec_now(&now_ts); - now = TS(now_ts); - while (1) { - uint64_t at; - char need_cbk = 0; - - /* - * This will be overridden with a shorter interval if - * there's an event scheduled sooner. That makes the - * system more responsive in most cases, but doesn't - * include the case where a timer is added while we're - * asleep. It's tempting to use pthread_cond_timedwait, - * with the caveat that we'd be relying on system time - * instead of monotonic time. That's a mess when the - * system time is adjusted. Another alternative might - * be to use pthread_kill, but that will remain TBD for - * now. - */ - sleepts.tv_sec = 1; - sleepts.tv_nsec = 0; - - LOCK(®->lock); - { - /* - * Using list_for_each and then always breaking - * after the first iteration might seem strange, - * but (unlike alternatives) is independent of - * the underlying list implementation. - */ - list_for_each_entry_safe(event, tmp, ®->active, list) - { - at = TS(event->at); - if (now >= at) { - need_cbk = 1; - event->fired = _gf_true; - list_del(&event->list); - } else { - uint64_t diff = now - at; - - if (diff < 1000000000) { - sleepts.tv_sec = 0; - sleepts.tv_nsec = diff; - } - } - break; - } - } - UNLOCK(®->lock); - if (need_cbk) { + if (list_empty(®->active)) { + pthread_cond_wait(®->cond, ®->lock); + } else { + struct timespec now; + + timespec_now(&now); + event = list_first_entry(®->active, gf_timer_t, list); + if (TS(now) < TS(event->at)) { + now = event->at; + pthread_cond_timedwait(®->cond, ®->lock, &now); + } else { + event->fired = _gf_true; + list_del_init(&event->list); + + pthread_mutex_unlock(®->lock); + old_THIS = NULL; if (event->xl) { old_THIS = THIS; @@ -188,26 +155,29 @@ gf_timer_proc(void *data) if (old_THIS) { THIS = old_THIS; } - } else { - break; + + pthread_mutex_lock(®->lock); } } - nanosleep(&sleepts, NULL); } - LOCK(®->lock); + /* Do not call gf_timer_call_cancel(), + * it will lead to deadlock + */ + list_for_each_entry_safe(event, tmp, ®->active, list) { - /* Do not call gf_timer_call_cancel(), - * it will lead to deadlock + list_del(&event->list); + /* TODO Possible resource leak + * Before freeing the event, we need to call the respective + * event functions and free any resources. + * For example, In case of rpc_clnt_reconnect, we need to + * unref rpc object which was taken when added to timer + * wheel. */ - list_for_each_entry_safe(event, tmp, ®->active, list) - { - list_del(&event->list); - GF_FREE(event); - } + GF_FREE(event); } - UNLOCK(®->lock); - LOCK_DESTROY(®->lock); + + pthread_mutex_unlock(®->lock); return NULL; } @@ -217,6 +187,7 @@ gf_timer_registry_init(glusterfs_ctx_t *ctx) { gf_timer_registry_t *reg = NULL; int ret = -1; + pthread_condattr_t attr; LOCK(&ctx->lock); { @@ -231,7 +202,10 @@ gf_timer_registry_init(glusterfs_ctx_t *ctx) goto out; } ctx->timer = reg; - LOCK_INIT(®->lock); + pthread_mutex_init(®->lock, NULL); + pthread_condattr_init(&attr); + pthread_condattr_setclock(&attr, CLOCK_MONOTONIC); + pthread_cond_init(®->cond, &attr); INIT_LIST_HEAD(®->active); } UNLOCK(&ctx->lock); @@ -265,7 +239,18 @@ gf_timer_registry_destroy(glusterfs_ctx_t *ctx) return; thr_id = reg->th; + + pthread_mutex_lock(®->lock); + reg->fin = 1; + pthread_cond_signal(®->cond); + + pthread_mutex_unlock(®->lock); + pthread_join(thr_id, NULL); + + pthread_cond_destroy(®->cond); + pthread_mutex_destroy(®->lock); + GF_FREE(reg); } |