[2/8] keep io_event in aio_kiocb

Commit Message

Al Viro March 10, 2019, 7:08 a.m. UTC

From: Al Viro <[e-mail]>

Signed-off-by: Al Viro <[e-mail]>
---
 fs/aio.c | 56 +++++++++++++++++++++-----------------------------------
 1 file changed, 21 insertions(+), 35 deletions(-)

Comments

Christoph Hellwig March 11, 2019, 7:43 p.m. UTC | #1

On Sun, Mar 10, 2019 at 07:08:16AM +0000, Al Viro wrote:
> From: Al Viro <[e-mail]>
> 
> Signed-off-by: Al Viro <[e-mail]>

This could use a little changelog at least that explains the why.

It also removes lookup_kiocb and folds that into the only caller,
which is at least worth mentioning (and probably should be a separate
patch).

Al Viro March 11, 2019, 9:17 p.m. UTC | #2

On Mon, Mar 11, 2019 at 08:43:06PM +0100, Christoph Hellwig wrote:
> On Sun, Mar 10, 2019 at 07:08:16AM +0000, Al Viro wrote:
> > From: Al Viro <[e-mail]>
> > 
> > Signed-off-by: Al Viro <[e-mail]>
> 
> This could use a little changelog at least that explains the why.
> 
> It also removes lookup_kiocb and folds that into the only caller,
> which is at least worth mentioning (and probably should be a separate
> patch).

More confusingly, it contains a pointless rudiment in aio_poll_wake();
that chunk doesn't solve the problems with ->woken, and the whole
thing gets killed off shortly...

Patch

diff --git a/fs/aio.c b/fs/aio.c
index 363d7d7c8bff..2249a7a1d6b3 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -204,8 +204,7 @@  struct aio_kiocb {
 	struct kioctx		*ki_ctx;
 	kiocb_cancel_fn		*ki_cancel;
 
-	struct iocb __user	*ki_user_iocb;	/* user's aiocb */
-	__u64			ki_user_data;	/* user's data for completion */
+	struct io_event		ki_res;
 
 	struct list_head	ki_list;	/* the aio core uses this
 						 * for cancellation */
@@ -1087,10 +1086,9 @@  static inline void iocb_put(struct aio_kiocb *iocb)
 static void aio_fill_event(struct io_event *ev, struct aio_kiocb *iocb,
 			   long res, long res2)
 {
-	ev->obj = (u64)(unsigned long)iocb->ki_user_iocb;
-	ev->data = iocb->ki_user_data;
-	ev->res = res;
-	ev->res2 = res2;
+	iocb->ki_res.res = res;
+	iocb->ki_res.res2 = res2;
+	*ev = iocb->ki_res;
 }
 
 /* aio_complete
@@ -1126,7 +1124,7 @@  static void aio_complete(struct aio_kiocb *iocb, long res, long res2)
 	flush_dcache_page(ctx->ring_pages[pos / AIO_EVENTS_PER_PAGE]);
 
 	pr_debug("%p[%u]: %p: %p %Lx %lx %lx\n",
-		 ctx, tail, iocb, iocb->ki_user_iocb, iocb->ki_user_data,
+		 ctx, tail, iocb, (void __user *)iocb->ki_res.obj, iocb->ki_res.data,
 		 res, res2);
 
 	/* after flagging the request as done, we
@@ -1674,13 +1672,13 @@  static int aio_poll_wake(struct wait_queue_entry *wait, unsigned mode, int sync,
 	__poll_t mask = key_to_poll(key);
 	unsigned long flags;
 
+	/* for instances that support it check for an event match first: */
+	if (mask && !(mask & req->events))
+		return 0;
+
 	req->woken = true;
 
-	/* for instances that support it check for an event match first: */
 	if (mask) {
-		if (!(mask & req->events))
-			return 0;
-
 		/*
 		 * Try to complete the iocb inline if we can. Use
 		 * irqsave/irqrestore because not all filesystems (e.g. fuse)
@@ -1844,8 +1842,10 @@  static int __io_submit_one(struct kioctx *ctx, const struct iocb *iocb,
 		goto out_put_req;
 	}
 
-	req->ki_user_iocb = user_iocb;
-	req->ki_user_data = iocb->aio_data;
+	req->ki_res.obj = (u64)(unsigned long)user_iocb;
+	req->ki_res.data = iocb->aio_data;
+	req->ki_res.res = 0;
+	req->ki_res.res2 = 0;
 
 	switch (iocb->aio_lio_opcode) {
 	case IOCB_CMD_PREAD:
@@ -2002,24 +2002,6 @@  COMPAT_SYSCALL_DEFINE3(io_submit, compat_aio_context_t, ctx_id,
 }
 #endif
 
-/* lookup_kiocb
- *	Finds a given iocb for cancellation.
- */
-static struct aio_kiocb *
-lookup_kiocb(struct kioctx *ctx, struct iocb __user *iocb)
-{
-	struct aio_kiocb *kiocb;
-
-	assert_spin_locked(&ctx->ctx_lock);
-
-	/* TODO: use a hash or array, this sucks. */
-	list_for_each_entry(kiocb, &ctx->active_reqs, ki_list) {
-		if (kiocb->ki_user_iocb == iocb)
-			return kiocb;
-	}
-	return NULL;
-}
-
 /* sys_io_cancel:
  *	Attempts to cancel an iocb previously passed to io_submit.  If
  *	the operation is successfully cancelled, the resulting event is
@@ -2037,6 +2019,7 @@  SYSCALL_DEFINE3(io_cancel, aio_context_t, ctx_id, struct iocb __user *, iocb,
 	struct aio_kiocb *kiocb;
 	int ret = -EINVAL;
 	u32 key;
+	u64 obj = (u64)(unsigned long)iocb;
 
 	if (unlikely(get_user(key, &iocb->aio_key)))
 		return -EFAULT;
@@ -2048,10 +2031,13 @@  SYSCALL_DEFINE3(io_cancel, aio_context_t, ctx_id, struct iocb __user *, iocb,
 		return -EINVAL;
 
 	spin_lock_irq(&ctx->ctx_lock);
-	kiocb = lookup_kiocb(ctx, iocb);
-	if (kiocb) {
-		ret = kiocb->ki_cancel(&kiocb->rw);
-		list_del_init(&kiocb->ki_list);
+	/* TODO: use a hash or array, this sucks. */
+	list_for_each_entry(kiocb, &ctx->active_reqs, ki_list) {
+		if (kiocb->ki_res.obj == obj) {
+			ret = kiocb->ki_cancel(&kiocb->rw);
+			list_del_init(&kiocb->ki_list);
+			break;
+		}
 	}
 	spin_unlock_irq(&ctx->ctx_lock);