Date: Thu, 15 Apr 2004 15:39:20 -0700 From: John-Mark Gurney <gurney_j@efn.org> To: "Brian F. Feldman" <green@freebsd.org> Cc: freebsd-arch@freebsd.org Subject: locking down kqueue (was some other completely unrelated topic) Message-ID: <20040415223920.GT567@funkthat.com> In-Reply-To: <200404152036.i3FKaUd5018165@green.homeunix.org> References: <gurney_j@efn.org> <20040409180106.GM567@funkthat.com> <200404152036.i3FKaUd5018165@green.homeunix.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Brian F. Feldman wrote this message on Thu, Apr 15, 2004 at 16:36 -0400:
> John-Mark Gurney <gurney_j@efn.org> wrote:
> > How does this sound to people? I have some code starting to implement
> > this, but I haven't gotten very far with it yet...
>
> You know, now I think Seigo's right on the money that due to the nature of
> the recursion of kqueue's current implementation, it's impossible to get
> right with this train of thought. So, let's redesign:
> * The kqueue object the user controls needs a mutex.
> * The lists (selinfo, mostly) that knotes are on need locking.
> * The filterops that kqueue calls out MUST be called with some
> kind of locking on the lists that the kqueue is on, and the
> user MUST be able to grab any kind of lock from inside a
> filterop.
You need to be more specific on this.. which filterops should be allowed
to grab any locks? In my opinion, filterops should only be allowed to
grab a limited number of locks and these are the object lock and the
kqueue (list) locks as necessary...
> * At some point the object being observed must call back into
> kqueue to add itself. We'll end up getting deadlocks if the
> locks kqueue holds are not the ones required to add the
> object to klists and held when we do the f_attach().
Yes, we need to prevent lock order inversion..
> * Anything that calls KNOTE() or KNOTE_ACTIVATE() directly
> will end up recursing back on itself if we don't convert
> it to putting the new event on a work queue. How
> filt_procattach() calls KNOTE_ACTIVATE() or filt_proc()
> calls kqueue_register() is a very good example.
Hmmm. I'm going to have to mull on the filt_proc problem a bit..
> * The functions that need to be exported are:
> * KNOTE()/KNOTE_ACTIVATE() <- put on a workqueue
> * kqueue_register() <- put on a workqueue
> * knote klist/(kn_selnext) linking and unlinking
> * knote klist/(kn_selnext) is disappearing
> The last two are the only ones that are not called recursively
> and should have easy locking semantics.
Personally, I'd prefer to invert the logic, and have linking/unlinking
and disappearing done via a work queue, and KNOTE/kqueue_register done
in line if possible...
It's also difficult because we need to optimize for both cases of long
existing events, and ONE_SHOT events where we are constantly
adding/removing events...
The reason I say this is because I have a visit for a webserver that
uses multiple processors but a single kqueue to handle events, and if
we use ONE_SHOT, then we are guarnateed that we are notified of each
event once... We need to make sure that the work queue will not be a
significant problem...
> Nothing is currently designed to work with anything even remotely not
> looking like spl(), so we have to either flatten it out (using workqueues)
> or change semantics so that when KNOTE() is called it acts like the closure
> that we pretend it is. Of course, the easy way to do this is with a worker
> queue/condvar/mutex/thread. What other ways do we have available to turn
> KNOTE() into a closure, bearing in mind that the entire point of the
> mechanism is that there is no memory allocation at the time of event
> generation -- only when events are defined (by the user or recursively by
> other events).
The proc case should be treated special as it is an [ab]use of the kevent
system with following children... I'm looking at it more..
--
John-Mark Gurney Voice: +1 415 225 5579
"All that I will do, has been done, All that I have, has not."
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20040415223920.GT567>