Date: Mon, 8 Sep 2014 04:51:35 GMT From: John Baldwin <jhb@FreeBSD.org> To: Perforce Change Reviews <perforce@FreeBSD.org> Subject: PERFORCE change 1199643 for review Message-ID: <201409080451.s884pZSf027999@skunkworks.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://p4web.freebsd.org/@@1199643?ac=10 Change 1199643 by jhb@jhb_pippin on 2014/09/01 13:43:29 Simplify the "fast" eventhandler stuff. The lists are left as dynamically allocated, but the fast invocations cache a pointer to the list (rather than statically allocating the list). Affected files ... .. //depot/projects/smpng/sys/kern/subr_eventhandler.c#28 edit .. //depot/projects/smpng/sys/modules/evbench/evbench.c#2 edit .. //depot/projects/smpng/sys/sys/eventhandler.h#44 edit Differences ... ==== //depot/projects/smpng/sys/kern/subr_eventhandler.c#28 (text+ko) ==== @@ -50,6 +50,8 @@ }; static struct eventhandler_list *_eventhandler_find_list(const char *name); +static struct eventhandler_list *eventhandler_list_alloc(const char *name); +static void eventhandler_list_free(struct eventhandler_list *el); /* * Initialize the eventhandler mutex and list. @@ -65,25 +67,53 @@ NULL); /* - * Initialize a pre-allocated "fast" eventhandler list. The list's - * name is set in 'el_name', but the list is otherwise uninitialized. + * Locate an existing eventhandler list with the given name. If an + * existing list is not found, create a new list. + */ +static struct eventhandler_list * +eventhandler_list_alloc(const char *name) +{ + struct eventhandler_list *list, *new_list; + + mtx_lock(&eventhandler_mutex); + list = _eventhandler_find_list(name); + if (list == NULL) { + mtx_unlock(&eventhandler_mutex); + + new_list = malloc(sizeof(struct eventhandler_list) + strlen(name) + 1, + M_EVENTHANDLER, M_WAITOK | M_ZERO); + + mtx_lock(&eventhandler_mutex); + list = _eventhandler_find_list(name); + if (list != NULL) { + free(new_list, M_EVENTHANDLER); + } else { + CTR2(KTR_EVH, "%s: creating list \"%s\"", __func__, name); + list = new_list; + list->el_name = (char *)list + sizeof(struct eventhandler_list); + strcpy(list->el_name, name); + TAILQ_INIT(&list->el_entries); + mtx_init(&list->el_lock, name, "eventhandler list", MTX_DEF); + TAILQ_INSERT_HEAD(&eventhandler_lists, list, el_link); + atomic_store_rel_int(&list->el_flags, EHL_INITTED); + } + } + mtx_unlock(&eventhandler_mutex); + return (list); +} + +/* + * Initialize a reference to a eventhandler list used for "fast" + * invocations. */ void eventhandler_fast_list_init(void *arg) { - struct eventhandler_list *list; + struct eventhandler_list_init *eli; - list = arg; - CTR2(KTR_EVH, "%s: initializing list \"%s\"", __func__, list->el_name); - TAILQ_INIT(&list->el_entries); - mtx_init(&list->el_lock, list->el_name, "eventhandler list", MTX_DEF); - atomic_store_rel_int(&list->el_flags, EHL_INITTED); - - mtx_lock(&eventhandler_mutex); - if (_eventhandler_find_list(list->el_name) != NULL) - panic("Duplicate fast eventhandler list: %s", list->el_name); - TAILQ_INSERT_HEAD(&eventhandler_lists, list, el_link); - mtx_unlock(&eventhandler_mutex); + eli = arg; + CTR2(KTR_EVH, "%s: looking up list \"%s\"", __func__, eli->eli_name); + eli->eli_list = eventhandler_list_alloc(eli->eli_name); } /* @@ -94,45 +124,13 @@ eventhandler_register_internal(struct eventhandler_list *list, const char *name, eventhandler_tag epn) { - struct eventhandler_list *new_list; struct eventhandler_entry *ep; KASSERT(eventhandler_lists_initted, ("eventhandler registered too early")); KASSERT(epn != NULL, ("%s: cannot register NULL event", __func__)); - /* lock the eventhandler lists */ - mtx_lock(&eventhandler_mutex); - - /* Do we need to find/create the list? */ - if (list == NULL) { - /* look for a matching, existing list */ - list = _eventhandler_find_list(name); - - /* Do we need to create the list? */ - if (list == NULL) { - mtx_unlock(&eventhandler_mutex); - - new_list = malloc(sizeof(struct eventhandler_list) + - strlen(name) + 1, M_EVENTHANDLER, M_WAITOK | M_ZERO); - - /* If someone else created it already, then use that one. */ - mtx_lock(&eventhandler_mutex); - list = _eventhandler_find_list(name); - if (list != NULL) { - free(new_list, M_EVENTHANDLER); - } else { - CTR2(KTR_EVH, "%s: creating list \"%s\"", __func__, name); - list = new_list; - list->el_name = (char *)list + sizeof(struct eventhandler_list); - strcpy(list->el_name, name); - TAILQ_INIT(&list->el_entries); - mtx_init(&list->el_lock, name, "eventhandler list", MTX_DEF); - TAILQ_INSERT_HEAD(&eventhandler_lists, list, el_link); - atomic_store_rel_int(&list->el_flags, EHL_INITTED); - } - } - } - mtx_unlock(&eventhandler_mutex); + if (list == NULL) + list = eventhandler_list_alloc(name); KASSERT(epn->ee_priority != EHE_DEAD_PRIORITY, ("%s: handler for %s registered with dead priority", __func__, name)); ==== //depot/projects/smpng/sys/modules/evbench/evbench.c#2 (text+ko) ==== @@ -49,9 +49,8 @@ typedef void (*evbench_fn)(void *); EVENTHANDLER_DECLARE(evbench, evbench_fn); -EVENTHANDLER_DECLARE(fast_evbench, evbench_fn); -static EVENTHANDLER_FAST_DEFINE(fast_evbench, evbench_fn); +static EVENTHANDLER_FAST_DEFINE(evbench); static SYSCTL_NODE(_debug, OID_AUTO, evbench, CTLFLAG_RD, 0, "eventhandler tree"); @@ -75,29 +74,28 @@ return; sched_pin(); - /* Invoke of "fast" list with no handlers. */ + /* "Fast" invocation with no handlers. */ start = rdtsc(); for (i = 0; i < loops; i++) - EVENTHANDLER_FAST_INVOKE(fast_evbench); + EVENTHANDLER_FAST_INVOKE(evbench); times[0] = rdtsc() - start; - /* Invoke of "slow" list with no handlers. */ + /* Default invocation with no handlers. */ start = rdtsc(); for (i = 0; i < loops; i++) EVENTHANDLER_INVOKE(evbench); times[1] = rdtsc() - start; - /* Add null handler to each list. */ - (void)EVENTHANDLER_REGISTER(fast_evbench, null_handler, NULL, 0); + /* Add null handler. */ (void)EVENTHANDLER_REGISTER(evbench, null_handler, NULL, 0); - /* Invoke of "fast" list with one handler. */ + /* "Fast" invocation with one handler. */ start = rdtsc(); for (i = 0; i < loops; i++) - EVENTHANDLER_FAST_INVOKE(fast_evbench); + EVENTHANDLER_FAST_INVOKE(evbench); times[2] = rdtsc() - start; - /* Invoke of "slow" list with one handler. */ + /* "Slow" invocation with one handler. */ start = rdtsc(); for (i = 0; i < loops; i++) EVENTHANDLER_INVOKE(evbench); @@ -133,9 +131,6 @@ load(void *arg) { - /* Add and remove a handler to the slow list to force its creation. */ - (void)EVENTHANDLER_REGISTER(evbench, null_handler, NULL, 0); - EVENTHANDLER_DEREGISTER(evbench, NULL); return (0); } @@ -143,8 +138,7 @@ unload(void *arg) { - /* Can't safely unload a "fast" list. */ - return (EBUSY); + return (0); } ==== //depot/projects/smpng/sys/sys/eventhandler.h#44 (text+ko) ==== @@ -59,6 +59,11 @@ TAILQ_HEAD(,eventhandler_entry) el_entries; }; +struct eventhandler_list_init { + char *eli_name; + struct eventhandler_list *eli_list; +}; + typedef struct eventhandler_entry *eventhandler_tag; #define EHL_LOCK(p) mtx_lock(&(p)->el_lock) @@ -98,40 +103,31 @@ } while (0) /* - * Fast handler lists require the eventhandler list be present - * at link time. They don't allow addition of entries to - * unknown eventhandler lists, ie. each list must have an - * "owner". - * - * Fast handler lists must be defined once by the owner - * of the eventhandler list, and the declaration must be in - * scope at any point the list is manipulated. - */ - -/* * Event handlers provide named lists of hooks to be invoked when a * specific event occurs. Hooks are added and removed to lists that - * are identified by name. + * are identified by name. Lists are allocated when the first hook + * is registered for an event. + * + * Invoking an event handler calls all of the hooks associated with + * the event. There are two ways to invoke an event handler: * - * There are two types of event handler lists. + * 1) The default invocation operation accepts a list name and uses an + * O(n^2) lookup to locate the list to operate on. This is + * backwards compatible with the old API, but does trigger the + * overhead of the lookup even if no hooks are registered. * - * Default, or "slow" lists allocate the list storage dynamically when - * the first hook is registered for an event. Invoking a default hook - * will always have some overhead as it uses an O(n) lookup to find - * the event handler list by name. + * 2) "Fast" invocations store a local reference to the named list + * when the containing module is loaded. This avoids the need for + * the O(n^2) lookup on each invocation. This does require + * EVENTHANDLER_FAST_DEFINE() to be invoked in the same code module + * at global scope to perform the lookup during module load. * - * "Fast" lists use statically allocated storage for the event handler - * list. As a result, they can avoid the O(n) lookup and should have - * very minimal overhead when no hooks are active. Fast lists can - * never be freed, so they cannot be used in kernel modules that - * support unloading. They must also be defined in addition to being - * declared. + * Note that both invocation methods may be used on the same list. * - * Registering a hook for an event or invoking an event requires that - * the declaration of the list is in scope. + * Registering a hook for an event or invoking an event require the + * event handler to be declared via EVENTHANDLER_DECLARE(). */ -/* "Slow" list operations. */ #define EVENTHANDLER_DECLARE(name, type) \ struct eventhandler_entry_ ## name \ { \ @@ -148,18 +144,14 @@ _EVENTHANDLER_INVOKE(name, _el , ## __VA_ARGS__); \ } while (0) -/* "Fast" list operations. */ -#define EVENTHANDLER_FAST_DECLARE(name, type) \ -extern struct eventhandler_list Xeventhandler_list_ ## name; \ -EVENTHANDLER_DECLARE(name, type) - -#define EVENTHANDLER_FAST_DEFINE(name, type) \ -struct eventhandler_list Xeventhandler_list_ ## name = { #name }; \ +#define EVENTHANDLER_FAST_DEFINE(name) \ +struct eventhandler_list_init Xeventhandler_list_ ## name = \ + { #name, NULL }; \ SYSINIT(eventhandler_list_ ##name, SI_SUB_EVENTHANDLER, SI_ORDER_SECOND, \ eventhandler_fast_list_init, &Xeventhandler_list_ ## name) #define EVENTHANDLER_FAST_INVOKE(name, ...) do { \ - struct eventhandler_list *_el = &Xeventhandler_list_ ## name ; \ + struct eventhandler_list *_el = Xeventhandler_list_ ## name.eli_list; \ \ KASSERT(_el->el_flags & EHL_INITTED, \ ("eventhandler_fast_invoke: too early")); \ @@ -169,7 +161,6 @@ } \ } while (0) -/* Operations to add and remove hooks. */ #define EVENTHANDLER_DEFINE(name, func, arg, priority) \ static eventhandler_tag name ## _tag; \ \ @@ -187,7 +178,7 @@ SYSUNINIT(name ## _evh_fini, SI_SUB_CONFIGURE, SI_ORDER_ANY, \ name ## _evh_fini, NULL) -#define EVENTHANDLER_REGISTER(name, func, arg, priority) \ +#define EVENTHANDLER_REGISTER(name, func, arg, priority) \ eventhandler_register(NULL, #name, func, arg, priority) #define EVENTHANDLER_DEREGISTER(name, tag) \
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201409080451.s884pZSf027999>