NetBSD Problem Report #45441

From www@NetBSD.org  Fri Oct  7 22:53:44 2011
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.netbsd.org [204.152.190.11])
	by www.NetBSD.org (Postfix) with ESMTP id A011F63D4DF
	for <gnats-bugs@gnats.NetBSD.org>; Fri,  7 Oct 2011 22:53:44 +0000 (UTC)
Message-Id: <20111007225343.E288563D4AC@www.NetBSD.org>
Date: Fri,  7 Oct 2011 22:53:43 +0000 (UTC)
From: alnsn@NetBSD.org
Reply-To: alnsn@NetBSD.org
To: gnats-bugs@NetBSD.org
Subject: aio_write and aio_read connected to one pipe don't make progress
X-Send-Pr-Version: www-1.0

>Number:         45441
>Category:       kern
>Synopsis:       aio_write and aio_read connected to one pipe don't make progress
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri Oct 07 22:55:00 +0000 2011
>Last-Modified:  Tue Nov 01 07:55:01 +0000 2011
>Originator:     Alexander Nasonov
>Release:        NetBSD 5.99.56
>Organization:
home sweet home
>Environment:
NetBSD nebeda.localdomain 5.99.56 NetBSD 5.99.56 (GENERIC) #0: Thu Oct  6 13:58:32 UTC 2011  builds@b6.netbsd.org:/home/builds/ab/HEAD/amd64/201110061150Z-obj/home/builds/ab/HEAD/src/sys/arch/amd64/compile/GENERIC amd64
>Description:
If you initialize two aiocb structs to connect to one pipe and call aio_read before aio_write, then aio_error() calls will always be returning EINPROGRESS.

>How-To-Repeat:
#include <err.h>                                                                                                               
#include <errno.h>                                                                                                             
#include <limits.h>                                                                                                            
#include <stdlib.h>                                                                                                            
#include <string.h>                                                                                                            

#include <aio.h>                                                                                                               
#include <unistd.h>                                                                                                            

static void                                                                                                                    
init_pipe_data(int fd[], struct aiocb cb[], char *buf[], size_t buf_sz)                                                        
{                                                                                                                              
        int i;                                                                                                                 

        if (pipe(fd) != 0)                                                                                                     
                err(EXIT_FAILURE, "pipe");                                                                                     

        for (i = 0; i < 2; i++) {                                                                                              
                buf[i] = malloc(buf_sz);                                                                                       
                if (buf[i] == NULL)                                                                                            
                        err(EXIT_FAILURE, "malloc");                                                                           
                buf[i][0] = i;                                                                                                 
                buf[i][buf_sz-1] = CHAR_MAX - i;                                                                               
                memset(&cb[i], 0, sizeof(cb[i]));                                                                              
                cb[i].aio_buf = buf[i];                                                                                        
                cb[i].aio_fildes = fd[i];                                                                                      
                cb[i].aio_offset = 0;                                                                                          
                cb[i].aio_nbytes = buf_sz;                                                                                     
        }                                                                                                                      
}

static void                                                                                                                    
destroy_pipe_data(int fd[], char *buf[])                                                                                       
{                                                                                                                              
        int i;                                                                                                                 

        for (i = 0; i < 2; i++) {                                                                                              
                free(buf[i]);                                                                                                  
                close(fd[i]);                                                                                                  
        }                                                                                                                      
}

int main()                                                                                                                     
{                                                                                                                              
        const size_t sz = PIPE_BUF + 13;                                                                                       

        int res[2] = { EINPROGRESS, EINPROGRESS };                                                                             
        int fd[2];                                                                                                             
        struct aiocb cb[2];                                                                                                    
        char *buf[2];                                                                                                          

        init_pipe_data(fd, cb, buf, sz);                                                                                       

        if (aio_read(&cb[0]) != 0)                                                                                             
                err(EXIT_FAILURE, "aio_read");                                                                                 

        if (aio_write(&cb[1]) != 0)                                                                                            
                err(EXIT_FAILURE, "aio_write");                                                                                

        do {                                                                                                                   
                int i;                                                                                                         
                for (i = 0; i < 2; i++)                                                                                        
                        if (res[i] == EINPROGRESS)                                                                             
                                res[i] = aio_error(&cb[i]);                                                                    
        } while (res[0] == EINPROGRESS || res[1] == EINPROGRESS);                                                              

        destroy_pipe_data(fd, buf);                                                                                            

        return EXIT_SUCCESS;                                                                                                   
}                                                                                                                     

