[Patch] kqueue(2) <-> procdesc(4): EVFILT_PROCDESC

[Ed Schouten]

Hi there,

First of all, thanks for the quick review. Well appreciated!

On 5 April 2014 17:36, Konstantin Belousov <kostikbel at gmail.com> wrote:
> procdesc_kqops_event() should mimic the filt_proc() much more closely
> than you do.  In particular:
> - I do not see why do you check for NOTE_EXIT in kqops_event().
>   If enforcing NOTE_EXIT anywhere, procdesc_kqfilter() is more natural place
>   to put the temporal restriction.  I do think that signalling and execing
>   are equally useful as exiting notifications, but this is for future.
> - The whole structure of filt_proc(), which copies masked sfflags to
>   fflags is easier to extend later.  It seems that just repeating
>   most of the code from filt_proc() would do what needed.

My plan was initially to mimick filt_proc() almost entirely and just
use KNOTE_LOCKED(..., NOTE_EXIT) just like EVFILT_PROC does, but
unfortunately this doesn't seem possible, because if we went down this
path, there would be no way for us to activate the knote in
procdesc_kqfilter() immediately, as KNOTE_ACTIVATE() is not available
outside of kern_event.c.

This is why I just changed the code, so that procdesc_kqops_event() is
almost literally a copy of filt_proc(), which the difference that it
tests against PDF_EXITED instead of using the hint. While there, I
made some style fixes to filt_proc(). Thoughts?

http://80386.nl/pub/kqueue-evfilt-procdesc.txt (refresh)

> I think you must do knlist_clear() before knlist_destroy().

If I understand kqueue internals properly, I am pretty certain that
knlist_clear() is not needed in this case, for two reasons:

- knlist_clear man page entry: "This function must not be used when
f_isfd is set in struct filterops, as the td argument of fdrop() will
be NULL." Other file descriptor types also don't seem to call this
function.

- struct procdesc is only deallocated if the file descriptor is
closed. This means that the knlist is guaranteed to be empty at this
point in time. knlist_destroy() also has an assertion for this, which
is good.

-- 
Ed Schouten <ed at 80386.nl>