From fc09164658bf68154962af2c722862ed8ec97bb3 Mon Sep 17 00:00:00 2001 From: Ian Lepore Date: Sun, 8 Oct 2017 17:21:16 +0000 Subject: [PATCH] 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 --- share/man/man9/EVENTHANDLER.9 | 35 ++++++++++++++++++++++++++++++++++- sys/kern/subr_eventhandler.c | 22 +++++++++++++++++++--- sys/sys/eventhandler.h | 11 ++++++++++- 3 files changed, 63 insertions(+), 5 deletions(-) diff --git a/share/man/man9/EVENTHANDLER.9 b/share/man/man9/EVENTHANDLER.9 index b6be88319b4f..93dcfc02b0a2 100644 --- a/share/man/man9/EVENTHANDLER.9 +++ b/share/man/man9/EVENTHANDLER.9 @@ -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 associated with tag .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 @@ -182,6 +200,21 @@ 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 +.Fa tag +from the event handler list pointed to by +.Fa list . This function is safe to call from inside an event handler callback. .It Fn eventhandler_find_list diff --git a/sys/kern/subr_eventhandler.c b/sys/kern/subr_eventhandler.c index 5894099abd2e..793210cfe946 100644 --- a/sys/kern/subr_eventhandler.c +++ b/sys/kern/subr_eventhandler.c @@ -180,8 +180,9 @@ vimage_eventhandler_register(struct eventhandler_list *list, const char *name, } #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,11 +216,26 @@ eventhandler_deregister(struct eventhandler_list *list, eventhandler_tag tag) 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); +} + /* * Internal version for use when eventhandler list is already locked. */ diff --git a/sys/sys/eventhandler.h b/sys/sys/eventhandler.h index b071c63926ee..ffefdcc9c389 100644 --- a/sys/sys/eventhandler.h +++ b/sys/sys/eventhandler.h @@ -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);