Re: lio_listio fixes and adding kqueue notification patch

Doug Ambrisko wrote this message on Fri, Oct 22, 2004 at 08:42 -0700:
> John-Mark Gurney writes:
> | Doug Ambrisko wrote this message on Thu, Oct 21, 2004 at 16:48 -0700:
> | > John-Mark Gurney writes:
> | > | 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?
> | > 
> | > When an aio operation is complete it is dead and it can't happen
> | > again.  The aio_return is acknowledgement that the transaction is 
> | > complete and can't happen again like a close (the only thing different
> | > from a close is that other I/O can still happen on the fd.  If you leave it
> | > around then the next aio op gets the prior knote done when it hasn't
> | > done anything.  Bad things happen.  This is documented in the aio 
> | > usage stuff.
> | 
> | Ok, we should mark the kevent as EV_ONESHOT, then once the event has
> | finished, we could simply detached the knote, and not need to clean it
> | up anymore...
> 
> Yes you could do that.  There are some caveats.  The user could 
> register a kevent on that aio operation that isn't one shot and
> someone could do a aio_error check and see if it is ready then
> aio_return without doing a the kevent check.  The last case orphans
> the kevent and it would trigger when the operation is gone giving
> you a bogus event.  Also the aio returned by the now stale kevent
> would be a reference to a bad pointer that could have been free'ed 
> up after the aio_return leading to an application seg fault :-(  Before 
> your change the aio_return cleaned it up no matter what and I think 
> that is more robust.  aio_return should be viewed as a "close" event
> and all kernel structures reclaimed that are associated with that aio
> operation no matter what.  I wouldn't say aio is simple but it can
> be very usefull tool.

Oh, there is also the method of just setting KN_DETACHED on the knote
and leaving it in the queue.. once you set KN_DETACHED (with the knlist
lock held, i.e. object lock), then the filtopt's detach function is not
called...

/me just realized that we should check KN_DETACHED at f_event time,
and always return true to prevent calling the f_event on an inactive
knote.

Ok, yes, now that I think of it, I do agree that aio_return should
remove the kevent, since it's the same as a close for an fd..  The
user has already disposed of the event, no more need to do that..

> | > then listens to them.  If there are no knotes then you are going to 
> | > use the signal method of delivery.  Mostly it is a panic saver of 
> | > which I hit a few.  Strictly it isn't needed but it doesn't hurt.  
> | > kqueue & aio is special.  Have you read the EVFILT_AIO part of 
> | > man kqueue?  It has to be registered on the aio operation or you
> | 
> | actually, we need to cut out the second paragraph.. hmmm. this is
> | funny, EV_FLAG1 is not restricted to the kernel, which means that the
> | userland can register an AIO request...
> 
> I agreed we should remove the alternative method.  I didn't implement
> it for the EVFILT_LIO case.  I will need to update the man page
> for the EVFILT_LIO case.

You know, there isn't even a way to restrict the alternative method..

> | sure...  we'll find a solution... do you have some good test programs?
> 
> Attached is aio.c and lio.c using kqueue.  It can work on a file
> or a raw disk device.  The raw disk device is more interesting in
> the lio_listio case since it can only process a few requests in the
> fast path method then it has to queue up tasks for the aio_daemon
> to do the rest so you end up stressing a bunch of different code paths
> that do the same thing.  Aio can do the same but the accounting is
> deferent and easier.  I have commented out code that will create
> register a kevent filter.  Note that making a one shot like I did
> isn't strictly needed.  I think we shouldn't depend on that or
> bad things will happen.  I'd like to see a more robust solution.  I 
> know aio has issues but it would be nice to not introduce new ones
> versus how the code worked before.  Otherwise it could break legacy
> applications.

Cool, I'll take a look at it..

> Note to make lio.c you need to install the event header or point lio.c 
> to it since EVFILT_LIO is new.  There is no header change for aio.c.
> 
> We have an app. here that really stresses the system which is our 
> acid test.  It has been running on 4.X for a long time.
> 
> BTW this could show my own ignorance since I don't think this exists.
> What would be nice is if a signal could be associated with kevents 
> happening.  The idea would be that instead of polling for kevents that 
> a signal handler could be setup to notify the process when a kevent 
> just happened.  The usage would be to set a tell kqueue to signal a 
> certain signal if any kevent events occur.  This would allow a program
> not to be just focused on I/O but allow a bunch of computation to 
> happen then deal with I/O without polling for it.  Not every app.
> is just a "bit pusher".

Alfred Perlstein started work on this area, but we need to revisit it now
since kqueue locking is done...   look at the FIOASYNC in kqueue_ioctl..
we need to solve the recursive lock issue, which could now be as simple
as doing a work task and moving it to the thread I used to solve the
kq in kq problem...  Alfred put a good comment there..

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

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