Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 8 Oct 2017 17:21:16 +0000 (UTC)
From:      Ian Lepore <ian@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r324413 - in head: share/man/man9 sys/kern sys/sys
Message-ID:  <201710081721.v98HLGlT086889@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: ian
Date: Sun Oct  8 17:21:16 2017
New Revision: 324413
URL: https://svnweb.freebsd.org/changeset/base/324413

Log:
  Restore the ability to deregister an eventhandler from within the callback.
  
  When the EVENTHANDLER(9) subsystem was created, it was a documented feature
  that an eventhandler callback function could safely deregister itself. In
  r200652 that feature was inadvertantly broken by adding drain-wait logic to
  eventhandler_deregister(), so that it would be safe to unload a module upon
  return from deregistering its event handlers.
  
  There are now 145 callers of EVENTHANDLER_DEREGISTER(), and it's likely many
  of them are depending on the drain-wait logic that has been in place for 8
  years. So instead of creating a separate eventhandler_drain() and adding it
  to some or all of those 145 call sites, this creates a separate
  eventhandler_drain_nowait() function for the specific purpose of
  deregistering a callback from within the running callback.
  
  Differential Revision:	https://reviews.freebsd.org/D12561

Modified:
  head/share/man/man9/EVENTHANDLER.9
  head/sys/kern/subr_eventhandler.c
  head/sys/sys/eventhandler.h

Modified: head/share/man/man9/EVENTHANDLER.9
==============================================================================
--- head/share/man/man9/EVENTHANDLER.9	Sun Oct  8 17:14:45 2017	(r324412)
+++ head/share/man/man9/EVENTHANDLER.9	Sun Oct  8 17:21:16 2017	(r324413)
@@ -23,7 +23,7 @@
 .\" SUCH DAMAGE.
 .\" $FreeBSD$
 .\"
-.Dd March 27, 2017
+.Dd October 1, 2017
 .Dt EVENTHANDLER 9
 .Os
 .Sh NAME
@@ -37,6 +37,7 @@
 .Ft eventhandler_tag
 .Fn EVENTHANDLER_REGISTER name func arg priority
 .Fn EVENTHANDLER_DEREGISTER name tag
+.Fn EVENTHANDLER_DEREGISTER_NOWAIT name tag
 .Ft eventhandler_tag
 .Fo eventhandler_register
 .Fa "struct eventhandler_list *list"
@@ -50,6 +51,11 @@
 .Fa "struct eventhandler_list *list"
 .Fa "eventhandler_tag tag"
 .Fc
+.Ft void
+.Fo eventhandler_deregister_nowait
+.Fa "struct eventhandler_list *list"
+.Fa "eventhandler_tag tag"
+.Fc
 .Ft "struct eventhandler_list *"
 .Fn eventhandler_find_list "const char *name"
 .Ft void
@@ -121,6 +127,18 @@ This macro removes a previously registered callback as
 .Fa tag
 from the event handler named by argument
 .Fa name .
+It waits until no threads are running handlers for this event before
+returning, making it safe to unload a module immediately upon return
+from this function.
+.It Fn EVENTHANDLER_DEREGISTER_NOWAIT
+This macro removes a previously registered callback associated with tag
+.Fa tag
+from the event handler named by argument
+.Fa name .
+Upon return, one or more threads could still be running the removed
+function(s), but no new calls will be made.
+To remove a handler function from within that function, use this
+version of deregister, to avoid a deadlock.
 .It Fn EVENTHANDLER_INVOKE
 This macro is used to invoke all the callbacks associated with event
 handler
@@ -176,6 +194,21 @@ that can later be used with
 .Fn eventhandler_deregister
 to remove the particular callback function.
 .It Fn eventhandler_deregister
+The
+.Fn eventhandler_deregister
+function removes the callback associated with tag
+.Fa tag
+from the event handler list pointed to by
+.Fa list .
+If
+.Fa tag
+is
+.Va NULL ,
+all callback functions for the event are removed.
+This function will not return until all threads have exited from the
+removed handler callback function(s).
+This function is not safe to call from inside an event handler callback.
+.It Fn eventhandler_deregister_nowait
 The
 .Fn eventhandler_deregister
 function removes the callback associated with tag

Modified: head/sys/kern/subr_eventhandler.c
==============================================================================
--- head/sys/kern/subr_eventhandler.c	Sun Oct  8 17:14:45 2017	(r324412)
+++ head/sys/kern/subr_eventhandler.c	Sun Oct  8 17:21:16 2017	(r324413)
@@ -180,8 +180,9 @@ vimage_eventhandler_register(struct eventhandler_list 
 }
 #endif
 
-void
-eventhandler_deregister(struct eventhandler_list *list, eventhandler_tag tag)
+static void
+_eventhandler_deregister(struct eventhandler_list *list, eventhandler_tag tag,
+    bool wait)
 {
     struct eventhandler_entry	*ep = tag;
 
@@ -215,9 +216,24 @@ eventhandler_deregister(struct eventhandler_list *list
 		ep->ee_priority = EHE_DEAD_PRIORITY;
 	}
     }
-    while (list->el_runcount > 0)
+    while (wait && list->el_runcount > 0)
 	    mtx_sleep(list, &list->el_lock, 0, "evhrm", 0);
     EHL_UNLOCK(list);
+}
+
+void
+eventhandler_deregister(struct eventhandler_list *list, eventhandler_tag tag)
+{
+
+	_eventhandler_deregister(list, tag, true);
+}
+
+void
+eventhandler_deregister_nowait(struct eventhandler_list *list,
+    eventhandler_tag tag)
+{
+
+	_eventhandler_deregister(list, tag, false);
 }
 
 /*

Modified: head/sys/sys/eventhandler.h
==============================================================================
--- head/sys/sys/eventhandler.h	Sun Oct  8 17:14:45 2017	(r324412)
+++ head/sys/sys/eventhandler.h	Sun Oct  8 17:21:16 2017	(r324413)
@@ -141,11 +141,20 @@ do {									\
 	if ((_el = eventhandler_find_list(#name)) != NULL)		\
 		eventhandler_deregister(_el, tag);			\
 } while(0)
-	
 
+#define EVENTHANDLER_DEREGISTER_NOWAIT(name, tag)			\
+do {									\
+	struct eventhandler_list *_el;					\
+									\
+	if ((_el = eventhandler_find_list(#name)) != NULL)		\
+		eventhandler_deregister_nowait(_el, tag);		\
+} while(0)
+
 eventhandler_tag eventhandler_register(struct eventhandler_list *list, 
 	    const char *name, void *func, void *arg, int priority);
 void	eventhandler_deregister(struct eventhandler_list *list,
+	    eventhandler_tag tag);
+void	eventhandler_deregister_nowait(struct eventhandler_list *list,
 	    eventhandler_tag tag);
 struct eventhandler_list *eventhandler_find_list(const char *name);
 void	eventhandler_prune_list(struct eventhandler_list *list);



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201710081721.v98HLGlT086889>