Skip site navigation (1)Skip section navigation (2)
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>