Date: Thu, 21 Oct 2004 15:52:01 -0700 From: John-Mark Gurney <gurney_j@resnet.uoregon.edu> To: Doug Ambrisko <ambrisko@ambrisko.com> Cc: current@freebsd.org Subject: Re: lio_listio fixes and adding kqueue notification patch Message-ID: <20041021225201.GT22681@funkthat.com> In-Reply-To: <200410201910.i9KJA1wQ017232@ambrisko.com> References: <200410201910.i9KJA1wQ017232@ambrisko.com>
next in thread | previous in thread | raw e-mail | index | archive | help
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."
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20041021225201.GT22681>