>Fix:
not known

>Audit-Trail:
From: Alexander Nasonov <alnsn@yandex.ru>
To: gnats-bugs@NetBSD.org
Cc: kern-bug-people@netbsd.org, gnats-admin@netbsd.org,
	netbsd-bugs@netbsd.org
Subject: Re: kern/45441: aio_write and aio_read connected to one pipe don't
 make progress
Date: Wed, 12 Oct 2011 21:26:40 +0100

 alnsn@NetBSD.org wrote:
 > If you initialize two aiocb structs to connect to one pipe and call aio_read before aio_write, then aio_error() calls will always be returning EINPROGRESS.
 ...
 >Fix:
 > not known

 The best fix, I think, is return ESPIPE for non-seekable fildes rather
 than silently ignoring aio_offset. This would indirectly solve the issue
 described in this PR.

 Any objections if I commit the patch below? (I'll document ESPIPE as well)
 Alex


 Index: sys/kern/sys_aio.c
 ===================================================================
 RCS file: /cvsroot/src/sys/kern/sys_aio.c,v
 retrieving revision 1.37
 diff -p -u -u -r1.37 sys_aio.c
 --- sys/kern/sys_aio.c	17 Feb 2011 19:02:50 -0000	1.37
 +++ sys/kern/sys_aio.c	12 Oct 2011 20:15:34 -0000
 @@ -360,6 +360,7 @@ aio_process(struct aio_job *a_job)
  	struct proc *p = curlwp->l_proc;
  	struct aiocb *aiocbp = &a_job->aiocbp;
  	struct file *fp;
 +	struct vnode *vp;
  	int fd = aiocbp->aio_fildes;
  	int error = 0;

 @@ -380,6 +381,13 @@ aio_process(struct aio_job *a_job)
  			goto done;
  		}

 +		vp = (struct vnode *)fp->f_data;
 +		if (fp->f_type != DTYPE_VNODE || vp->v_type == VFIFO) {
 +			fd_putfile(fd);
 +			error = ESPIPE;
 +			goto done;
 +		}
 +
  		aiov.iov_base = (void *)(uintptr_t)aiocbp->aio_buf;
  		aiov.iov_len = aiocbp->aio_nbytes;
  		auio.uio_iov = &aiov;
 @@ -427,7 +435,6 @@ aio_process(struct aio_job *a_job)
  		/*
  		 * Perform a file Sync operation
  		 */
 -		struct vnode *vp;

  		if ((error = fd_getvnode(fd, &fp)) != 0)
  			goto done; 

From: David Laight <david@l8s.co.uk>
To: Alexander Nasonov <alnsn@yandex.ru>
Cc: gnats-bugs@NetBSD.org
Subject: Re: kern/45441: aio_write and aio_read connected to one pipe don't make progress
Date: Thu, 13 Oct 2011 21:47:30 +0100

 On Wed, Oct 12, 2011 at 09:26:40PM +0100, Alexander Nasonov wrote:
 > alnsn@NetBSD.org wrote:
 > > If you initialize two aiocb structs to connect to one pipe and call aio_read before aio_write, then aio_error() calls will always be returning EINPROGRESS.
 > ...
 > >Fix:
 > > not known
 > 
 > The best fix, I think, is return ESPIPE for non-seekable fildes rather
 > than silently ignoring aio_offset. This would indirectly solve the issue
 > described in this PR.

 IIRC that breaks historic practise.

 The likely fubar here is that 'aio' is synchronous on a single kernel
 thread.

 	David

 -- 
 David Laight: david@l8s.co.uk

From: Alexander Nasonov <alnsn@yandex.ru>
To: gnats-bugs@NetBSD.org
Cc: kern-bug-people@netbsd.org, gnats-admin@netbsd.org,
	netbsd-bugs@netbsd.org, alnsn@NetBSD.org
Subject: Re: kern/45441: aio_write and aio_read connected to one pipe don't
 make progress
