1 files changed, 204 insertions, 212 deletions

diff --git a/libglusterfs/src/timer.c b/libglusterfs/src/timer.c
index 0d845cdeb38..66c861b04cd 100644
--- a/libglusterfs/src/timer.c
+++ b/libglusterfs/src/timer.c
@@ -8,257 +8,249 @@
   cases as published by the Free Software Foundation.
 */
 
-#ifndef _CONFIG_H
-#define _CONFIG_H
-#include "config.h"
-#endif
+#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"
 
-#include "timer.h"
-#include "logging.h"
-#include "common-utils.h"
-#include "globals.h"
-#include "timespec.h"
+/* fwd decl */
+static gf_timer_registry_t *
+gf_timer_registry_init(glusterfs_ctx_t *);
 
 gf_timer_t *
-gf_timer_call_after (glusterfs_ctx_t *ctx,
-                     struct timespec delta,
-                     gf_timer_cbk_t callbk,
-                     void *data)
+gf_timer_call_after(glusterfs_ctx_t *ctx, struct timespec delta,
+                    gf_timer_cbk_t callbk, void *data)
 {
-        gf_timer_registry_t *reg = NULL;
-        gf_timer_t *event = NULL;
-        gf_timer_t *trav = NULL;
-        uint64_t at = 0;
-
-        if (ctx == NULL)
-        {
-                gf_log_callingfn ("timer", GF_LOG_ERROR, "invalid argument");
-                return NULL;
-        }
-
-        /* ctx and its fields are not accessed inside mutex!?
-         * TODO: Even with this there is a possiblity of race
-         * when cleanup_started is set after checking for it
-         */
-        if (ctx->cleanup_started) {
-                gf_log_callingfn ("timer", GF_LOG_INFO, "ctx cleanup started");
-                return NULL;
-        }
+    gf_timer_registry_t *reg = NULL;
+    gf_timer_t *event = NULL;
+    gf_timer_t *trav = NULL;
+    uint64_t at = 0;
+
+    if ((ctx == NULL) || (ctx->cleanup_started)) {
+        gf_msg_callingfn("timer", GF_LOG_ERROR, EINVAL, LG_MSG_INVALID_ARG,
+                         "Either ctx is NULL or"
+                         " ctx cleanup started");
+        return NULL;
+    }
 
-        reg = gf_timer_registry_init (ctx);
+    reg = gf_timer_registry_init(ctx);
 
-        if (!reg) {
-                gf_log_callingfn ("timer", GF_LOG_ERROR, "!reg");
-                return NULL;
-        }
+    if (!reg) {
+        gf_msg_callingfn("timer", GF_LOG_ERROR, 0, LG_MSG_TIMER_REGISTER_ERROR,
+                         "!reg");
+        return NULL;
+    }
 
-        event = GF_CALLOC (1, sizeof (*event), gf_common_mt_gf_timer_t);
-        if (!event) {
-                return NULL;
-        }
-        timespec_now (&event->at);
-        timespec_adjust_delta (&event->at, delta);
-        at = TS (event->at);
-        event->callbk = callbk;
-        event->data = data;
-        event->xl = THIS;
-        pthread_mutex_lock (&reg->lock);
+    event = GF_CALLOC(1, sizeof(*event), gf_common_mt_gf_timer_t);
+    if (!event) {
+        return NULL;
+    }
+    timespec_now(&event->at);
+    timespec_adjust_delta(&event->at, delta);
+    at = TS(event->at);
+    event->callbk = callbk;
+    event->data = data;
+    event->xl = THIS;
+    pthread_mutex_lock(&reg->lock);
+    {
+        list_for_each_entry_reverse(trav, &reg->active, list)
         {
-                trav = reg->active.prev;
-                while (trav != &reg->active) {
-                        if (TS (trav->at) < at)
-                                break;
-                        trav = trav->prev;
-                }
-                event->prev = trav;
-                event->next = event->prev->next;
-                event->prev->next = event;
-                event->next->prev = event;
+            if (TS(trav->at) < at)
+                break;
+        }
+        list_add(&event->list, &trav->list);
+        if (&trav->list == &reg->active) {
+            pthread_cond_signal(&reg->cond);
         }
-        pthread_mutex_unlock (&reg->lock);
-        return event;
+    }
+    pthread_mutex_unlock(&reg->lock);
+    return event;
 }
 
 int32_t
