Rework the eventhandler locking for hopefully the last time. The scheme

used popped into my head during my morning commute a few weeks ago, but
it is also very similar (though a bit simpler) to a patch that mini@
developed a while ago.  Basically, each eventhandler list has a mutex and
a run count.  During an eventhandler invocation, the mutex is held while
we traverse the list but is dropped while we execute actual handlers.  Also,
a runcount counter is incremented at the start of an invocation and
decremented at the end of an invocation.  Adding to the list is not a big
deal since the reference of a thread currently executing the handlers
remains valid across an add operation.  Whether or not new handlers are
executed by threads currently executing the handlers for a given list is
indeterminate however.  The harder case is when a handler is removed from
the list.  If the runcount is zero, the handler is simply removed from the
list directly.  If the runcount is not zero, then another thread is
currently executing the handlers of this list, so the priority of this
handler is set to a magic value (currently -1) to mark it as dead.  Dead
handlers are not executed during an invocation.  If the runcount is zero
after it is decremented at the end of an invocation, then a new
eventhandler_prune_list() function is called to remove dead handlers from
the list.

Additional minor notes:
- All the common parts of EVENTHANDLER_INVOKE() and
  EVENTHANDLER_FAST_INVOKE() have been merged into a common
  _EVENTHANDLER_INVOKE() macro to reduce duplication and ease maintenance.
- KTR logging for eventhandlers is now available via the KTR_EVH mask.
- The global eventhander_mutex is no longer recursive.

Tested by:	scottl (SMP i386)

1 files changed, 62 insertions, 43 deletions

diff --git a/sys/sys/eventhandler.h b/sys/sys/eventhandler.h
index 1378217..a168928 100644
--- a/sys/sys/eventhandler.h
+++ b/sys/sys/eventhandler.h
@@ -30,28 +30,65 @@
 #define SYS_EVENTHANDLER_H
 
 #include <sys/lock.h>
-#include <sys/sx.h>
+#include <sys/ktr.h>
+#include <sys/mutex.h>
 #include <sys/queue.h>
 
 struct eventhandler_entry {
 	TAILQ_ENTRY(eventhandler_entry)	ee_link;
 	int				ee_priority;
+#define	EHE_DEAD_PRIORITY	(-1)
 	void				*ee_arg;
 };
 
 struct eventhandler_list {
 	char				*el_name;
 	int				el_flags;
-#define EHE_INITTED	(1<<0)
-	struct sx			el_lock;
+#define EHL_INITTED	(1<<0)
+	u_int				el_runcount;
+	struct mtx			el_lock;
 	TAILQ_ENTRY(eventhandler_list)	el_link;
 	TAILQ_HEAD(,eventhandler_entry)	el_entries;
 };
 
 typedef struct eventhandler_entry	*eventhandler_tag;
 
