Re: [PATCH 2.6-bk] aio not returning error code(?)

Re: [PATCH 2.6-bk] aio not returning error code(?)

[Benjamin LaHaise]

On Wed, Jul 07, 2004 at 02:42:44PM +0100, Anton Altaparmakov wrote:
> --- bklinux-2.6/fs/aio.c	2004-07-01 11:19:35.000000000 +0100
> +++ bklinux-2.6/fs/aio.c.new	2004-07-07 14:26:19.445631304 +0100
> @@ -1086,7 +1086,7 @@ int fastcall io_submit_one(struct kioctx
>  	if (likely(-EIOCBQUEUED == ret))
>  		return 0;
>  	aio_complete(req, ret, 0);	/* will drop i/o ref to req */
> -	return 0;
> +	return ret;

That's wrong: you now get 2 results for the same operation -- an error on 
the submit, and an event with a return code.  In order for the user code 
to do the right thing, you must only get one or the other.  If io_submit 
fails for a particular iocb, there must be no event returned.

		-ben
-- 
"Time is what keeps everything from happening all at once." -- John Wheeler

Re: [PATCH 2.6-bk] aio not returning error code(?)

[Benjamin LaHaise]

Hello Anton,

On Thu, Jul 08, 2004 at 01:56:24PM +0100, Anton Altaparmakov wrote:
> You are saying that this is wrong and that no error code is returned
> from io_submit despite its manpage saying that it is so one of the two
> must be wrong, no?

Either or.  Yes, the man page needs to talk about early vs late errors 
(remember that any early error for a given iocb is valid late), or your 
patch needs to be fixed.  Something like the following would be the 
right way of fixing it -- it doesn't report the same error twice (both 
early and late) as your patch did, but it's not clear which was is better.  
There are good arguements in favour of both early and late error 
reporting, and late errors must always be handled by the application.  I 
hope this clears things up to the level of mud. ;-)  Cheers,

		-ben
-- 
"Time is what keeps everything from happening all at once." -- John Wheeler


--- fs/aio.c.orig	2004-07-08 09:18:02.208534208 -0400
+++ fs/aio.c	2004-07-08 09:23:42.346825328 -0400
@@ -1044,6 +1044,8 @@
 		if (file->f_op->aio_read)
 			ret = file->f_op->aio_read(req, buf,
 					iocb->aio_nbytes, req->ki_pos);
+		else
+			goto out_put_req;
 		break;
 	case IOCB_CMD_PWRITE:
 		ret = -EBADF;
@@ -1059,20 +1061,27 @@
 		if (file->f_op->aio_write)
 			ret = file->f_op->aio_write(req, buf,
 					iocb->aio_nbytes, req->ki_pos);
+		else
+			goto out_put_req;
 		break;
 	case IOCB_CMD_FDSYNC:
 		ret = -EINVAL;
 		if (file->f_op->aio_fsync)
 			ret = file->f_op->aio_fsync(req, 1);
+		else
+			goto out_put_req;
 		break;
 	case IOCB_CMD_FSYNC:
 		ret = -EINVAL;
 		if (file->f_op->aio_fsync)
 			ret = file->f_op->aio_fsync(req, 0);
+		else
+			goto out_put_req;
 		break;
 	default:
 		dprintk("EINVAL: io_submit: no operation provided\n");
 		ret = -EINVAL;
+		goto out_put_req;
 	}
 
 	aio_put_req(req);	/* drop extra ref to req */

Re: [PATCH 2.6-bk] aio not returning error code(?)

[Suparna Bhattacharya]

On Thu, Jul 08, 2004 at 01:56:24PM +0100, Anton Altaparmakov wrote:
> On Wed, 2004-07-07 at 23:33, Benjamin LaHaise wrote:
> > On Wed, Jul 07, 2004 at 02:42:44PM +0100, Anton Altaparmakov wrote:
> > > --- bklinux-2.6/fs/aio.c	2004-07-01 11:19:35.000000000 +0100
> > > +++ bklinux-2.6/fs/aio.c.new	2004-07-07 14:26:19.445631304 +0100
> > > @@ -1086,7 +1086,7 @@ int fastcall io_submit_one(struct kioctx
> > >  	if (likely(-EIOCBQUEUED == ret))
> > >  		return 0;
> > >  	aio_complete(req, ret, 0);	/* will drop i/o ref to req */
> > > -	return 0;
> > > +	return ret;
> > 
> > That's wrong: you now get 2 results for the same operation -- an error on 
> > the submit, and an event with a return code.  In order for the user code 
> > to do the right thing, you must only get one or the other.  If io_submit 
> > fails for a particular iocb, there must be no event returned.
> 
> I see.  Perhaps the man page for io_sbumit(2) needs to be fixed or at
> least clarified.  It states:
> 
> "ERRORS
>        EINVAL The aio_context specified by ctx_id is invalid.  nr
>               is less than 0. The iocb at *iocbpp[0] is not prop­
>               erly initialized, or the operation specified is in­
>               valid for the file descriptor in the iocb."
> 
> Not a single file system in the kernel implements aio_fsync and
> according to the man page for io_submit as far as I understand what is
> written there if I call io_submit for an aio_fsync operation I should
> get an error code EINVAL returned.
> 
> You are saying that this is wrong and that no error code is returned
> from io_submit despite its manpage saying that it is so one of the two
> must be wrong, no?

Actually the semantics specified in the man page is reasonable. It is
just that you need to make sure that aio_complete() is not called 
for the case where we return an error from io_submit.

In my patchset, I broke up io_submit_one into two portions, aio_setup_iocb,
which checks for these error cases and hence returns rightaway, and then
the actual io submission path, which returns 0, and reports any errors 
from the fop aio routines via aio_complete(). So aio_fsync for example,
would return -EINVAL from io_submit.

Does that make sense ?

Regards
Suparna

> -- 
> Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
> Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
> Linux NTFS maintainer / IRC: #ntfs on irc.freenode.net
> WWW: http://linux-ntfs.sf.net/, http://www-stu.christs.cam.ac.uk/~aia21/
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-aio' in
> the body to majordomo@kvack.org.  For more info on Linux AIO,
> see: http://www.kvack.org/aio/
> Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>

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