-gf_timer_call_stale (gf_timer_registry_t *reg,
-                     gf_timer_t *event)
+gf_timer_call_cancel(glusterfs_ctx_t *ctx, gf_timer_t *event)
 {
-        if (reg == NULL || event == NULL)
-        {
-                gf_log_callingfn ("timer", GF_LOG_ERROR, "invalid argument");
-                return 0;
-        }
-
-        event->next->prev = event->prev;
-        event->prev->next = event->next;
-        event->next = &reg->stale;
-        event->prev = event->next->prev;
-        event->next->prev = event;
-        event->prev->next = event;
+    gf_timer_registry_t *reg = NULL;
+    gf_boolean_t fired = _gf_false;
+
+    if (ctx == NULL || event == NULL) {
+        gf_msg_callingfn("timer", GF_LOG_ERROR, EINVAL, LG_MSG_INVALID_ARG,
+                         "invalid argument");
+        return -1;
+    }
+
+    if (ctx->cleanup_started) {
+        gf_msg_callingfn("timer", GF_LOG_INFO, 0, LG_MSG_CTX_CLEANUP_STARTED,
+                         "ctx cleanup started");
+        return -1;
+    }
+
+    LOCK(&ctx->lock);
+    {
+        reg = ctx->timer;
+    }
+    UNLOCK(&ctx->lock);
 
+    if (!reg) {
+        /* This can happen when cleanup may have just started and
+         * gf_timer_registry_destroy() sets ctx->timer to NULL.
+         * gf_timer_proc() takes care of cleaning up the events.
+         */
+        return -1;
+    }
+
+    pthread_mutex_lock(&reg->lock);
+    {
+        fired = event->fired;
+        if (fired)
+            goto unlock;
+        list_del(&event->list);
+    }
+unlock:
+    pthread_mutex_unlock(&reg->lock);
+
+    if (!fired) {
+        GF_FREE(event);
         return 0;
+    }
+    return -1;
 }
 
-int32_t
-gf_timer_call_cancel (glusterfs_ctx_t *ctx,
-                      gf_timer_t *event)
+static void *
+gf_timer_proc(void *data)
 {
-        gf_timer_registry_t *reg = NULL;
-
-        if (ctx == NULL || event == NULL)
-        {
-                gf_log_callingfn ("timer", GF_LOG_ERROR, "invalid argument");
-                return 0;
-        }
-
-        reg = gf_timer_registry_init (ctx);
-        if (!reg) {
-                gf_log ("timer", GF_LOG_ERROR, "!reg");
-                GF_FREE (event);
-                return 0;
-        }
+    gf_timer_registry_t *reg = data;
+    gf_timer_t *event = NULL;
+    gf_timer_t *tmp = NULL;
+    xlator_t *old_THIS = NULL;
+
+    pthread_mutex_lock(&reg->lock);
+
+    while (!reg->fin) {
+        if (list_empty(&reg->active)) {
+            pthread_cond_wait(&reg->cond, &reg->lock);
+        } else {
+            struct timespec now;
+
+            timespec_now(&now);
+            event = list_first_entry(&reg->active, gf_timer_t, list);
+            if (TS(now) < TS(event->at)) {
+                now = event->at;
+                pthread_cond_timedwait(&reg->cond, &reg->lock, &now);
+            } else {
+                event->fired = _gf_true;
+                list_del_init(&event->list);
+
+                pthread_mutex_unlock(&reg->lock);
+
+                old_THIS = NULL;
+                if (event->xl) {
+                    old_THIS = THIS;
+                    THIS = event->xl;
+                }
+                event->callbk(event->data);
+                GF_FREE(event);
+                if (old_THIS) {
+                    THIS = old_THIS;
+                }
 
-        pthread_mutex_lock (&reg->lock);
-        {
-                event->next->prev = event->prev;
-                event->prev->next = event->next;
+                pthread_mutex_lock(&reg->lock);
+            }
         }
-        pthread_mutex_unlock (&reg->lock);
+    }
+
+    /* Do not call gf_timer_call_cancel(),
+     * it will lead to deadlock
+     */
+    list_for_each_entry_safe(event, tmp, &reg->active, list)
+    {
+        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.
+         */
+        GF_FREE(event);
+    }
 
-        GF_FREE (event);
-        return 0;
-}
+    pthread_mutex_unlock(&reg->lock);
 
-static inline void __delete_entry (gf_timer_t *event) {
-        event->next->prev = event->prev;
-        event->prev->next = event->next;
-        GF_FREE (event);
+    return NULL;
 }
 
-void *
-gf_timer_proc (void *ctx)
+static gf_timer_registry_t *
+gf_timer_registry_init(glusterfs_ctx_t *ctx)
 {
-        gf_timer_registry_t *reg = NULL;
-        const struct timespec sleepts = {.tv_sec = 1, .tv_nsec = 0, };
-        gf_timer_t *event = NULL;
-        xlator_t   *old_THIS = NULL;
+    gf_timer_registry_t *reg = NULL;
+    int ret = -1;
+    pthread_condattr_t attr;
 
-        if (ctx == NULL)
-        {
-                gf_log_callingfn ("timer", GF_LOG_ERROR, "invalid argument");
-                return NULL;
+    LOCK(&ctx->lock);
+    {
+        reg = ctx->timer;
+        if (reg) {
+            UNLOCK(&ctx->lock);
+            goto out;
         }
-
-        reg = gf_timer_registry_init (ctx);
+        reg = GF_CALLOC(1, sizeof(*reg), gf_common_mt_gf_timer_registry_t);
         if (!reg) {
-                gf_log ("timer", GF_LOG_ERROR, "!reg");
-                return NULL;
+            UNLOCK(&ctx->lock);
+            goto out;
         }
+        ctx->timer = reg;
+        pthread_mutex_init(&reg->lock, NULL);
+        pthread_condattr_init(&attr);
+        pthread_condattr_setclock(&attr, CLOCK_MONOTONIC);
+        pthread_cond_init(&reg->cond, &attr);
+        INIT_LIST_HEAD(&reg->active);
+    }
+    UNLOCK(&ctx->lock);
+    ret = gf_thread_create(&reg->th, NULL, gf_timer_proc, reg, "timer");
+    if (ret) {
+        gf_msg(THIS->name, GF_LOG_ERROR, ret, LG_MSG_PTHREAD_FAILED,
+               "Thread creation failed");
+    }
 
-        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;
-
-                        pthread_mutex_lock (&reg->lock);
-                        {
-                                event = reg->active.next;
-                                at = TS (event->at);
-                                if (event != &reg->active && now >= at) {
-                                        need_cbk = 1;
-                                        gf_timer_call_stale (reg, event);
-                                }
-                        }
-                        pthread_mutex_unlock (&reg->lock);
-                        if (need_cbk) {
-                                old_THIS = NULL;
-                                if (event->xl) {
-                                        old_THIS = THIS;
-                                        THIS = event->xl;
-                                }
-                                event->callbk (event->data);
-                                /*This callbk above would have freed the event
-                                 * by calling timer_cancel, don't ever touch it
-                                 * again*/
-                                if (old_THIS) {
-                                        THIS = old_THIS;
-                                }
-                        }
-                        else
-                                break;
-                }
-                nanosleep (&sleepts, NULL);
-        }
+out:
+    return reg;
+}
 
