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
On Mon, Oct 14, 2002 at 06:17:33PM -0700, John Myers wrote:
> Please apply.
> # 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
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
Benjamin LaHaise wrote:
>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.
>
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.
>It can also spuriously fail
>if the filedescriptor field points to an fd that doesn't exist,
>
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.
On Tue, Oct 15, 2002 at 01:50:40PM -0700, John Gardiner Myers wrote:
> Benjamin LaHaise wrote:
>
> >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.
> >
> 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.
aio_nbytes is clamped to disallow negative values at present.
> Currently the operation requires a valid fd just like every other
> operation does, so I don't consider such a failure to be spurious.
Okay, if it is documented, that makes sense. I just wasn't sure if it
happened to be an oversight.
> 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.
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?"
Benjamin LaHaise wrote:
>aio_nbytes is clamped to disallow negative values at present.
>
>
The IOCB_CMD_NOOP patch doesn't use aio_nbytes.
Benjamin LaHaise wrote:
>non-file descriptor consuming operations should most likely
>be their own syscalls (imho).
>
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.