On Tue, Apr 02, 2013 at 05:35:50PM -0400, Theodore Ts'o wrote: > On Thu, Mar 21, 2013 at 09:35:54AM -0700, Kent Overstreet wrote: > > From: Benjamin LaHaise <[email protected]> > > > > The recent changes overhauling fs/aio.c introduced a bug that results in the > > kioctx not being freed when outstanding kiocbs are cancelled at exit_aio() > > time. Specifically, a kiocb that is cancelled has its completion events > > discarded by batch_complete_aio(), which then fails to wake up the process > > stuck in free_ioctx(). Fix this by removing the event suppression in > > batch_complete_aio() and modify the wait_event() condition in free_ioctx() > > appropriately. > > Once you remove the event suppression, then it means that every single > cancelled AIO will result in ki_ctx->reqs_available getting double > incremented, right?
I'm not sure where you're seeing the double increment... Previously, when we were supressing the events we needed to increment reqs_available to account for the fact that we wouldn't be doing a put_reqs_available() when reaping the io_event. I think the commit description could've been a bit better - this patch is changing the behaviour of cancellation, and it makes more sense in context with some of the other cancellation patches - instead of returning the io_event via io_cancel(), we're returning it via io_getevents() as it would be normally. So all removing the event supression is doing is causing the io_events from cancelled kiocbs to be handled just like any other io_event. > But reqs_available gets used in more places than > just free_ioctx(). It also gets used (for example) by > get_reqs_available(), which in turn gets used by aio_get_req() to > decide whether or not it's safe to allocate another aio_request. > Since reqs_available is getting double allocated, won't we end up > allowing more AIO requests to be issued --- more than we would have > room in the ring? > > Am I missing something? You're right about how reqs_available is used, but unless I'm missing something the accounting is correct. Maybe we should go over it together? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
