Re: lio_listio fixes and adding kqueue notification patch

Doug Ambrisko wrote this message on Wed, Oct 20, 2004 at 12:10 -0700:
> Here's a patch to fix some lio_listio bugs and add kqueue support to it.
> BTW kqueue & aio_read/write is broken and that seems to have happened
> during the locking changes.  I could use some more testers for the
> signal causes.
> 
> http://www.ambrisko.com/doug/listio_kqueue/listio_kqueue.patch
> 
> The summary is:
> kqueue changes:
>       -	kqueue locking removed/broke knote_remove (transformed into
> 	knlist_clear??) which no longer deletes and clean up the
> 	the knotes so they are left hanging around and can break the
> 	next transfer since they are already triggered etc.
>       -	add knlist_delete to restore knote_remove functionality to
> 	totally take it of the system.  When you do an aio_return
> 	(which implies aio_free_entry) the kevent can never trigger
> 	again since the context is lost.

why did you copy so much of knlist_clear into knlist_delete?  why not
expand knlist_clear to optionally do the delete instead of duplicating
the code?  Also, why do you duplicate all the logic in knote_drop into
knlist_delete?  why not make a knote_drop_locked function, and call it
from knlist_delete?

I'll send you a kqueue manpage that I've written so far for your
review...

I don't like removing knotes completely w/o it being returned to the
userland unless there is good reason (like the user did a close on an
fd and expects the knote to go away)...  some people (like me) use knote's
in the kernel to keep track of resources...  so, will it be documented
that this is what happens?

also, why arey ou adding an splbio call?  spl calls in 5.x and 6.x are
if def'd to nothing.. it'd be better to add an XXX comment about requiring
locking...  or GIANT_REQUIRED;

>       -	hack, remove the mtx_assert in knlist_empty so it can be used
> 	to see if there are kqueue's on an aio task.

I don't like this..  The problem I have with this is that between the
time that knlist_empty returns, there could be a knote added...  so any
decision based upon that check is stale?  If you know you aren't going
to be adding anymore knlist_clear (and by the copying knlist_delete) is
safe to call w/o having to check knlist_empty..  They aren't going to
be doing much more work that knlist_empty isn't going to do...  (and
prevents you having to remove the mtx_assert in knlist_empty)..

[aio stuff deleted]

can't really comment on the aio stuff since I don't know it that
well..

-- 
  John-Mark Gurney				Voice: +1 415 225 5579

     "All that I will do, has been done, All that I have, has not."