On 11/27/18 2:53 AM, Benny Halevy wrote:

quoted

@@ -818,11 +853,15 @@ static int kill_ioctx(struct mm_struct *mm, struct kioctx *ctx,
 {
 	struct kioctx_table *table;
 
+	mutex_lock(&ctx->getevents_lock);
 	spin_lock(&mm->ioctx_lock);
 	if (atomic_xchg(&ctx->dead, 1)) {
 		spin_unlock(&mm->ioctx_lock);
+		mutex_unlock(&ctx->getevents_lock);
 		return -EINVAL;
 	}
+	aio_iopoll_reap_events(ctx);
+	mutex_unlock(&ctx->getevents_lock);

Is it worth handling the mutex lock and calling aio_iopoll_reap_events
only if (ctx->flags & IOCTX_FLAG_IOPOLL)?  If so, testing it can be
removed from aio_iopoll_reap_events() (and maybe it could even
be open coded
here since this is its only call site apparently)

I don't think it really matters, this only happens when you tear down an
io_context. FWIW, I think it's cleaner to retain the test in the
function, not outside it.

quoted

@@ -1072,6 +1112,15 @@ static inline void iocb_put(struct aio_kiocb *iocb)
 	}
 }
 
+static void iocb_put_many(struct kioctx *ctx, void **iocbs, int *nr)
+{
+	if (nr) {

How can nr by NULL?
And what's the point of supporting this case?
Did you mean: if (*nr)?
(In this case, if safe to call the functions below with *nr==0,
I'm not sure it's worth optimizing... especially since this is a static
function and its callers make sure to call it only when *nr > 0)

Indeed, that should be if (*nr), thanks! The slub implementation of the
bulk free complains if you pass in nr == 0. Outside of that, a single
check should be better than checking in multiple spots.

quoted

@@ -1261,6 +1310,166 @@ static bool aio_read_events(struct kioctx *ctx, long min_nr, long nr,
 	return ret < 0 || *i >= min_nr;
 }
 
+#define AIO_IOPOLL_BATCH	8
+
+/*
+ * Process completed iocb iopoll entries, copying the result to userspace.
+ */
+static long aio_iopoll_reap(struct kioctx *ctx, struct io_event __user *evs,
+			    unsigned int *nr_events, long max)
+{
+	void *iocbs[AIO_IOPOLL_BATCH];
+	struct aio_kiocb *iocb, *n;
+	int to_free = 0, ret = 0;
+
+	list_for_each_entry_safe(iocb, n, &ctx->poll_completing, ki_list) {
+		if (*nr_events == max)

*nr_events >= max would be safer.

I don't see how we can get there with it being larger than already, that
would be a big bug if we fill more events than userspace asked for.

quoted

@@ -1570,17 +1829,43 @@ static inline void aio_rw_done(struct kiocb *req, ssize_t ret)
 	default:
 		req->ki_complete(req, ret, 0);
 	}
+

nit: this hunk is probably unintentional

Looks like it, I'll kill it.


-- 
Jens Axboe