From owner-freebsd-current@FreeBSD.ORG Fri Oct 22 07:10:29 2004 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 20A2516A4CE for ; Fri, 22 Oct 2004 07:10:29 +0000 (GMT) Received: from mail3.speakeasy.net (mail3.speakeasy.net [216.254.0.203]) by mx1.FreeBSD.org (Postfix) with ESMTP id B628443D31 for ; Fri, 22 Oct 2004 07:10:28 +0000 (GMT) (envelope-from jmg@hydrogen.funkthat.com) Received: (qmail 7947 invoked from network); 22 Oct 2004 07:10:28 -0000 Received: from gate.funkthat.com (HELO hydrogen.funkthat.com) ([69.17.45.168]) (envelope-sender ) by mail3.speakeasy.net (qmail-ldap-1.03) with SMTP for ; 22 Oct 2004 07:10:28 -0000 Received: from hydrogen.funkthat.com (ufwdix@localhost.funkthat.com [127.0.0.1])i9M7AQlb052904; Fri, 22 Oct 2004 00:10:27 -0700 (PDT) (envelope-from jmg@hydrogen.funkthat.com) Received: (from jmg@localhost) by hydrogen.funkthat.com (8.12.10/8.12.10/Submit) id i9M7AQIk052898; Fri, 22 Oct 2004 00:10:26 -0700 (PDT) Date: Fri, 22 Oct 2004 00:10:26 -0700 From: John-Mark Gurney To: Doug Ambrisko Message-ID: <20041022071026.GW22681@funkthat.com> Mail-Followup-To: Doug Ambrisko , current@freebsd.org References: <20041021225201.GT22681@funkthat.com> <200410212348.i9LNmucN008525@ambrisko.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <200410212348.i9LNmucN008525@ambrisko.com> User-Agent: Mutt/1.4.1i X-Operating-System: FreeBSD 4.2-RELEASE i386 X-PGP-Fingerprint: B7 EC EF F8 AE ED A7 31 96 7A 22 B3 D8 56 36 F4 X-Files: The truth is out there X-URL: http://resnet.uoregon.edu/~gurney_j/ X-Resume: http://resnet.uoregon.edu/~gurney_j/resume.html cc: current@freebsd.org Subject: Re: lio_listio fixes and adding kqueue notification patch X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list Reply-To: John-Mark Gurney List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 22 Oct 2004 07:10:29 -0000 Doug Ambrisko wrote this message on Thu, Oct 21, 2004 at 16:48 -0700: > 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. np... > | 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. 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... > | 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 ahh, ok. yeh, I see that now... > 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... > 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. yeh, definately need to fix this... > | [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. sure... we'll find a solution... do you have some good test programs? -- John-Mark Gurney Voice: +1 415 225 5579 "All that I will do, has been done, All that I have, has not."