Date: Thu, 9 Nov 2017 22:51:48 +0000 (UTC) From: Matt Joras <mjoras@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r325621 - in head: share/man/man9 sys/kern sys/sys Message-ID: <201711092251.vA9MpmA1035327@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: mjoras Date: Thu Nov 9 22:51:48 2017 New Revision: 325621 URL: https://svnweb.freebsd.org/changeset/base/325621 Log: Introduce EVENTHANDLER_LIST and some users. This introduces a facility to EVENTHANDLER(9) for explicitly defining a reference to an event handler list. This is useful since previously all invokers of events had to do a locked traversal of the global list of event handler lists in order to find the appropriate event handler list. By keeping a pointer to the appropriate list an invoker can avoid this traversal completely. The pointer is initialized with SYSINIT(9) during the eventhandler stage. Users registering interest in events do not need to know if the event is backed by such a list, since the list is added to the global list of lists. As with lists that are not pre-defined it is safe to register for the events before the list has been created. This converts the process_* and thread_* events to using the new facility, as these are events whose locked traversals end up showing up significantly in ports build workflows (and presumably other workflows with many short lived threads/procs). It may be advantageous to convert other events to using the new facility. The el_flags field is now unused, but leave it be so that this revision can be MFC'd. Reviewed by: bdrewery, markj, mjg Approved by: rstone (mentor) In collaboration with: ian MFC after: 4 weeks Sponsored by: Dell EMC Isilon Differential Revision: https://reviews.freebsd.org/D12814 Modified: head/share/man/man9/EVENTHANDLER.9 head/sys/kern/init_main.c head/sys/kern/kern_exec.c head/sys/kern/kern_exit.c head/sys/kern/kern_fork.c head/sys/kern/kern_proc.c head/sys/kern/kern_thread.c head/sys/kern/subr_eventhandler.c head/sys/sys/eventhandler.h Modified: head/share/man/man9/EVENTHANDLER.9 ============================================================================== --- head/share/man/man9/EVENTHANDLER.9 Thu Nov 9 22:26:49 2017 (r325620) +++ head/share/man/man9/EVENTHANDLER.9 Thu Nov 9 22:51:48 2017 (r325621) @@ -23,7 +23,7 @@ .\" SUCH DAMAGE. .\" $FreeBSD$ .\" -.Dd October 1, 2017 +.Dd October 31, 2017 .Dt EVENTHANDLER 9 .Os .Sh NAME @@ -38,6 +38,9 @@ .Fn EVENTHANDLER_REGISTER name func arg priority .Fn EVENTHANDLER_DEREGISTER name tag .Fn EVENTHANDLER_DEREGISTER_NOWAIT name tag +.Fn EVENTHANDLER_LIST_DECLARE name +.Fn EVENTHANDLER_LIST_DEFINE name +.Fn EVENTHANDLER_DIRECT_INVOKE name .Ft eventhandler_tag .Fo eventhandler_register .Fa "struct eventhandler_list *list" @@ -82,8 +85,13 @@ may be used if the handler does not have a specific pr associated with it. .Pp The normal way to use this subsystem is via the macro interface. -The macros that can be used for working with event handlers and callback -function lists are: +For events that are high frequency it is suggested that you additionally use +.Fn EVENTHANDLER_DEFINE_LIST +so that the event handlers can be invoked directly using +.Fn EVENTHANDLER_DIRECT_INVOKE +(see below). +This saves the invoker from having to do a locked traversal of a global +list of event handler lists. .Bl -tag -width indent .It Fn EVENTHANDLER_DECLARE This macro declares an event handler named by argument @@ -148,6 +156,27 @@ Additional arguments to the macro after the .Fa name parameter are passed as the second and subsequent arguments to each registered callback function. +.It Fn EVENTHANDLER_LIST_DEFINE +This macro defines a reference to an event handler list named by +argument +.Fa name . +It uses +.Xr SYSINIT 9 +to initialize the reference and the eventhandler list. +.It Fn EVENTHANDLER_LIST_DECLARE +This macro declares an event handler list named by argument +.Fa name . +This is only needed for users of +.Fn EVENTHANDLER_DIRECT_INVOKE +which are not in the same compilation unit of that list's definition. +.It Fn EVENTHANDLER_DIRECT_INVOKE +This macro invokes the event handlers registered for the list named by +argument +.Fa name . +This macro can only be used if the list was defined with +.Fn EVENTHANDLER_LIST_DEFINE . +The macro is variadic with the same semantics as +.Fn EVENTHANDLER_INVOKE . .El .Pp The macros are implemented using the following functions: @@ -315,7 +344,7 @@ This is never called. .It Vt process_fork Callbacks invoked when a process forks a child. .It Vt process_init -Callback invoked when a process is initalized. +Callback invoked when a process is initialized. .It Vt random_adaptor_attach Callback invoked when a new random module has been loaded. .It Vt register_framebuffer @@ -337,7 +366,7 @@ Callback invoked when a thread object is created. .It Vt thread_dtor Callback invoked when a thread object is destroyed. .It Vt thread_init -Callback invoked when a thread object is initalized. +Callback invoked when a thread object is initialized. .It Vt thread_fini Callback invoked when a thread object is deinitalized. .It Vt usb_dev_configured @@ -384,4 +413,6 @@ facility first appeared in .Fx 4.0 . .Sh AUTHORS This manual page was written by -.An Joseph Koshy Aq Mt jkoshy@FreeBSD.org . +.An Joseph Koshy Aq Mt jkoshy@FreeBSD.org +and +.An Matt Joras Aq Mt mjoras@FreeBSD.org . Modified: head/sys/kern/init_main.c ============================================================================== --- head/sys/kern/init_main.c Thu Nov 9 22:26:49 2017 (r325620) +++ head/sys/kern/init_main.c Thu Nov 9 22:51:48 2017 (r325621) @@ -136,6 +136,11 @@ SET_DECLARE(sysinit_set, struct sysinit); struct sysinit **sysinit, **sysinit_end; struct sysinit **newsysinit, **newsysinit_end; +EVENTHANDLER_LIST_DECLARE(process_init); +EVENTHANDLER_LIST_DECLARE(thread_init); +EVENTHANDLER_LIST_DECLARE(process_ctor); +EVENTHANDLER_LIST_DECLARE(thread_ctor); + /* * Merge a new sysinit set into the current set, reallocating it if * necessary. This can only be called after malloc is running. @@ -580,10 +585,10 @@ proc0_init(void *dummy __unused) * Call the init and ctor for the new thread and proc. We wait * to do this until all other structures are fairly sane. */ - EVENTHANDLER_INVOKE(process_init, p); - EVENTHANDLER_INVOKE(thread_init, td); - EVENTHANDLER_INVOKE(process_ctor, p); - EVENTHANDLER_INVOKE(thread_ctor, td); + EVENTHANDLER_DIRECT_INVOKE(process_init, p); + EVENTHANDLER_DIRECT_INVOKE(thread_init, td); + EVENTHANDLER_DIRECT_INVOKE(process_ctor, p); + EVENTHANDLER_DIRECT_INVOKE(thread_ctor, td); /* * Charge root for one process. Modified: head/sys/kern/kern_exec.c ============================================================================== --- head/sys/kern/kern_exec.c Thu Nov 9 22:26:49 2017 (r325620) +++ head/sys/kern/kern_exec.c Thu Nov 9 22:51:48 2017 (r325621) @@ -144,6 +144,8 @@ static int map_at_zero = 0; SYSCTL_INT(_security_bsd, OID_AUTO, map_at_zero, CTLFLAG_RWTUN, &map_at_zero, 0, "Permit processes to map an object at virtual address 0."); +EVENTHANDLER_LIST_DECLARE(process_exec); + static int sysctl_kern_ps_strings(SYSCTL_HANDLER_ARGS) { @@ -1071,7 +1073,7 @@ exec_new_vmspace(struct image_params *imgp, struct sys imgp->sysent = sv; /* May be called with Giant held */ - EVENTHANDLER_INVOKE(process_exec, p, imgp); + EVENTHANDLER_DIRECT_INVOKE(process_exec, p, imgp); /* * Blow away entire process VM, if address space not shared, Modified: head/sys/kern/kern_exit.c ============================================================================== --- head/sys/kern/kern_exit.c Thu Nov 9 22:26:49 2017 (r325620) +++ head/sys/kern/kern_exit.c Thu Nov 9 22:51:48 2017 (r325621) @@ -99,6 +99,8 @@ SDT_PROBE_DEFINE1(proc, , , exit, "int"); /* Hook for NFS teardown procedure. */ void (*nlminfo_release_p)(struct proc *p); +EVENTHANDLER_LIST_DECLARE(process_exit); + struct proc * proc_realparent(struct proc *child) { @@ -329,7 +331,7 @@ exit1(struct thread *td, int rval, int signo) * Event handler could change exit status. * XXX what if one of these generates an error? */ - EVENTHANDLER_INVOKE(process_exit, p); + EVENTHANDLER_DIRECT_INVOKE(process_exit, p); /* * If parent is waiting for us to exit or exec, Modified: head/sys/kern/kern_fork.c ============================================================================== --- head/sys/kern/kern_fork.c Thu Nov 9 22:26:49 2017 (r325620) +++ head/sys/kern/kern_fork.c Thu Nov 9 22:51:48 2017 (r325621) @@ -97,6 +97,8 @@ struct fork_args { }; #endif +EVENTHANDLER_LIST_DECLARE(process_fork); + /* ARGSUSED */ int sys_fork(struct thread *td, struct fork_args *uap) @@ -699,7 +701,7 @@ do_fork(struct thread *td, struct fork_req *fr, struct * Both processes are set up, now check if any loadable modules want * to adjust anything. */ - EVENTHANDLER_INVOKE(process_fork, p1, p2, fr->fr_flags); + EVENTHANDLER_DIRECT_INVOKE(process_fork, p1, p2, fr->fr_flags); /* * Set the child start time and mark the process as being complete. Modified: head/sys/kern/kern_proc.c ============================================================================== --- head/sys/kern/kern_proc.c Thu Nov 9 22:26:49 2017 (r325620) +++ head/sys/kern/kern_proc.c Thu Nov 9 22:51:48 2017 (r325621) @@ -151,6 +151,17 @@ const int thread_off_td_oncpu = offsetof(struct thread const int thread_off_td_pcb = offsetof(struct thread, td_pcb); const int thread_off_td_plist = offsetof(struct thread, td_plist); +EVENTHANDLER_LIST_DEFINE(process_ctor); +EVENTHANDLER_LIST_DEFINE(process_dtor); +EVENTHANDLER_LIST_DEFINE(process_init); +EVENTHANDLER_LIST_DEFINE(process_fini); +EVENTHANDLER_LIST_DEFINE(process_exit); +EVENTHANDLER_LIST_DEFINE(process_fork); +EVENTHANDLER_LIST_DEFINE(process_exec); + +EVENTHANDLER_LIST_DECLARE(thread_ctor); +EVENTHANDLER_LIST_DECLARE(thread_dtor); + int kstack_pages = KSTACK_PAGES; SYSCTL_INT(_kern, OID_AUTO, kstack_pages, CTLFLAG_RD, &kstack_pages, 0, "Kernel stack size in pages"); @@ -195,12 +206,12 @@ proc_ctor(void *mem, int size, void *arg, int flags) p = (struct proc *)mem; SDT_PROBE4(proc, , ctor , entry, p, size, arg, flags); - EVENTHANDLER_INVOKE(process_ctor, p); + EVENTHANDLER_DIRECT_INVOKE(process_ctor, p); SDT_PROBE4(proc, , ctor , return, p, size, arg, flags); td = FIRST_THREAD_IN_PROC(p); if (td != NULL) { /* Make sure all thread constructors are executed */ - EVENTHANDLER_INVOKE(thread_ctor, td); + EVENTHANDLER_DIRECT_INVOKE(thread_ctor, td); } return (0); } @@ -230,9 +241,9 @@ proc_dtor(void *mem, int size, void *arg) MPASS(td->td_su == NULL); /* Make sure all thread destructors are executed */ - EVENTHANDLER_INVOKE(thread_dtor, td); + EVENTHANDLER_DIRECT_INVOKE(thread_dtor, td); } - EVENTHANDLER_INVOKE(process_dtor, p); + EVENTHANDLER_DIRECT_INVOKE(process_dtor, p); if (p->p_ksi != NULL) KASSERT(! KSI_ONQ(p->p_ksi), ("SIGCHLD queue")); SDT_PROBE3(proc, , dtor, return, p, size, arg); @@ -256,7 +267,7 @@ proc_init(void *mem, int size, int flags) cv_init(&p->p_pwait, "ppwait"); cv_init(&p->p_dbgwait, "dbgwait"); TAILQ_INIT(&p->p_threads); /* all threads in proc */ - EVENTHANDLER_INVOKE(process_init, p); + EVENTHANDLER_DIRECT_INVOKE(process_init, p); p->p_stats = pstats_alloc(); p->p_pgrp = NULL; SDT_PROBE3(proc, , init, return, p, size, flags); @@ -274,7 +285,7 @@ proc_fini(void *mem, int size) struct proc *p; p = (struct proc *)mem; - EVENTHANDLER_INVOKE(process_fini, p); + EVENTHANDLER_DIRECT_INVOKE(process_fini, p); pstats_free(p->p_stats); thread_free(FIRST_THREAD_IN_PROC(p)); mtx_destroy(&p->p_mtx); Modified: head/sys/kern/kern_thread.c ============================================================================== --- head/sys/kern/kern_thread.c Thu Nov 9 22:26:49 2017 (r325620) +++ head/sys/kern/kern_thread.c Thu Nov 9 22:51:48 2017 (r325621) @@ -144,6 +144,11 @@ struct tidhashhead *tidhashtbl; u_long tidhash; struct rwlock tidhash_lock; +EVENTHANDLER_LIST_DEFINE(thread_ctor); +EVENTHANDLER_LIST_DEFINE(thread_dtor); +EVENTHANDLER_LIST_DEFINE(thread_init); +EVENTHANDLER_LIST_DEFINE(thread_fini); + static lwpid_t tid_alloc(void) { @@ -201,7 +206,7 @@ thread_ctor(void *mem, int size, void *arg, int flags) */ td->td_critnest = 1; td->td_lend_user_pri = PRI_MAX; - EVENTHANDLER_INVOKE(thread_ctor, td); + EVENTHANDLER_DIRECT_INVOKE(thread_ctor, td); #ifdef AUDIT audit_thread_alloc(td); #endif @@ -247,7 +252,7 @@ thread_dtor(void *mem, int size, void *arg) td_softdep_cleanup(td); MPASS(td->td_su == NULL); - EVENTHANDLER_INVOKE(thread_dtor, td); + EVENTHANDLER_DIRECT_INVOKE(thread_dtor, td); tid_free(td->td_tid); } @@ -264,7 +269,7 @@ thread_init(void *mem, int size, int flags) td->td_sleepqueue = sleepq_alloc(); td->td_turnstile = turnstile_alloc(); td->td_rlqe = NULL; - EVENTHANDLER_INVOKE(thread_init, td); + EVENTHANDLER_DIRECT_INVOKE(thread_init, td); umtx_thread_init(td); td->td_kstack = 0; td->td_sel = NULL; @@ -280,7 +285,7 @@ thread_fini(void *mem, int size) struct thread *td; td = (struct thread *)mem; - EVENTHANDLER_INVOKE(thread_fini, td); + EVENTHANDLER_DIRECT_INVOKE(thread_fini, td); rlqentry_free(td->td_rlqe); turnstile_free(td->td_turnstile); sleepq_free(td->td_sleepqueue); Modified: head/sys/kern/subr_eventhandler.c ============================================================================== --- head/sys/kern/subr_eventhandler.c Thu Nov 9 22:26:49 2017 (r325620) +++ head/sys/kern/subr_eventhandler.c Thu Nov 9 22:51:48 2017 (r325621) @@ -38,7 +38,7 @@ __FBSDID("$FreeBSD$"); static MALLOC_DEFINE(M_EVENTHANDLER, "eventhandler", "Event handler records"); -/* List of 'slow' lists */ +/* List of all eventhandler lists */ static TAILQ_HEAD(, eventhandler_list) eventhandler_lists; static int eventhandler_lists_initted = 0; static struct mtx eventhandler_mutex; @@ -64,25 +64,11 @@ eventhandler_init(void *dummy __unused) SYSINIT(eventhandlers, SI_SUB_EVENTHANDLER, SI_ORDER_FIRST, eventhandler_init, NULL); -/* - * Insertion is O(n) due to the priority scan, but optimises to O(1) - * if all priorities are identical. - */ -static eventhandler_tag -eventhandler_register_internal(struct eventhandler_list *list, - const char *name, eventhandler_tag epn) +static struct eventhandler_list * +eventhandler_find_or_create_list(const char *name) { - 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__)); + struct eventhandler_list *list, *new_list; - /* lock the eventhandler lists */ - mtx_lock(&eventhandler_mutex); - - /* Do we need to find/create the (slow) list? */ - if (list == NULL) { /* look for a matching, existing list */ list = _eventhandler_find_list(name); @@ -90,8 +76,8 @@ eventhandler_register_internal(struct eventhandler_lis if (list == NULL) { mtx_unlock(&eventhandler_mutex); - new_list = malloc(sizeof(struct eventhandler_list) + - strlen(name) + 1, M_EVENTHANDLER, M_WAITOK); + new_list = malloc(sizeof(*new_list) + strlen(name) + 1, + M_EVENTHANDLER, M_WAITOK | M_ZERO); /* If someone else created it already, then use that one. */ mtx_lock(&eventhandler_mutex); @@ -101,21 +87,35 @@ eventhandler_register_internal(struct eventhandler_lis } else { CTR2(KTR_EVH, "%s: creating list \"%s\"", __func__, name); list = new_list; - list->el_flags = 0; - list->el_runcount = 0; - bzero(&list->el_lock, sizeof(list->el_lock)); - list->el_name = (char *)list + sizeof(struct eventhandler_list); + TAILQ_INIT(&list->el_entries); + mtx_init(&list->el_lock, name, "eventhandler list", MTX_DEF); + list->el_name = (char *)(list + 1); strcpy(list->el_name, name); TAILQ_INSERT_HEAD(&eventhandler_lists, list, el_link); } } + return (list); +} + +/* + * Insertion is O(n) due to the priority scan, but optimises to O(1) + * if all priorities are identical. + */ +static eventhandler_tag +eventhandler_register_internal(struct eventhandler_list *list, + const char *name, eventhandler_tag epn) +{ + struct eventhandler_entry *ep; + + KASSERT(eventhandler_lists_initted, ("eventhandler registered too early")); + KASSERT(epn != NULL, ("%s: cannot register NULL event", __func__)); + + /* Do we need to find/create the list? */ + if (list == NULL) { + mtx_lock(&eventhandler_mutex); + list = eventhandler_find_or_create_list(name); + mtx_unlock(&eventhandler_mutex); } - if (!(list->el_flags & EHL_INITTED)) { - TAILQ_INIT(&list->el_entries); - mtx_init(&list->el_lock, name, "eventhandler list", MTX_DEF); - atomic_store_rel_int(&list->el_flags, EHL_INITTED); - } - mtx_unlock(&eventhandler_mutex); KASSERT(epn->ee_priority != EHE_DEAD_PRIORITY, ("%s: handler for %s registered with dead priority", __func__, name)); @@ -293,4 +293,23 @@ eventhandler_prune_list(struct eventhandler_list *list } if (pruned > 0) wakeup(list); +} + +/* + * Create (or get the existing) list so the pointer can be stored by + * EVENTHANDLER_LIST_DEFINE. + */ +struct eventhandler_list * +eventhandler_create_list(const char *name) +{ + struct eventhandler_list *list; + + KASSERT(eventhandler_lists_initted, + ("eventhandler list created too early")); + + mtx_lock(&eventhandler_mutex); + list = eventhandler_find_or_create_list(name); + mtx_unlock(&eventhandler_mutex); + + return (list); } Modified: head/sys/sys/eventhandler.h ============================================================================== --- head/sys/sys/eventhandler.h Thu Nov 9 22:26:49 2017 (r325620) +++ head/sys/sys/eventhandler.h Thu Nov 9 22:51:48 2017 (r325621) @@ -51,8 +51,7 @@ struct eventhandler_entry_vimage { struct eventhandler_list { char *el_name; - int el_flags; -#define EHL_INITTED (1<<0) + int el_flags; /* Unused. */ u_int el_runcount; struct mtx el_lock; TAILQ_ENTRY(eventhandler_list) el_link; @@ -72,8 +71,6 @@ typedef struct eventhandler_entry *eventhandler_tag; struct eventhandler_entry *_ep; \ struct eventhandler_entry_ ## name *_t; \ \ - KASSERT((list)->el_flags & EHL_INITTED, \ - ("eventhandler_invoke: running non-inited list")); \ EHL_LOCK_ASSERT((list), MA_OWNED); \ (list)->el_runcount++; \ KASSERT((list)->el_runcount > 0, \ @@ -98,10 +95,41 @@ typedef struct eventhandler_entry *eventhandler_tag; } while (0) /* - * Slow handlers are entirely dynamic; lists are created - * when entries are added to them, and thus have no concept of "owner", - * - * Slow handlers need to be declared, but do not need to be defined. The + * You can optionally use the EVENTHANDLER_LIST and EVENTHANDLER_DIRECT macros + * to pre-define a symbol for the eventhandler list. This symbol can be used by + * EVENTHANDLER_DIRECT_INVOKE, which has the advantage of not needing to do a + * locked search of the global list of eventhandler lists. At least + * EVENTHANDLER_LIST_DEFINE must be be used for EVENTHANDLER_DIRECT_INVOKE to + * work. EVENTHANDLER_LIST_DECLARE is only needed if the call to + * EVENTHANDLER_DIRECT_INVOKE is in a different compilation unit from + * EVENTHANDLER_LIST_DEFINE. If the events are even relatively high frequency + * it is suggested that you directly define a list for them. + */ +#define EVENTHANDLER_LIST_DECLARE(name) \ +extern struct eventhandler_list *_eventhandler_list_ ## name \ + +#define EVENTHANDLER_LIST_DEFINE(name) \ +struct eventhandler_list *_eventhandler_list_ ## name ; \ +static void _ehl_init_ ## name (void * ctx __unused) \ +{ \ + _eventhandler_list_ ## name = eventhandler_create_list(#name); \ +} \ +SYSINIT(name ## _ehl_init, SI_SUB_EVENTHANDLER, SI_ORDER_ANY, \ + _ehl_init_ ## name, NULL); \ + struct __hack + +#define EVENTHANDLER_DIRECT_INVOKE(name, ...) do { \ + struct eventhandler_list *_el; \ + \ + _el = _eventhandler_list_ ## name ; \ + if (!TAILQ_EMPTY(&_el->el_entries)) { \ + EHL_LOCK(_el); \ + _EVENTHANDLER_INVOKE(name, _el , ## __VA_ARGS__); \ + } \ +} while (0) + +/* + * Event handlers need to be declared, but do not need to be defined. The * declaration must be in scope wherever the handler is to be invoked. */ #define EVENTHANDLER_DECLARE(name, type) \ @@ -158,6 +186,7 @@ void eventhandler_deregister_nowait(struct eventhandle eventhandler_tag tag); struct eventhandler_list *eventhandler_find_list(const char *name); void eventhandler_prune_list(struct eventhandler_list *list); +struct eventhandler_list *eventhandler_create_list(const char *name); #ifdef VIMAGE typedef void (*vimage_iterator_func_t)(void *, ...);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201711092251.vA9MpmA1035327>