Date: Thu, 13 Oct 2011 22:44:43 +0100

 David Laight wrote:
 >  On Wed, Oct 12, 2011 at 09:26:40PM +0100, Alexander Nasonov wrote:
 >  > The best fix, I think, is return ESPIPE for non-seekable fildes rather
 >  > than silently ignoring aio_offset. This would indirectly solve the issue
 >  > described in this PR.
 >  
 >  IIRC that breaks historic practise.

 Do you have any reference or any more details on this?

 >  The likely fubar here is that 'aio' is synchronous on a single kernel
 >  thread.

 Yes, I know. I wrote the test to prove that the current implementation
 has problems.

 Having separate writer and reader kernel threads would solve the
 problem but it's quite a lot of work.

 Alex

From: Marc Balmer <mbalmer@NetBSD.org>
To: Alexander Nasonov <alnsn@yandex.ru>
Cc: gnats-bugs@NetBSD.org, kern-bug-people@NetBSD.org, gnats-admin@NetBSD.org,
        netbsd-bugs@NetBSD.org
Subject: Re: kern/45441: aio_write and aio_read connected to one pipe don't
 make progress
Date: Tue, 01 Nov 2011 08:53:16 +0100

 Am 12.10.11 22:26, schrieb Alexander Nasonov:
 > alnsn@NetBSD.org wrote:
 >> If you initialize two aiocb structs to connect to one pipe and call aio_read before aio_write, then aio_error() calls will always be returning EINPROGRESS.
 > ...
 >> Fix:
 >> not known
 > 
 > The best fix, I think, is return ESPIPE for non-seekable fildes rather
 > than silently ignoring aio_offset. This would indirectly solve the issue
 > described in this PR.

 IMO, this is just a hack.  aio ist fundamentally broken, or rather not
 working at the moment due to it's design.  There is another aio PR open,
 see there for details.

 > 
 > Any objections if I commit the patch below? (I'll document ESPIPE as well)
 > Alex
 > 
 > 
 > Index: sys/kern/sys_aio.c
 > ===================================================================
 > RCS file: /cvsroot/src/sys/kern/sys_aio.c,v
 > retrieving revision 1.37
 > diff -p -u -u -r1.37 sys_aio.c
 > --- sys/kern/sys_aio.c	17 Feb 2011 19:02:50 -0000	1.37
 > +++ sys/kern/sys_aio.c	12 Oct 2011 20:15:34 -0000
 > @@ -360,6 +360,7 @@ aio_process(struct aio_job *a_job)
 >  	struct proc *p = curlwp->l_proc;
 >  	struct aiocb *aiocbp = &a_job->aiocbp;
 >  	struct file *fp;
 > +	struct vnode *vp;
 >  	int fd = aiocbp->aio_fildes;
 >  	int error = 0;
 >  
 > @@ -380,6 +381,13 @@ aio_process(struct aio_job *a_job)
 >  			goto done;
 >  		}
 >  
 > +		vp = (struct vnode *)fp->f_data;
 > +		if (fp->f_type != DTYPE_VNODE || vp->v_type == VFIFO) {
 > +			fd_putfile(fd);
 > +			error = ESPIPE;
 > +			goto done;
 > +		}
 > +
 >  		aiov.iov_base = (void *)(uintptr_t)aiocbp->aio_buf;
 >  		aiov.iov_len = aiocbp->aio_nbytes;
 >  		auio.uio_iov = &aiov;
 > @@ -427,7 +435,6 @@ aio_process(struct aio_job *a_job)
 >  		/*
 >  		 * Perform a file Sync operation
 >  		 */
 > -		struct vnode *vp;
 >  
 >  		if ((error = fd_getvnode(fd, &fp)) != 0)
 >  			goto done; 


 -- 
   \~~~~~.                The NetBSD Foundation
    \~~~~~'               Marc Balmer, Developer / Marketing
   NetBSD
      \                   mbalmer@NetBSD.org   http://www.NetBSD.org/

NetBSD Home
NetBSD PR Database Search

(Contact us) $NetBSD: query-full-pr,v 1.39 2013/11/01 18:47:49 spz Exp $
$NetBSD: gnats_config.sh,v 1.8 2006/05/07 09:23:38 tsutsui Exp $
Copyright © 1994-2007 The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.