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

["Sébastien Dugué" <sebastien.dugue@bull.net>]

[-- 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) {