Date: Mon, 9 Oct 2017 13:47:45 +0200 From: Mateusz Guzik <mjguzik@gmail.com> To: Ian Lepore <ian@freebsd.org> Cc: "src-committers@freebsd.org" <src-committers@freebsd.org>, "svn-src-all@freebsd.org" <svn-src-all@freebsd.org>, "svn-src-head@freebsd.org" <svn-src-head@freebsd.org> Subject: Re: svn commit: r324415 - in head/sys: kern sys Message-ID: <CAGudoHHYX3hWL5bOUAzALZTffk%2BkSZvGuFgiZ0vLmJwMurRf9A@mail.gmail.com> In-Reply-To: <1507485390.86205.323.camel@freebsd.org> References: <201710081733.v98HXnu1094645@repo.freebsd.org> <1507485390.86205.323.camel@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, Oct 08, 2017 at 11:56:30AM -0600, Ian Lepore wrote: > On Sun, 2017-10-08 at 17:33 +0000, Ian Lepore wrote: > > Author: ian > > Date: Sun Oct 8 17:33:49 2017 > > New Revision: 324415 > > URL: https://svnweb.freebsd.org/changeset/base/324415 > > > > Log: > > Add eventhandler notifications for newbus device attach/detach. > > > > [...] > > > > 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... > > About this last point... sys/eventhandler.h is now an ever-growing list > of EVENTHANDLER_DECLARE() statements for events that are unrelated to > each other. I think we are at the point where it's no longer a few > well-known "standard system event queues", it's turning into a mess. > > My first thought was to add these to bus.h because they're bus events. > But you have to include eventhandler.h to use EVENTHANDLER_DECLARE, > and I didn't want to pull it (and its dependencies) into bus.h. > The current implementation has to be retired and significant tinkering with it is imho a waste of time. It is weirdly inefficient both single and multithreaded. Vast majority (if not all) lists are statically defined. And yet invocing the handlers starts with following a linked list and strcmping to find the instance. All this under a global eventhanadler lock which starts showing up. Most registered handlers for any paritcular event are never going to get unregistered. The current code locks the list do the traversal, which again is slow and starts showing up in lock profiles. So the proposal is to partially unscrew the current code so that it largely gets out of the way and then implement something new and migrate evertyhing on head to use it. A new set of eventhnadler declarations can be added (_STATIC?) which provides a known at compilation time symbol. Then a paired invocation macro to just lock the handler list without the need to look it up. As for the new mechanism, I don't have anything fleshed out, but in general it would do away with excessive macro use. handlers which once registered never get removed get a lockless treatment for traveral. say they also get implemented as a linked list. Invocations would do a lockless traversal of the list. If there is an entry, it is fully installed and never goes away. Registrations would lock the list and add an entry in traversal-friendly manner updating the '->next' pointer at the lsat step. Later this can be made even nicer with a table, but that will require a little bit of trickery to make it freeable. With this in place there will be no central "here are the handlers" header. That said, I have no immediate plans working on this, but I'll eventually get around to it unless someone beats me or has a better idea. -- Mateusz Guzik <mjguzik gmail.com>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAGudoHHYX3hWL5bOUAzALZTffk%2BkSZvGuFgiZ0vLmJwMurRf9A>