Bug 162732

Created attachment 116503 [details]
Test program - compile with gcc -laio -o aiotest3 aiotest3.c.

Created attachment 116504 [details]
Patch to address this issue.

Here is the documented behaviour:

/* sys_io_cancel:
 *	Attempts to cancel an iocb previously passed to io_submit.  If
 *	the operation is successfully cancelled, the resulting event is
 *	copied into the memory pointed to by result without being placed
 *	into the completion queue and 0 is returned.  May fail with
 *	-EFAULT if any of the data structures pointed to are invalid.
 *	May fail with -EINVAL if aio_context specified by ctx_id is
 *	invalid.  May fail with -EAGAIN if the iocb specified was not
 *	cancelled.  Will fail with -ENOSYS if not implemented.
 */

It's working as it should.  Please also note that in RHEL 4 you need to specify
O_DIRECT when doing AIO to file system files.  Otherwise, it will fall back to
synchronous I/O.

Your fix, to return -ENOSYS, is clearly wrong in this case.  The system call is
implemented.

If you mean the test case doesn't specify O_DIRECT in open(), it was my
oversight while uploading the wrong program. Note that the io_cancel behaves the
same way with/without O_DIRECT. 

Now let's check sys_io_cancel:

   1660
   1661         spin_lock_irq(&ctx->ctx_lock);
   1662         ret = -EAGAIN;
   1663         kiocb = lookup_kiocb(ctx, iocb, key);
   1664         if (kiocb && kiocb->ki_cancel) {
   1665                 cancel = kiocb->ki_cancel;
   1666                 kiocb->ki_users ++;
   1667                 kiocbSetCancelled(kiocb);
   1668         } else
   1669                 cancel = NULL;
   1670         spin_unlock_irq(&ctx->ctx_lock);
   1671
   1672         if (NULL != cancel) {
   1673                 struct io_event tmp;
   1674                 pr_debug("calling cancel\n");
   1675                 memset(&tmp, 0, sizeof(tmp));
   1676                 tmp.obj = (u64)(unsigned long)kiocb->ki_obj.user;
   1677                 tmp.data = kiocb->ki_user_data;
   1678                 ret = cancel(kiocb, &tmp);
   1679                 if (!ret) {
   1680                         /* Cancellation succeeded -- copy the result
   1681                          * into the user's buffer.
   1682                          */
   1683                         if (copy_to_user(result, &tmp, sizeof(tmp)))
   1684                                 ret = -EFAULT;
   1685                 }
   1686         } else
   1687                 printk(KERN_DEBUG "iocb has no cancel operation\n");
   1688
   1689         put_ioctx(ctx);
   1690
   1691         return ret;
   1692 }

Line #1662 specifies ret = -EAGAIN - it is assuming *everything* supports
io_cancel. However, no one would fill in ki_cancel other than usb driver. So the
cancel would always be NULL. so io_cancel() system call would always returns
EAGAIN. 

Now you tell me this is not a bug ? 

The above is pure logical error, regardless O_DIRECT or not. Am I missing
something ? 

sys_io_cancel may fail with -EAGAIN if the iocb specified was not cancelled.

The iocb was not cancelled, and so you get back -EAGAIN.  Even if the file
system did implement a cancel routine, there is no guarantee that you would be
able to cancel the iocb.  No, this is not a bug.  If you like, you can refile
this as an enhancement request.

Who/what is relying on cancellation?

This "may return -EAGAIN" should be done in the "if" clause within #line 1672
and #line 1686. The *current* code logic read as:

"I'll assume io_cancel will return -EAGAIN unless ki_cancel tell me otherwise".
Now, the bug here is that there is *no* ki_cancel to tell you anything.

Jeff, this *IS* a bug :) !!!

I'm reopening this.  The disconnect here was that I interpreted ENOSYS to mean
the system call was not implemented.  Wendy offers the interpretation of
Function not implemented.  She will send a patch upstream for discussion.

Putting bug in NEEDINFO until we hear back from upstream.

Subject: aio-add-enosys-into-sys_io_cancel.patch added to -mm tree
To: wcheng, bcrl, mm-commits.org
From: akpm
Date: Tue, 12 Jul 2005 12:57:48 -0700
                                                                                
The patch titled                                                               
                 aio: add ENOSYS into sys_io_cancel()                          
                                                      has been added to the -mm
tree.                
                                                                         
From:   Wendy Cheng <wcheng>
                                                                                
Note that other than few exceptions, most of the current filesystem and/or
drivers do not have aio cancel specifically defined (kiob->ki_cancel field
is mostly NULL).  However, sys_io_cancel system call universally sets
return code to -EGAIN.  This gives applications a wrong impression that
this call is implemented but just never works.  We have customer inquires
about this issue.

Signed-off-by: S. Wendy Cheng <wcheng>
Acked-by: Benjamin LaHaise <bcrl>
Signed-off-by: Andrew Morton <akpm>
---
                                                                                
 fs/aio.c |    2 +-
 1 files changed, 1 insertion(+), 1 deletion(-)
                                                                                
diff -puN fs/aio.c~aio-add-enosys-into-sys_io_cancel fs/aio.c
--- devel/fs/aio.c~aio-add-enosys-into-sys_io_cancel    2005-07-12
12:57:44.000000000 -0700
+++ devel-akpm/fs/aio.c 2005-07-12 12:57:44.000000000 -0700
@@ -1669,7 +1669,7 @@ asmlinkage long sys_io_cancel(aio_contex
                                ret = -EFAULT;
                }
        } else
-               printk(KERN_DEBUG "iocb has no cancel operation\n");
+               ret = -ENOSYS;
                                                                                
        put_ioctx(ctx);

Forgot to update this issue - received a mail from upstream few weeks ago saying
 ENOSYS is changed to EINVAL and the patch had pushed into Linux's tree.

An advisory has been issued which should help the problem
described in this bug report. This report is therefore being
closed with a resolution of ERRATA. For more information
on the solution and/or where to find the updated files,
please follow the link below. You may reopen this bug report
if the solution does not work for you.

http://rhn.redhat.com/errata/RHSA-2006-0132.html