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>