Re: [PATCH 0/6] Integrate AIO with wait-bit based filtered wakeups

Suparna Bhattacharya
2005-07-01T07:28:27Z

On Thu, Jun 30, 2005 at 05:49:00PM +0200, Sébastien Dugué wrote:
> On Mon, 2005-06-20 at 21:31 +0530, Suparna Bhattacharya wrote:
> > On Mon, Jun 20, 2005 at 05:31:54PM +0530, Suparna Bhattacharya wrote:
> > > Since AIO development is gaining momentum once again, ocfs2 and
> > > samba both appear to be using AIO, NFS needs async semaphores etc,
> > > there appears to be an increase in interest in straightening out some
> > > of the pending work in this area. So this seems like a good
> > > time to re-post some of those patches for discussion and decision.
> > >
> > > Just to help sync up, here is an initial list based on the pieces
> > > that have been in progress with patches in existence (please feel free
> > > to add/update ones I missed or reflected inaccurately here):
> > >
> > > (1) Updating AIO to use wait-bit based filtered wakeups (me/wli)
> > > Status: Updated to 2.6.12-rc6, needs review
> >
> > Here is a little bit of background on the motivation behind this set of
> > patches to update AIO for filtered wakeups:
> >
> > (a) Since the introduction of filtered wakeups support and
> > the wait_bit_queue infrastructure in mainline, it is no longer
> > sufficient to just embed a wait queue entry in the kiocb
> > for AIO operations involving filtered wakeups.
> > (b) Given that filesystem reads/writes use filtered wakeups underlying
> > wait_on_page_bit, fixing this becomes a pre-req for buffered
> > filesystem AIO.
> > (c) The wait_bit_queue infrastructure actually enables a cleaner
> > implementation of filesystem AIO because it already provides
> > for an action routine intended to allow both blocking and
> > non-blocking or asynchronous behaviour.
> >
> > As I was rewriting the patches to address this, there is one other
> > change I made to resolve one remaining ugliness in my earlier
> > patchsets - special casing of the form
> > if (wait == NULL) wait = &local_wait
> > to switch to a stack based wait queue entry if not passed a wait
> > queue entry associated with an iocb.
> >
> > To avoid this, I have tried biting the bullet by including a default
> > wait bit queue entry in the task structure, to be used instead of
> > on-demand allocation of a wait bit queue entry on stack.
> >
> > All in all, these changes have (hopefully) simplified the code,
> > as well as made it more up-to-date. Comments (including
> > better names etc as requested by Zach) are welcome !
> >
> > Regards
> > Suparna
> >
>
> Just found a bug in aio_run_iocb: after having called the retry
> method for the iocb, current->io_wait is RESET to NULL. While this
> does not affect applications doing only AIO, applications
> mixing sync and async IO (MySQL for example) end up crashing
> later on in the sync path when calling lock_page_slow as the io_wait
> queue is NULL.

Yes this is a problem. I had spotted it too but the implications hadn't
registered well enough for prompt fix - thanks for the patch.

Regards
Suparna

>
> Therefore after the retry method has been called the task io_wait
> queue should be set to the default queue.
>
> This patch applies over Suparna's wait-bit patchset and maybe should
> be folded into aio-wait-bit.
>
> Sébastien.
>
> --
> ------------------------------------------------------
>
> Sébastien Dugué BULL/FREC:B1-247
> phone: (+33) 476 29 77 70 Bullcom: 229-7770
>
> mailto:sebastien.dugue@bull.net
>
> Linux POSIX AIO: http://www.bullopensource.org/posix
>
> ------------------------------------------------------