Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 6 Jan 2018 17:20:59 +0000 (UTC)
From:      Ian Lepore <ian@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-11@freebsd.org
Subject:   svn commit: r327626 - in stable/11: share/man/man9 sys/kern sys/sys
Message-ID:  <201801061720.w06HKxRS051229@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: ian
Date: Sat Jan  6 17:20:58 2018
New Revision: 327626
URL: https://svnweb.freebsd.org/changeset/base/327626

Log:
  MFC r324413, r324415
  
  r324413:
  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
  
  r324415:
  Add eventhandler notifications for newbus device attach/detach.
  
  The detach case is slightly complicated by the fact that some in-kernel
  consumers may want to know before a device detaches (so they can release
  related resources, stop using the device, etc), but the detach can fail. So
  there are pre- and post-detach notifications for those consumers who need to
  handle all cases.
  
  A couple salient comments from the review, they amount to some helpful
  documentation about these events, but there's currently no good place for
  such documentation...
  
  Note that in the current newbus locking model, DETACH_BEGIN and
  DETACH_COMPLETE/FAILED sequence of event handler invocation might interweave
  with other attach/detach events arbitrarily. The handlers should be prepared
  for such situations.
  
  Also should note that detach may be called after the parent bus knows the
  hardware has left the building. In-kernel consumers have to be prepared to
  cope with this race.
  
  Differential Revision:	https://reviews.freebsd.org/D12557

Modified:
  stable/11/share/man/man9/EVENTHANDLER.9
  stable/11/sys/kern/subr_bus.c
  stable/11/sys/kern/subr_eventhandler.c
  stable/11/sys/sys/eventhandler.h
Directory Properties:
  stable/11/   (props changed)

Modified: stable/11/share/man/man9/EVENTHANDLER.9
==============================================================================
--- stable/11/share/man/man9/EVENTHANDLER.9	Sat Jan  6 16:29:00 2018	(r327625)
+++ stable/11/share/man/man9/EVENTHANDLER.9	Sat Jan  6 17:20:58 2018	(r327626)
@@ -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: stable/11/sys/kern/subr_bus.c
==============================================================================
--- stable/11/sys/kern/subr_bus.c	Sat Jan  6 16:29:00 2018	(r327625)
+++ stable/11/sys/kern/subr_bus.c	Sat Jan  6 17:20:58 2018	(r327626)
@@ -2934,6 +2934,7 @@ device_attach(device_t dev)
 	else
 		dev->state = DS_ATTACHED;
 	dev->flags &= ~DF_DONENOMATCH;
+	EVENTHANDLER_INVOKE(device_attach, dev);
 	devadded(dev);
 	return (0);
 }
@@ -2967,8 +2968,13 @@ device_detach(device_t dev)
 	if (dev->state != DS_ATTACHED)
 		return (0);
 
-	if ((error = DEVICE_DETACH(dev)) != 0)
+	EVENTHANDLER_INVOKE(device_detach, dev, EVHDEV_DETACH_BEGIN);
+	if ((error = DEVICE_DETACH(dev)) != 0) {
+		EVENTHANDLER_INVOKE(device_detach, dev, EVHDEV_DETACH_FAILED);
 		return (error);
+	} else {
+		EVENTHANDLER_INVOKE(device_detach, dev, EVHDEV_DETACH_COMPLETE);
+	}
 	devremoved(dev);
 	if (!device_is_quiet(dev))
 		device_printf(dev, "detached\n");

Modified: stable/11/sys/kern/subr_eventhandler.c
==============================================================================
--- stable/11/sys/kern/subr_eventhandler.c	Sat Jan  6 16:29:00 2018	(r327625)
+++ stable/11/sys/kern/subr_eventhandler.c	Sat Jan  6 17:20:58 2018	(r327626)
@@ -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: stable/11/sys/sys/eventhandler.h
==============================================================================
--- stable/11/sys/sys/eventhandler.h	Sat Jan  6 16:29:00 2018	(r327625)
+++ stable/11/sys/sys/eventhandler.h	Sat Jan  6 17:20:58 2018	(r327626)
@@ -141,12 +141,21 @@ 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);
 
@@ -276,5 +285,16 @@ struct ata_params;
 typedef void (*ada_probe_veto_fn)(void *, struct cam_path *,
     struct ata_params *, int *);
 EVENTHANDLER_DECLARE(ada_probe_veto, ada_probe_veto_fn);
+
+/* newbus device events */
+enum evhdev_detach {
+	EVHDEV_DETACH_BEGIN,    /* Before detach() is called */
+	EVHDEV_DETACH_COMPLETE, /* After detach() returns 0 */
+	EVHDEV_DETACH_FAILED    /* After detach() returns err */
+};
+typedef void (*device_attach_fn)(void *, device_t);
+typedef void (*device_detach_fn)(void *, device_t, enum evhdev_detach);
+EVENTHANDLER_DECLARE(device_attach, device_attach_fn);
+EVENTHANDLER_DECLARE(device_detach, device_detach_fn);
 
 #endif /* _SYS_EVENTHANDLER_H_ */



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