Date: Thu, 21 Oct 2004 16:48:56 -0700 (PDT) From: Doug Ambrisko <ambrisko@ambrisko.com> To: John-Mark Gurney <gurney_j@resnet.uoregon.edu> Cc: current@freebsd.org Subject: Re: lio_listio fixes and adding kqueue notification patch Message-ID: <200410212348.i9LNmucN008525@ambrisko.com> In-Reply-To: <20041021225201.GT22681@funkthat.com>
next in thread | previous in thread | raw e-mail | index | archive | help
John-Mark Gurney writes: | 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? Since I'm not an expert on this stuff and I didn't want to break the existing code ... such as happened when the locking was done. When I initially did some change I ran into locking issues that were not obvious to figure out. I'm willing to adjust that as you suggest. Didn't want to spend a lot of time it and deal with merge conflicts. This is way I asked for a review to work toward a proper solution. | 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? 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. | 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; Consistancy for back-porting to 4.X ... it already has them. Might as well make it correct in the one minor case that I changed it. It is a minor point. Future work would be to make it giant free like Alfred has started. I brought this code up to 5.X via diff and patch since I did this implementation under 4.X. So that no big thing to change if absolutely needed. | > - 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).. How are the knotes added since the aio call makes them when you submit an aio I/O request and only lives until acknowledged by aio_return. So aio creates, registers and clears the kevent. The user 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 might not get it registered fast enough before it returns. If you happen to register a kevent on the aio op. outside the aio commands you might never get notified since the check is only done on I/O completion which may have already occured. So this isn't a problem. | [aio stuff deleted] | | can't really comment on the aio stuff since I don't know it that | well.. Which is why you broke it and it doesn't work now :-( The lio_listio is pretty busted as is and doesn't work reliably regardless of kevent. I hope we can work to a solution that is stable and works well with aio. The guts are here it more of an issue of how to best integrate it. Doug A.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200410212348.i9LNmucN008525>