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

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

[Suparna Bhattacharya]

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

-- 
Suparna Bhattacharya (suparna@in.ibm.com)
Linux Technology Center
IBM Software Lab, India

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

[Andi Kleen]

Suparna Bhattacharya <suparna@in.ibm.com> writes:
> 
> 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 !

I looked over the patches and they look all fine to me.

-Andi

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

[Sébastien Dugué]

[-- Attachment #1: Type: text/plain, Size: 3283 bytes --]

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.

  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
  
------------------------------------------------------

[-- Attachment #2: aio-retry-iowait-fix --]
[-- Type: application/octet-stream, Size: 1139 bytes --]


  When an application is mixing sync and async IO it ends up crashing
later on in the sync path when calling lock_page_slow as the io_wait
queue has been set to NULL in a previous AIO request.

  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:

	- modify-wait-bit-action-args
	- lock_page_wait
	- init-wait-bit-key
	- tsk-default-io-wait
	- aio-wait-bit
	- aio-wait-page

and could be folded into aio-wait-bit.

Signed-off-by: Sébastien Dugué <sebastien.dugue@bull.net>


 aio.c |    2 +-
 1 files changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6.12/fs/aio.c
===================================================================
--- linux-2.6.12.orig/fs/aio.c	2005-06-30 17:48:47.000000000 +0200
+++ linux-2.6.12/fs/aio.c	2005-06-30 17:48:57.000000000 +0200
@@ -714,7 +714,7 @@
 	BUG_ON(!is_sync_wait(current->io_wait));
 	current->io_wait = &iocb->ki_wait.wait;
 	ret = retry(iocb);
-	current->io_wait = NULL;
+	current->io_wait = &current->__wait.wait;
 
 	if (-EIOCBRETRY != ret) {
  		if (-EIOCBQUEUED != ret) {

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

[Suparna Bhattacharya]

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
>   
> ------------------------------------------------------



-- 
Suparna Bhattacharya (suparna@in.ibm.com)
Linux Technology Center
IBM Software Lab, India