Update the timing API to have better support for multiple timing interfaces.

1) Add module use count handling so that timing modules can be unloaded.

2) Implement unload_module() functions for the timing interface modules.

3) Allow multiple timing modules to be loaded, and use the one with the
   highest priority value.

4) Report which timing module is being use in the "timing test" CLI command.

(closes issue #14489)
Reported by: russell

Review: http://reviewboard.digium.com/r/162/


git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@176666 65c4cc65-6c06-0410-ace0-fbb531ad65f3

include/asterisk/timing.h

@@ -1,9 +1,10 @@
 /*
  * Asterisk -- An open source telephony toolkit.
  *
- * Copyright (C) 2008, Digium, Inc.
+ * Copyright (C) 2008 - 2009, Digium, Inc.
  *
  * Kevin P. Fleming <kpfleming@digium.com>
+ * Russell Bryant <russell@digium.com>
  *
  * See http://www.asterisk.org for more information about
  * the Asterisk project. Please do not directly contact
@@ -20,6 +21,7 @@
   \file timing.h
   \brief Timing source management
   \author Kevin P. Fleming <kpfleming@digium.com>
+  \author Russell Bryant <russell@digium.com>
 
   Portions of Asterisk require a timing source, a periodic trigger
   for media handling activities. The functions in this file allow
@@ -55,7 +57,7 @@
 extern "C" {
 #endif
 
-enum ast_timing_event {
+enum ast_timer_event {
 	AST_TIMING_EVENT_EXPIRED = 1,
 	AST_TIMING_EVENT_CONTINUOUS = 2,
 };
@@ -67,37 +69,44 @@ enum ast_timing_event {
  * So, the behavior of these calls should match the documentation of the
  * public API calls.
  */
-struct ast_timing_functions {
+struct ast_timing_interface {
+	const char *name;
+	/*! This handles the case where multiple timing modules are loaded.
+	 *  The highest priority timing interface available will be used. */
+	unsigned int priority;
 	int (*timer_open)(void);
 	void (*timer_close)(int handle);
 	int (*timer_set_rate)(int handle, unsigned int rate);
 	void (*timer_ack)(int handle, unsigned int quantity);
 	int (*timer_enable_continuous)(int handle);
 	int (*timer_disable_continuous)(int handle);
-	enum ast_timing_event (*timer_get_event)(int handle);
+	enum ast_timer_event (*timer_get_event)(int handle);
 	unsigned int (*timer_get_max_rate)(int handle);
 };
 
 /*!
- * \brief Install a set of timing functions.
+ * \brief Register a set of timing functions.
  *
- * \param funcs An instance of the \c ast_timing_functions structure with pointers
+ * \param funcs An instance of the \c ast_timing_interfaces structure with pointers
  *        to the functions provided by the timing implementation.
  *
  * \retval NULL failure 
- * \retval non-Null handle to be passed to ast_uninstall_timing_functions() on success
+ * \retval non-Null handle to be passed to ast_unregister_timing_interface() on success
  */
-void *ast_install_timing_functions(struct ast_timing_functions *funcs);
+#define ast_register_timing_interface(i) _ast_register_timing_interface(i, ast_module_info->self)
+void *_ast_register_timing_interface(struct ast_timing_interface *funcs,
+		struct ast_module *mod);
 
 /*!
- * \brief Uninstall a previously-installed set of timing functions.
+ * \brief Unregister a previously registered timing interface.
  *
  * \param handle The handle returned from a prior successful call to
- *        ast_install_timing_functions().
+ *        ast_register_timing_interface().
  *
- * \return nothing
+ * \retval 0 success
+ * \retval non-zero failure
  */
-void ast_uninstall_timing_functions(void *handle);
+int ast_unregister_timing_interface(void *handle);
 
 /*!
  * \brief Open a timing fd
@@ -177,7 +186,7 @@ int ast_timer_disable_continuous(int handle);
  *
  * \return which event triggered the timing fd
  */
-enum ast_timing_event ast_timer_get_event(int handle);
+enum ast_timer_event ast_timer_get_event(int handle);
 
 /*!
  * \brief Get maximum rate supported for a timing handle

main/channel.c

@@ -2499,7 +2499,7 @@ static struct ast_frame *__ast_read(struct ast_channel *chan, int dropaudio)
 	}
 
 	if (chan->timingfd > -1 && chan->fdno == AST_TIMING_FD) {
-		enum ast_timing_event res;
+		enum ast_timer_event res;
 
 		ast_clear_flag(chan, AST_FLAG_EXCEPTION);
 

main/timing.c

@@ -1,9 +1,10 @@
 /*
  * Asterisk -- An open source telephony toolkit.
  *
- * Copyright (C) 2008, Digium, Inc.
+ * Copyright (C) 2008 - 2009, Digium, Inc.
  *
  * Kevin P. Fleming <kpfleming@digium.com>
+ * Russell Bryant <russell@digium.com>
  *
  * See http://www.asterisk.org for more information about
  * the Asterisk project. Please do not directly contact
@@ -21,6 +22,7 @@
  * \brief Timing source management
  *
  * \author Kevin P. Fleming <kpfleming@digium.com>
+ * \author Russell Bryant <russell@digium.com>
  */
 
 #include "asterisk.h"
@@ -34,13 +36,37 @@ ASTERISK_FILE_VERSION(__FILE__, "$Revision$")
 #include "asterisk/cli.h"
 #include "asterisk/utils.h"
 #include "asterisk/time.h"
+#include "asterisk/heap.h"
+#include "asterisk/module.h"
+
+struct timing_holder {
+	/*! Do _not_ move this from the beginning of the struct. */
+	ssize_t __heap_index;
+	struct ast_module *mod;
+	struct ast_timing_interface *iface;
+};
 
-AST_RWLOCK_DEFINE_STATIC(lock);
+static struct ast_heap *timing_interfaces;
 
-static struct ast_timing_functions timer_funcs;
+static int timing_holder_cmp(void *_h1, void *_h2)
+{
+	struct timing_holder *h1 = _h1;
+	struct timing_holder *h2 = _h2;
+
+	if (h1->iface->priority > h2->iface->priority) {
+		return 1;
+	} else if (h1->iface->priority == h2->iface->priority) {
+		return 0;
+	} else {
+		return -1;
+	}
+}
 
-void *ast_install_timing_functions(struct ast_timing_functions *funcs)
+void *_ast_register_timing_interface(struct ast_timing_interface *funcs, 
+		struct ast_module *mod)
 {
+	struct timing_holder *h;
+
 	if (!funcs->timer_open ||
 	    !funcs->timer_close ||
 		!funcs->timer_set_rate ||
@@ -52,162 +78,158 @@ void *ast_install_timing_functions(struct ast_timing_functions *funcs)
 		return NULL;
 	}
 
-	ast_rwlock_wrlock(&lock);
-
-	if (timer_funcs.timer_open) {
-		ast_rwlock_unlock(&lock);
-		ast_log(LOG_NOTICE, "Multiple timing modules are loaded.  You should only load one.\n");
+	if (!(h = ast_calloc(1, sizeof(*h)))) {
 		return NULL;
 	}
-	
-	timer_funcs = *funcs;
 
-	ast_rwlock_unlock(&lock);
+	h->iface = funcs;
+	h->mod = mod;
 
-	return &timer_funcs;
+	ast_heap_wrlock(timing_interfaces);
+	ast_heap_push(timing_interfaces, h);
+	ast_heap_unlock(timing_interfaces);
+
+	return h;
 }
 
-void ast_uninstall_timing_functions(void *handle)
+int ast_unregister_timing_interface(void *handle)
 {
-	ast_rwlock_wrlock(&lock);
+	struct timing_holder *h = handle;
+	int res = -1;
 
-	if (handle != &timer_funcs) {
-		ast_rwlock_unlock(&lock);
-		return;
-	}
+	ast_heap_wrlock(timing_interfaces);
+	h = ast_heap_remove(timing_interfaces, h);
+	ast_heap_unlock(timing_interfaces);
 
-	memset(&timer_funcs, 0, sizeof(timer_funcs));
+	if (h) {
+		ast_free(h);
+		h = NULL;
+		res = 0;
+	}
 
-	ast_rwlock_unlock(&lock);
+	return res;
 }
 
 int ast_timer_open(void)
 {
-	int timer;
+	int fd = -1;
+	struct timing_holder *h;
 
-	ast_rwlock_rdlock(&lock);
+	ast_heap_rdlock(timing_interfaces);
 
-	if (!timer_funcs.timer_open) {
-		ast_rwlock_unlock(&lock);
-		return -1;
+	if ((h = ast_heap_peek(timing_interfaces, 1))) {
+		fd = h->iface->timer_open();
+		ast_module_ref(h->mod);
 	}
 
-	timer = timer_funcs.timer_open();
-
-	ast_rwlock_unlock(&lock);
+	ast_heap_unlock(timing_interfaces);
 
-	return timer;
+	return fd;
 }
 
 void ast_timer_close(int timer)
 {
-	ast_rwlock_rdlock(&lock);
+	struct timing_holder *h;
 
-	if (!timer_funcs.timer_close) {
-		ast_rwlock_unlock(&lock);
-		return;
-	}
+	ast_heap_rdlock(timing_interfaces);
 
-	timer_funcs.timer_close(timer);
+	if ((h = ast_heap_peek(timing_interfaces, 1))) {
+		h->iface->timer_close(timer);
+		ast_module_unref(h->mod);
+	}
 
-	ast_rwlock_unlock(&lock);
+	ast_heap_unlock(timing_interfaces);
 }
 
 int ast_timer_set_rate(int handle, unsigned int rate)
 {
-	int res;
+	struct timing_holder *h;
+	int res = -1;
 
-	ast_rwlock_rdlock(&lock);
+	ast_heap_rdlock(timing_interfaces);
 
-	if (!timer_funcs.timer_set_rate) {
-		ast_rwlock_unlock(&lock);
-		return -1;
+	if ((h = ast_heap_peek(timing_interfaces, 1))) {
+		res = h->iface->timer_set_rate(handle, rate);
 	}
 
-	res = timer_funcs.timer_set_rate(handle, rate);
-
-	ast_rwlock_unlock(&lock);
+	ast_heap_unlock(timing_interfaces);
 
 	return res;
 }
 
 void ast_timer_ack(int handle, unsigned int quantity)
 {
-	ast_rwlock_rdlock(&lock);
+	struct timing_holder *h;
 
-	if (!timer_funcs.timer_ack) {
-		ast_rwlock_unlock(&lock);
-		return;
-	}
+	ast_heap_rdlock(timing_interfaces);
 
-	timer_funcs.timer_ack(handle, quantity);
+	if ((h = ast_heap_peek(timing_interfaces, 1))) {
+		h->iface->timer_ack(handle, quantity);
+	}
 
-	ast_rwlock_unlock(&lock);
+	ast_heap_unlock(timing_interfaces);
 }
 
 int ast_timer_enable_continuous(int handle)
 {
-	int result;
+	struct timing_holder *h;
+	int res = -1;
 
-	ast_rwlock_rdlock(&lock);
+	ast_heap_rdlock(timing_interfaces);
 
-	if (!timer_funcs.timer_enable_continuous) {
-		ast_rwlock_unlock(&lock);
-		return -1;
+	if ((h = ast_heap_peek(timing_interfaces, 1))) {
+		res = h->iface->timer_enable_continuous(handle);
 	}
 
-	result = timer_funcs.timer_enable_continuous(handle);
+	ast_heap_unlock(timing_interfaces);
 
-	ast_rwlock_unlock(&lock);
-
-	return result;
+	return res;
 }
 
 int ast_timer_disable_continuous(int handle)
 {
-	int result;
+	struct timing_holder *h;
+	int res = -1;
 
-	ast_rwlock_rdlock(&lock);
+	ast_heap_rdlock(timing_interfaces);
 
-	if (!timer_funcs.timer_disable_continuous) {
-		ast_rwlock_unlock(&lock);
-		return -1;
+	if ((h = ast_heap_peek(timing_interfaces, 1))) {
+		res = h->iface->timer_disable_continuous(handle);
 	}
 
-	result = timer_funcs.timer_disable_continuous(handle);
-
-	ast_rwlock_unlock(&lock);
+	ast_heap_unlock(timing_interfaces);
 
-	return result;
+	return res;
 }
 
-enum ast_timing_event ast_timer_get_event(int handle)
+enum ast_timer_event ast_timer_get_event(int handle)
 {
-	enum ast_timing_event result;
+	struct timing_holder *h;
+	enum ast_timer_event res = -1;
 
-	ast_rwlock_rdlock(&lock);
+	ast_heap_rdlock(timing_interfaces);
 
-	if (!timer_funcs.timer_get_event) {
-		ast_rwlock_unlock(&lock);
-		return -1;
+	if ((h = ast_heap_peek(timing_interfaces, 1))) {
+		res = h->iface->timer_get_event(handle);
 	}
 
-	result = timer_funcs.timer_get_event(handle);
+	ast_heap_unlock(timing_interfaces);
 
-	ast_rwlock_unlock(&lock);
-
-	return result;
+	return res;
 }
 
 unsigned int ast_timer_get_max_rate(int handle)
 {
-	unsigned int res;
+	struct timing_holder *h;
+	unsigned int res = 0;
 
-	ast_rwlock_rdlock(&lock);
+	ast_heap_rdlock(timing_interfaces);
 
-	res = timer_funcs.timer_get_max_rate(handle);
+	if ((h = ast_heap_peek(timing_interfaces, 1))) {
+		res = h->iface->timer_get_max_rate(handle);
+	}
 
-	ast_rwlock_unlock(&lock);
+	ast_heap_unlock(timing_interfaces);
 
 	return res;
 }
@@ -217,6 +239,7 @@ static char *timing_test(struct ast_cli_entry *e, int cmd, struct ast_cli_args *
 	int fd, count = 0;
 	struct timeval start, end;
 	unsigned int test_rate = 50;
+	struct timing_holder *h;
 
 	switch (cmd) {
 	case CLI_INIT:
@@ -242,13 +265,20 @@ static char *timing_test(struct ast_cli_entry *e, int cmd, struct ast_cli_args *
 		}
 	}
 
-	ast_cli(a->fd, "Attempting to test a timer with %u ticks per second ...\n", test_rate);
+	ast_cli(a->fd, "Attempting to test a timer with %u ticks per second.\n", test_rate);
 
 	if ((fd = ast_timer_open()) == -1) {
 		ast_cli(a->fd, "Failed to open timing fd\n");
 		return CLI_FAILURE;
 	}
 
+	ast_heap_rdlock(timing_interfaces);
+	if ((h = ast_heap_peek(timing_interfaces, 1))) {
+		ast_cli(a->fd, "Using the '%s' timing module for this test.\n", h->iface->name);
+		h = NULL;
+	}
+	ast_heap_unlock(timing_interfaces);
+
 	start = ast_tvnow();
 
 	ast_timer_set_rate(fd, test_rate);
@@ -286,5 +316,9 @@ static struct ast_cli_entry cli_timing[] = {
 
 int ast_timing_init(void)
 {
+	if (!(timing_interfaces = ast_heap_create(2, timing_holder_cmp, 0))) {
+		return -1;
+	}
+
 	return ast_cli_register_multiple(cli_timing, ARRAY_LEN(cli_timing));
 }

res/res_timing_dahdi.c

@@ -25,8 +25,6 @@
 
 /*** MODULEINFO
 	<depend>dahdi</depend>
-	<conflict>res_timing_timerfd</conflict>
-	<conflict>res_timing_pthread</conflict>
  ***/
 
 #include "asterisk.h"
@@ -52,10 +50,12 @@ static int dahdi_timer_set_rate(int handle, unsigned int rate);
 static void dahdi_timer_ack(int handle, unsigned int quantity);
 static int dahdi_timer_enable_continuous(int handle);
 static int dahdi_timer_disable_continuous(int handle);
-static enum ast_timing_event dahdi_timer_get_event(int handle);
+static enum ast_timer_event dahdi_timer_get_event(int handle);
 static unsigned int dahdi_timer_get_max_rate(int handle);
 
-static struct ast_timing_functions dahdi_timing_functions = {
+static struct ast_timing_interface dahdi_timing = {
+	.name = "DAHDI",
+	.priority = 100,
 	.timer_open = dahdi_timer_open,
 	.timer_close = dahdi_timer_close,
 	.timer_set_rate = dahdi_timer_set_rate,
@@ -112,7 +112,7 @@ static int dahdi_timer_disable_continuous(int handle)
 	return ioctl(handle, DAHDI_TIMERPONG, &flags) ? -1 : 0;
 }
 
-static enum ast_timing_event dahdi_timer_get_event(int handle)
+static enum ast_timer_event dahdi_timer_get_event(int handle)
 {
 	int res;
 	int event;
@@ -184,17 +184,13 @@ static int load_module(void)
 		return AST_MODULE_LOAD_DECLINE;
 	}
 
-	return (timing_funcs_handle = ast_install_timing_functions(&dahdi_timing_functions)) ?
+	return (timing_funcs_handle = ast_register_timing_interface(&dahdi_timing)) ?
 		AST_MODULE_LOAD_SUCCESS : AST_MODULE_LOAD_DECLINE;
 }
 
 static int unload_module(void)
 {
-	/* ast_uninstall_timing_functions(timing_funcs_handle); */
-
-	/* This module can not currently be unloaded.  No use count handling is being done. */
-
-	return -1;
+	return ast_unregister_timing_interface(timing_funcs_handle);
 }
 
 AST_MODULE_INFO_STANDARD(ASTERISK_GPL_KEY, "DAHDI Timing Interface");

res/res_timing_pthread.c

@@ -23,11 +23,6 @@
  * \brief pthread timing interface 
  */
 
-/*** MODULEINFO
-	<conflict>res_timing_timerfd</conflict>
-	<conflict>res_timing_dahdi</conflict>
- ***/
-
 #include "asterisk.h"
 
 ASTERISK_FILE_VERSION(__FILE__, "$Revision$");
@@ -50,10 +45,12 @@ static int pthread_timer_set_rate(int handle, unsigned int rate);
 static void pthread_timer_ack(int handle, unsigned int quantity);
 static int pthread_timer_enable_continuous(int handle);
 static int pthread_timer_disable_continuous(int handle);
-static enum ast_timing_event pthread_timer_get_event(int handle);
+static enum ast_timer_event pthread_timer_get_event(int handle);
 static unsigned int pthread_timer_get_max_rate(int handle);
 
-static struct ast_timing_functions pthread_timing_functions = {
+static struct ast_timing_interface pthread_timing = {
+	.name = "pthread",
+	.priority = 0, /* use this as a last resort */
 	.timer_open = pthread_timer_open,
 	.timer_close = pthread_timer_close,
 	.timer_set_rate = pthread_timer_set_rate,
@@ -255,10 +252,10 @@ static int pthread_timer_disable_continuous(int handle)
 	return 0;
 }
 
-static enum ast_timing_event pthread_timer_get_event(int handle)
+static enum ast_timer_event pthread_timer_get_event(int handle)
 {
 	struct pthread_timer *timer;
-	enum ast_timing_event res = AST_TIMING_EVENT_EXPIRED;
+	enum ast_timer_event res = AST_TIMING_EVENT_EXPIRED;
 
 	if (!(timer = find_timer(handle, 0))) {
 		return res;
@@ -491,22 +488,26 @@ static int load_module(void)
 		return AST_MODULE_LOAD_DECLINE;
 	}
 
-	return (timing_funcs_handle = ast_install_timing_functions(&pthread_timing_functions)) ?
+	return (timing_funcs_handle = ast_register_timing_interface(&pthread_timing)) ?
 		AST_MODULE_LOAD_SUCCESS : AST_MODULE_LOAD_DECLINE;
 }
 
 static int unload_module(void)
 {
-#if 0
-	/* XXX code to stop the timing thread ... */
+	int res;
 
-	ast_uninstall_timing_functions(timing_funcs_handle);
-	ao2_ref(pthread_timers, -1);
-#endif
+	ast_mutex_lock(&timing_thread.lock);
+	timing_thread.stop = 1;
+	ast_cond_signal(&timing_thread.cond);
+	ast_mutex_unlock(&timing_thread.lock);
+	pthread_join(timing_thread.thread, NULL);
 
-	/* This module can not currently be unloaded.  No use count handling is being done. */
+	if (!(res = ast_unregister_timing_interface(timing_funcs_handle))) {
+		ao2_ref(pthread_timers, -1);
+		pthread_timers = NULL;
+	}
 
-	return -1;
+	return res;
 }
 
 AST_MODULE_INFO_STANDARD(ASTERISK_GPL_KEY, "pthread Timing Interface");

res/res_timing_timerfd.c

@@ -25,8 +25,6 @@
 
 /*** MODULEINFO
 	<depend>timerfd</depend>
-	<conflict>res_timing_pthread</conflict>
-	<conflict>res_timing_dahdi</conflict>
  ***/
 
 #include "asterisk.h"
@@ -48,10 +46,12 @@ static int timerfd_timer_set_rate(int handle, unsigned int rate);
 static void timerfd_timer_ack(int handle, unsigned int quantity);
 static int timerfd_timer_enable_continuous(int handle);
 static int timerfd_timer_disable_continuous(int handle);
-static enum ast_timing_event timerfd_timer_get_event(int handle);
+static enum ast_timer_event timerfd_timer_get_event(int handle);
 static unsigned int timerfd_timer_get_max_rate(int handle);
 
-static struct ast_timing_functions timerfd_timing_functions = {
+static struct ast_timing_interface timerfd_timing = {
+	.name = "timerfd",
+	.priority = 200,
 	.timer_open = timerfd_timer_open,
 	.timer_close = timerfd_timer_close,
 	.timer_set_rate = timerfd_timer_set_rate,
@@ -226,9 +226,9 @@ static int timerfd_timer_disable_continuous(int handle)
 	return res;
 }
 
-static enum ast_timing_event timerfd_timer_get_event(int handle)
+static enum ast_timer_event timerfd_timer_get_event(int handle)
 {
-	enum ast_timing_event res;
+	enum ast_timer_event res;
 	struct timerfd_timer *our_timer, find_helper = {
 		.handle = handle,
 	};
@@ -259,7 +259,7 @@ static int load_module(void)
 		return AST_MODULE_LOAD_DECLINE;
 	}
 
-	if (!(timing_funcs_handle = ast_install_timing_functions(&timerfd_timing_functions))) {
+	if (!(timing_funcs_handle = ast_register_timing_interface(&timerfd_timing))) {
 		ao2_ref(timerfd_timers, -1);
 		return AST_MODULE_LOAD_DECLINE;
 	}
@@ -269,11 +269,14 @@ static int load_module(void)
 
 static int unload_module(void)
 {
-	/* ast_uninstall_timing_functions(timing_funcs_handle); */
+	int res;
 
-	/* This module can not currently be unloaded.  No use count handling is being done. */
+	if (!(res = ast_unregister_timing_interface(timing_funcs_handle))) {
+		ao2_ref(timerfd_timers, -1);
+		timerfd_timers = NULL;
+	}
 
-	return -1;
+	return res;
 }
 
 AST_MODULE_INFO_STANDARD(ASTERISK_GPL_KEY, "Timerfd Timing Interface");