-        pthread_mutex_lock (&reg->lock);
-        {
-                /* Do not call gf_timer_call_cancel(),
-                 * it will lead to deadlock
-                 */
-                while (reg->active.next != &reg->active) {
-                        event = reg->active.next;
-                        /* cannot call list_del as the event doesnt have
-                         * list_head*/
-                        __delete_entry (event);
-                }
+void
+gf_timer_registry_destroy(glusterfs_ctx_t *ctx)
+{
+    pthread_t thr_id;
+    gf_timer_registry_t *reg = NULL;
 
-                while (reg->stale.next != &reg->stale) {
-                        event = reg->stale.next;
-                        __delete_entry (event);
-                }
-        }
-        pthread_mutex_unlock (&reg->lock);
-        pthread_mutex_destroy (&reg->lock);
-        GF_FREE (((glusterfs_ctx_t *)ctx)->timer);
+    if (ctx == NULL)
+        return;
 
-        return NULL;
-}
+    LOCK(&ctx->lock);
+    {
+        reg = ctx->timer;
+        ctx->timer = NULL;
+    }
+    UNLOCK(&ctx->lock);
 
-gf_timer_registry_t *
-gf_timer_registry_init (glusterfs_ctx_t *ctx)
-{
-        if (ctx == NULL) {
-                gf_log_callingfn ("timer", GF_LOG_ERROR, "invalid argument");
-                return NULL;
-        }
+    if (!reg)
+        return;
 
-        if (!ctx->timer) {
-                gf_timer_registry_t *reg = NULL;
+    thr_id = reg->th;
 
-                reg = GF_CALLOC (1, sizeof (*reg),
-                                 gf_common_mt_gf_timer_registry_t);
-                if (!reg)
-                        goto out;
+    pthread_mutex_lock(&reg->lock);
 
-                pthread_mutex_init (&reg->lock, NULL);
-                reg->active.next = &reg->active;
-                reg->active.prev = &reg->active;
-                reg->stale.next = &reg->stale;
-                reg->stale.prev = &reg->stale;
+    reg->fin = 1;
+    pthread_cond_signal(&reg->cond);
 
-                ctx->timer = reg;
-                gf_thread_create (&reg->th, NULL, gf_timer_proc, ctx);
-        }
-out:
-        return ctx->timer;
-}
+    pthread_mutex_unlock(&reg->lock);
 
-void
-gf_timer_registry_destroy (glusterfs_ctx_t *ctx)
-{
-        pthread_t thr_id;
-        gf_timer_registry_t *reg = NULL;
+    pthread_join(thr_id, NULL);
 
-        if (ctx == NULL)
-                return;
+    pthread_cond_destroy(&reg->cond);
+    pthread_mutex_destroy(&reg->lock);
 
-        reg = ctx->timer;
-        thr_id = reg->th;
-        reg->fin = 1;
-        pthread_join (thr_id, NULL);
+    GF_FREE(reg);
 }