Bug 162732 - io_cancel doesn't work properly
io_cancel doesn't work properly
Status: CLOSED ERRATA
Product: Red Hat Enterprise Linux 4
Classification: Red Hat
Component: kernel (Show other bugs)
4.0
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jeff Moyer
Brian Brock
:
Depends On:
Blocks: 168429
  Show dependency treegraph
 
Reported: 2005-07-08 00:19 EDT by Wendy Cheng
Modified: 2010-10-21 23:08 EDT (History)
2 users (show)

See Also:
Fixed In Version: RHSA-2006-0132
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-03-07 14:16:16 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
Test program - compile with gcc -laio -o aiotest3 aiotest3.c. (2.29 KB, text/plain)
2005-07-08 00:21 EDT, Wendy Cheng
no flags Details
Patch to address this issue. (383 bytes, patch)
2005-07-08 00:22 EDT, Wendy Cheng
no flags Details | Diff

  None (edit)
Description Wendy Cheng 2005-07-08 00:19:08 EDT
From Bugzilla Helper:
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.7) Gecko/20050427 Red Hat/1.7.7-1.1.3.4

Description of problem:
Other than usb/gadget driver, none of the current filesystems and/or drivers that support aio has io_cancel implemented. However, the io_cancel() system call (sys_io_cancle) somehow sets return code to EAGAIN. This gives our customers a false impression that io_cancel() is supported (implemented) but just never works. 




Version-Release number of selected component (if applicable):
linux-2.6.9-11.25.EL

How reproducible:
Always

Steps to Reproduce:
Run the uploaded test program.


Additional info:
Comment 1 Wendy Cheng 2005-07-08 00:21:19 EDT
Created attachment 116503 [details]
Test program - compile with gcc -laio -o aiotest3 aiotest3.c.
Comment 2 Wendy Cheng 2005-07-08 00:22:03 EDT
Created attachment 116504 [details]
Patch to address this issue.
Comment 3 Jeff Moyer 2005-07-08 13:12:04 EDT
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.
Comment 4 Wendy Cheng 2005-07-08 13:42:22 EDT
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 ? 
Comment 5 Wendy Cheng 2005-07-08 13:45:25 EDT
The above is pure logical error, regardless O_DIRECT or not. Am I missing
something ? 
Comment 6 Jeff Moyer 2005-07-08 13:47:36 EDT
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?
Comment 7 Wendy Cheng 2005-07-08 13:56:27 EDT
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 :) !!!
Comment 8 Jeff Moyer 2005-07-08 14:08:46 EDT
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.
Comment 10 Jeff Moyer 2005-07-08 14:19:46 EDT
Putting bug in NEEDINFO until we hear back from upstream.
Comment 11 Wendy Cheng 2005-07-27 11:13:19 EDT
Subject: aio-add-enosys-into-sys_io_cancel.patch added to -mm tree
To: wcheng@redhat.com, bcrl@kvack.org, mm-commits@vger.kernel.org
From: akpm@osdl.org
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@redhat.com>
                                                                                
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@redhat.com>
Acked-by: Benjamin LaHaise <bcrl@kvack.org>
Signed-off-by: Andrew Morton <akpm@osdl.org>
---
                                                                                
 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);

Comment 12 Wendy Cheng 2005-10-05 10:38:01 EDT
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.
Comment 22 Red Hat Bugzilla 2006-03-07 14:16:16 EST
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

Note You need to log in before you can comment on or make changes to this bug.