-#define	EHE_LOCK(p)	sx_xlock(&(p)->el_lock)
-#define	EHE_UNLOCK(p)	sx_xunlock(&(p)->el_lock)
+#define	EHL_LOCK(p)		mtx_lock(&(p)->el_lock)
+#define	EHL_UNLOCK(p)		mtx_unlock(&(p)->el_lock)
+#define	EHL_LOCK_ASSERT(p, x)	mtx_assert(&(p)->el_lock, x)
+
+/*
+ * Macro to invoke the handlers for a given event.
+ */
+#define _EVENTHANDLER_INVOKE(name, list, ...) do {			\
+	struct eventhandler_entry *_ep;					\
+	struct eventhandler_entry_ ## name *_t;				\
+									\
+	KASSERT((list)->el_flags & EHL_INITTED,				\
+ 	   ("eventhandler_invoke: running non-inited list"));		\
+	EHL_LOCK_ASSERT((list), MA_OWNED);				\
+	(list)->el_runcount++;						\
+	KASSERT((list)->el_runcount > 0,				\
+	    ("eventhandler_invoke: runcount overflow"));		\
+	CTR0(KTR_EVH, "eventhandler_invoke(\"" __STRING(name) "\")");	\
+	TAILQ_FOREACH(_ep, &((list)->el_entries), ee_link) {		\
+		if (_ep->ee_priority != EHE_DEAD_PRIORITY) {		\
+			EHL_UNLOCK((list));				\
+			_t = (struct eventhandler_entry_ ## name *)_ep;	\
+			CTR1(KTR_EVH, "eventhandler_invoke: executing %p", \
+ 			    (void *)_t->eh_func);			\
+			_t->eh_func(_ep->ee_arg , ## __VA_ARGS__);	\
+			EHL_LOCK((list));				\
+		}							\
+	}								\
+	KASSERT((list)->el_runcount > 0,				\
+	    ("eventhandler_invoke: runcount underflow"));		\
+	(list)->el_runcount--;						\
+	if ((list)->el_runcount == 0)					\
+		eventhandler_prune_list(list);				\
+	EHL_UNLOCK((list));						\
+} while (0)
+
 
 /* 
  * Fast handler lists require the eventhandler list be present
@@ -75,22 +112,12 @@ struct __hack
 struct eventhandler_list Xeventhandler_list_ ## name = { #name };	\
 struct __hack
 
-#define EVENTHANDLER_FAST_INVOKE(name, ...)				\
-do {									\
+#define EVENTHANDLER_FAST_INVOKE(name, ...) do {			\
 	struct eventhandler_list *_el = &Xeventhandler_list_ ## name ;	\
-	struct eventhandler_entry *_ep, *_en;				\
-	struct eventhandler_entry_ ## name *_t;				\
 									\
-	if (_el->el_flags & EHE_INITTED) {				\
-		EHE_LOCK(_el);						\
-		_ep = TAILQ_FIRST(&(_el->el_entries));			\
-		while (_ep != NULL) {					\
-			_en = TAILQ_NEXT(_ep, ee_link);			\
-			_t = (struct eventhandler_entry_ ## name *)_ep; \
-			_t->eh_func(_ep->ee_arg , __VA_ARGS__);		\
-			_ep = _en;					\
-		}							\
-		EHE_UNLOCK(_el);					\
+	if (_el->el_flags & EHL_INITTED) {				\
+		EHL_LOCK(_el);						\
+		_EVENTHANDLER_INVOKE(name, _el , ## __VA_ARGS__);	\
 	}								\
 } while (0)
 
@@ -98,8 +125,14 @@ do {									\
 	eventhandler_register(&Xeventhandler_list_ ## name,		\
 	#name, func, arg, priority)
 
-#define EVENTHANDLER_FAST_DEREGISTER(name, tag)				\
-	eventhandler_deregister(&Xeventhandler_list_ ## name, tag)
+#define EVENTHANDLER_FAST_DEREGISTER(name, tag)	do {			\
+	struct eventhandler_list *_el = &Xeventhandler_list_ ## name ;	\
+									\
+	KASSERT(_el->el_flags & EHL_INITTED,				\
+	    ("eventhandler_fast_deregister on un-inited list %s", ## name)); \
+	EHL_LOCK(_el);							\
+	eventhandler_deregister(_el, tag);				\
+} while (0)
 
 /*
  * Slow handlers are entirely dynamic; lists are created
@@ -119,21 +152,9 @@ struct __hack
 #define EVENTHANDLER_INVOKE(name, ...)					\
 do {									\
 	struct eventhandler_list *_el;					\
-	struct eventhandler_entry *_ep, *_en;				\
-	struct eventhandler_entry_ ## name *_t;				\
 									\
-	if (((_el = eventhandler_find_list(#name)) != NULL) && 		\
-		(_el->el_flags & EHE_INITTED)) {			\
-		EHE_LOCK(_el);						\
-		_ep = TAILQ_FIRST(&(_el->el_entries));			\
-		while (_ep != NULL) {					\
-			_en = TAILQ_NEXT(_ep, ee_link);			\
-			_t = (struct eventhandler_entry_ ## name *)_ep;	\
-			_t->eh_func(_ep->ee_arg , __VA_ARGS__);		\
-			_ep = _en;					\
-		}							\
-		EHE_UNLOCK(_el);					\
-	}								\
+	if ((_el = eventhandler_find_list(#name)) != NULL) 		\
+		_EVENTHANDLER_INVOKE(name, _el , ## __VA_ARGS__);	\
 } while (0)
 
 #define EVENTHANDLER_REGISTER(name, func, arg, priority)		\
@@ -148,14 +169,12 @@ do {									\
 } while(0)
 	
 
-extern eventhandler_tag	eventhandler_register(struct eventhandler_list *list, 
-					      char *name,
-					      void *func, 
-					      void *arg, 
-					      int priority);
-extern void		eventhandler_deregister(struct eventhandler_list *list,
-						eventhandler_tag tag);
-extern struct eventhandler_list	*eventhandler_find_list(char *name);
+eventhandler_tag eventhandler_register(struct eventhandler_list *list, 
+	    char *name, void *func, void *arg, int priority);
+void	eventhandler_deregister(struct eventhandler_list *list,
+	    eventhandler_tag tag);
+struct eventhandler_list *eventhandler_find_list(char *name);
+void	eventhandler_prune_list(struct eventhandler_list *list);
 
 /*
  * Standard system event queues.