aio updates

Please apply.

# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher. # This patch includes the following deltas:
# ChangeSet 1.850 -> 1.851
# fs/aio.c 1.22 -> 1.23
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 02/10/14 [email protected] 1.851

# aio updates
#     fix uninitialized variable causing incorrect timeout
#     add support for IO_CMD_NOOP
#     make sys_io_cancel(), not cancel method, initialize most of returned result
#     minor aio_cancel_all() optimization
#     fix a debug printk
# --------------------------------------------
#
diff -Nru a/fs/aio.c b/fs/aio.c
--- a/fs/aio.c	Mon Oct 14 16:51:45 2002
+++ b/fs/aio.c	Mon Oct 14 16:51:45 2002
@@ -248,7 +248,7 @@
 	write_unlock(&mm->ioctx_list_lock);

dprintk("aio: allocated ioctx %p[%ld]: mm=%p mask=0x%x\n",
- ctx, ctx->user_id, current->mm, ctx->ring_info.ring->nr);
+ ctx, ctx->user_id, current->mm, ctx->ring_info.nr);
return ctx;

out_cleanup:
@@ -281,12 +281,12 @@
struct kiocb *iocb = list_kiocb(pos); list_del_init(&iocb->ki_list); cancel = iocb->ki_cancel;
- if (cancel)
+ if (cancel) {
iocb->ki_users++;
- spin_unlock_irq(&ctx->ctx_lock);
- if (cancel)
+ spin_unlock_irq(&ctx->ctx_lock);
cancel(iocb, &res);
- spin_lock_irq(&ctx->ctx_lock);
+ spin_lock_irq(&ctx->ctx_lock);
+ }
}
spin_unlock_irq(&ctx->ctx_lock);
}
@@ -845,13 +845,13 @@

/* End fast path */

+ init_timeout(&to);
if (timeout) {
struct timespec ts;
ret = -EFAULT;
if (unlikely(copy_from_user(&ts, timeout, sizeof(ts))))
goto out;

- init_timeout(&to);
set_timeout(start_jiffies, &to, &ts);
}

@@ -1073,6 +1073,9 @@
if (file->f_op->aio_fsync)
ret = file->f_op->aio_fsync(req, 0);
break;
+ case IOCB_CMD_NOOP:
+ aio_complete(req, iocb->aio_buf, iocb->aio_offset);
+ return 0;
default:
dprintk("EINVAL: io_submit: no operation provided\n"); ret = -EINVAL;
@@ -1197,6 +1200,9 @@
if (NULL != cancel) {
struct io_event tmp;
printk("calling cancel\n");
+ memset(&tmp, 0, sizeof(tmp));
+ tmp.obj = (u64)(unsigned long)kiocb->ki_user_obj;
+ tmp.data = kiocb->ki_user_data;
ret = cancel(kiocb, &tmp);
if (!ret) {
/* Cancellation succeeded -- copy the result

I've applied most of this to my tree, except for the NOOP bits. My concern is that the way you've implemented NOOP does not allow for all possible return codes to be passed in due to the error checking the iocb submit code performs on the inputs. It can also spuriously fail if the filedescriptor field points to an fd that doesn't exist, which could be somewhat unexpected (especially if it is initialized to 0 by default, and therefore would not fail during normal operation where a program is run with stdin). Got any idea for cleaning those problems up?

-ben

Could you provide an example of a possible return code that cannot be passed in? I know you can't pass a 64 bit return code on a 32 bit platform, but then neither can any other operation.

Currently the operation requires a valid fd just like every other operation does, so I don't consider such a failure to be spurious.

The alternative is to change the aio framework itself to support operations that don't work on fds. That would move the fget() call and the overflow check to below where it sets req->ki_user_data. The check for IOCB_CMD_NOOP would then go before the fget() call and overflow check.

If you think this is the way to go, I can code up patch to do this.

aio_nbytes is clamped to disallow negative values at present.

Okay, if it is documented, that makes sense. I just wasn't sure if it happened to be an oversight.

That's a possibility. I don't like the idea of splitting things out too much, as special cases look ugly. The io_submit interface really does assume that the file descriptor exists, and removing that doesn't really make sense -- non-file descriptor consuming operations should most likely be their own syscalls (imho). But if you're okay with requiring an fd for the NOOP, then that seems alright. Does anyone else have any thoughts on the matter?

-ben

"Do you seek knowledge in time travel?"

The IOCB_CMD_NOOP patch doesn't use aio_nbytes.

This would defeat the multiple iocb submission feature of io_submit(). All async operations should be submittable through io_submit() to permit multiple operation submission.