* [PATCH 1/8] futex: move FUTEX2_VALID_MASK to futex.h
2023-09-21 18:29 [PATCHSET v5] Add io_uring futex/futexv support Jens Axboe
@ 2023-09-21 18:29 ` Jens Axboe
2023-09-21 18:29 ` [PATCH 2/8] futex: factor out the futex wake handling Jens Axboe
` (6 subsequent siblings)
7 siblings, 0 replies; 21+ messages in thread
From: Jens Axboe @ 2023-09-21 18:29 UTC (permalink / raw)
To: io-uring, linux-kernel; +Cc: peterz, andres, tglx, Jens Axboe
We need this for validating the futex2 flags outside of the normal
futex syscalls.
Signed-off-by: Jens Axboe <[email protected]>
---
kernel/futex/futex.h | 2 ++
kernel/futex/syscalls.c | 2 --
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/futex/futex.h b/kernel/futex/futex.h
index a06030a1a27b..a173a9d501e1 100644
--- a/kernel/futex/futex.h
+++ b/kernel/futex/futex.h
@@ -52,6 +52,8 @@ static inline unsigned int futex_to_flags(unsigned int op)
return flags;
}
+#define FUTEX2_VALID_MASK (FUTEX2_SIZE_MASK | FUTEX2_PRIVATE)
+
/* FUTEX2_ to FLAGS_ */
static inline unsigned int futex2_to_flags(unsigned int flags2)
{
diff --git a/kernel/futex/syscalls.c b/kernel/futex/syscalls.c
index 8200d86d30e1..2b5cafdfdc50 100644
--- a/kernel/futex/syscalls.c
+++ b/kernel/futex/syscalls.c
@@ -179,8 +179,6 @@ SYSCALL_DEFINE6(futex, u32 __user *, uaddr, int, op, u32, val,
return do_futex(uaddr, op, val, tp, uaddr2, (unsigned long)utime, val3);
}
-#define FUTEX2_VALID_MASK (FUTEX2_SIZE_MASK | FUTEX2_PRIVATE)
-
/**
* futex_parse_waitv - Parse a waitv array from userspace
* @futexv: Kernel side list of waiters to be filled
--
2.40.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 2/8] futex: factor out the futex wake handling
2023-09-21 18:29 [PATCHSET v5] Add io_uring futex/futexv support Jens Axboe
2023-09-21 18:29 ` [PATCH 1/8] futex: move FUTEX2_VALID_MASK to futex.h Jens Axboe
@ 2023-09-21 18:29 ` Jens Axboe
2023-09-21 18:29 ` [PATCH 3/8] futex: abstract out a __futex_wake_mark() helper Jens Axboe
` (5 subsequent siblings)
7 siblings, 0 replies; 21+ messages in thread
From: Jens Axboe @ 2023-09-21 18:29 UTC (permalink / raw)
To: io-uring, linux-kernel; +Cc: peterz, andres, tglx, Jens Axboe
In preparation for having another waker that isn't futex_wake_mark(),
add a wake handler in futex_q. No extra data is associated with the
handler outside of struct futex_q itself. futex_wake_mark() is defined as
the standard wakeup helper, now set through futex_q_init like other
defaults.
Normal sync futex waiting relies on wake_q holding tasks that should
be woken up. This is what futex_wake_mark() does, it'll unqueue the
futex and add the associated task to the wake queue. For async usage of
futex waiting, rather than having tasks sleeping on the futex, we'll
need to deal with a futex wake differently. For the planned io_uring
case, that means posting a completion event for the task in question.
Having a definable wake handler can help support that use case.
Signed-off-by: Jens Axboe <[email protected]>
---
kernel/futex/futex.h | 5 +++++
kernel/futex/requeue.c | 3 ++-
kernel/futex/waitwake.c | 6 +++---
3 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/kernel/futex/futex.h b/kernel/futex/futex.h
index a173a9d501e1..547f509b2c87 100644
--- a/kernel/futex/futex.h
+++ b/kernel/futex/futex.h
@@ -139,11 +139,15 @@ struct futex_pi_state {
union futex_key key;
} __randomize_layout;
+struct futex_q;
+typedef void (futex_wake_fn)(struct wake_q_head *wake_q, struct futex_q *q);
+
/**
* struct futex_q - The hashed futex queue entry, one per waiting task
* @list: priority-sorted list of tasks waiting on this futex
* @task: the task waiting on the futex
* @lock_ptr: the hash bucket lock
+ * @wake: the wake handler for this queue
* @key: the key the futex is hashed on
* @pi_state: optional priority inheritance state
* @rt_waiter: rt_waiter storage for use with requeue_pi
@@ -168,6 +172,7 @@ struct futex_q {
struct task_struct *task;
spinlock_t *lock_ptr;
+ futex_wake_fn *wake;
union futex_key key;
struct futex_pi_state *pi_state;
struct rt_mutex_waiter *rt_waiter;
diff --git a/kernel/futex/requeue.c b/kernel/futex/requeue.c
index a0a79954f506..9dc789399a1a 100644
--- a/kernel/futex/requeue.c
+++ b/kernel/futex/requeue.c
@@ -58,6 +58,7 @@ enum {
const struct futex_q futex_q_init = {
/* list gets initialized in futex_queue()*/
+ .wake = futex_wake_mark,
.key = FUTEX_KEY_INIT,
.bitset = FUTEX_BITSET_MATCH_ANY,
.requeue_state = ATOMIC_INIT(Q_REQUEUE_PI_NONE),
@@ -593,7 +594,7 @@ int futex_requeue(u32 __user *uaddr1, unsigned int flags1,
/* Plain futexes just wake or requeue and are done */
if (!requeue_pi) {
if (++task_count <= nr_wake)
- futex_wake_mark(&wake_q, this);
+ this->wake(&wake_q, this);
else
requeue_futex(this, hb1, hb2, &key2);
continue;
diff --git a/kernel/futex/waitwake.c b/kernel/futex/waitwake.c
index 37860f794bf7..35c6a637a4bb 100644
--- a/kernel/futex/waitwake.c
+++ b/kernel/futex/waitwake.c
@@ -177,7 +177,7 @@ int futex_wake(u32 __user *uaddr, unsigned int flags, int nr_wake, u32 bitset)
if (!(this->bitset & bitset))
continue;
- futex_wake_mark(&wake_q, this);
+ this->wake(&wake_q, this);
if (++ret >= nr_wake)
break;
}
@@ -292,7 +292,7 @@ int futex_wake_op(u32 __user *uaddr1, unsigned int flags, u32 __user *uaddr2,
ret = -EINVAL;
goto out_unlock;
}
- futex_wake_mark(&wake_q, this);
+ this->wake(&wake_q, this);
if (++ret >= nr_wake)
break;
}
@@ -306,7 +306,7 @@ int futex_wake_op(u32 __user *uaddr1, unsigned int flags, u32 __user *uaddr2,
ret = -EINVAL;
goto out_unlock;
}
- futex_wake_mark(&wake_q, this);
+ this->wake(&wake_q, this);
if (++op_ret >= nr_wake2)
break;
}
--
2.40.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 3/8] futex: abstract out a __futex_wake_mark() helper
2023-09-21 18:29 [PATCHSET v5] Add io_uring futex/futexv support Jens Axboe
2023-09-21 18:29 ` [PATCH 1/8] futex: move FUTEX2_VALID_MASK to futex.h Jens Axboe
2023-09-21 18:29 ` [PATCH 2/8] futex: factor out the futex wake handling Jens Axboe
@ 2023-09-21 18:29 ` Jens Axboe
2023-09-21 18:29 ` [PATCH 4/8] io_uring: add support for futex wake and wait Jens Axboe
` (4 subsequent siblings)
7 siblings, 0 replies; 21+ messages in thread
From: Jens Axboe @ 2023-09-21 18:29 UTC (permalink / raw)
To: io-uring, linux-kernel; +Cc: peterz, andres, tglx, Jens Axboe
Move the unqueue and lock_ptr clear into a helper that futex_wake_mark()
calls. Add it to the public functions as well, in preparation for using
it outside the core futex code.
Suggested-by: Peter Zijlstra <[email protected]>
Signed-off-by: Jens Axboe <[email protected]>
---
kernel/futex/futex.h | 1 +
kernel/futex/waitwake.c | 33 ++++++++++++++++++++++-----------
2 files changed, 23 insertions(+), 11 deletions(-)
diff --git a/kernel/futex/futex.h b/kernel/futex/futex.h
index 547f509b2c87..33835b81e0c3 100644
--- a/kernel/futex/futex.h
+++ b/kernel/futex/futex.h
@@ -219,6 +219,7 @@ extern int futex_wait_setup(u32 __user *uaddr, u32 val, unsigned int flags,
struct futex_q *q, struct futex_hash_bucket **hb);
extern void futex_wait_queue(struct futex_hash_bucket *hb, struct futex_q *q,
struct hrtimer_sleeper *timeout);
+extern bool __futex_wake_mark(struct futex_q *q);
extern void futex_wake_mark(struct wake_q_head *wake_q, struct futex_q *q);
extern int fault_in_user_writeable(u32 __user *uaddr);
diff --git a/kernel/futex/waitwake.c b/kernel/futex/waitwake.c
index 35c6a637a4bb..6fcf5f723719 100644
--- a/kernel/futex/waitwake.c
+++ b/kernel/futex/waitwake.c
@@ -106,20 +106,11 @@
* double_lock_hb() and double_unlock_hb(), respectively.
*/
-/*
- * The hash bucket lock must be held when this is called.
- * Afterwards, the futex_q must not be accessed. Callers
- * must ensure to later call wake_up_q() for the actual
- * wakeups to occur.
- */
-void futex_wake_mark(struct wake_q_head *wake_q, struct futex_q *q)
+bool __futex_wake_mark(struct futex_q *q)
{
- struct task_struct *p = q->task;
-
if (WARN(q->pi_state || q->rt_waiter, "refusing to wake PI futex\n"))
- return;
+ return false;
- get_task_struct(p);
__futex_unqueue(q);
/*
* The waiting task can free the futex_q as soon as q->lock_ptr = NULL
@@ -130,6 +121,26 @@ void futex_wake_mark(struct wake_q_head *wake_q, struct futex_q *q)
*/
smp_store_release(&q->lock_ptr, NULL);
+ return true;
+}
+
+/*
+ * The hash bucket lock must be held when this is called.
+ * Afterwards, the futex_q must not be accessed. Callers
+ * must ensure to later call wake_up_q() for the actual
+ * wakeups to occur.
+ */
+void futex_wake_mark(struct wake_q_head *wake_q, struct futex_q *q)
+{
+ struct task_struct *p = q->task;
+
+ get_task_struct(p);
+
+ if (!__futex_wake_mark(q)) {
+ put_task_struct(p);
+ return;
+ }
+
/*
* Queue the task for later wakeup for after we've released
* the hb->lock.
--
2.40.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 4/8] io_uring: add support for futex wake and wait
2023-09-21 18:29 [PATCHSET v5] Add io_uring futex/futexv support Jens Axboe
` (2 preceding siblings ...)
2023-09-21 18:29 ` [PATCH 3/8] futex: abstract out a __futex_wake_mark() helper Jens Axboe
@ 2023-09-21 18:29 ` Jens Axboe
2023-09-27 9:05 ` Peter Zijlstra
2023-09-21 18:29 ` [PATCH 5/8] futex: add wake_data to struct futex_q Jens Axboe
` (3 subsequent siblings)
7 siblings, 1 reply; 21+ messages in thread
From: Jens Axboe @ 2023-09-21 18:29 UTC (permalink / raw)
To: io-uring, linux-kernel; +Cc: peterz, andres, tglx, Jens Axboe
Add support for FUTEX_WAKE/WAIT primitives.
IORING_OP_FUTEX_WAKE is mix of FUTEX_WAKE and FUTEX_WAKE_BITSET, as
it does support passing in a bitset.
Similary, IORING_OP_FUTEX_WAIT is a mix of FUTEX_WAIT and
FUTEX_WAIT_BITSET.
FUTEX_WAKE is straight forward, as those can always be done directly from
the io_uring submission without needing async handling. For FUTEX_WAIT,
things are a bit more complicated. If the futex isn't ready, then we
rely on a callback via futex_queue->wake() when someone wakes up the
futex. From that calback, we queue up task_work with the original task,
which will post a CQE and wake it, if necessary.
Cancelations are supported, both from the application point-of-view,
but also to be able to cancel pending waits if the ring exits before
all events have occurred. The return value of futex_unqueue() is used
to gate who wins the potential race between cancelation and futex
wakeups. Whomever gets a 'ret == 1' return from that claims ownership
of the io_uring futex request.
This is just the barebones wait/wake support. PI or REQUEUE support is
not added at this point, unclear if we might look into that later.
Likewise, explicit timeouts are not supported either. It is expected
that users that need timeouts would do so via the usual io_uring
mechanism to do that using linked timeouts.
Signed-off-by: Jens Axboe <[email protected]>
---
include/linux/io_uring_types.h | 5 +
include/uapi/linux/io_uring.h | 3 +
io_uring/Makefile | 1 +
io_uring/cancel.c | 5 +
io_uring/cancel.h | 4 +
io_uring/futex.c | 230 +++++++++++++++++++++++++++++++++
io_uring/futex.h | 34 +++++
io_uring/io_uring.c | 7 +
io_uring/opdef.c | 23 ++++
9 files changed, 312 insertions(+)
create mode 100644 io_uring/futex.c
create mode 100644 io_uring/futex.h
diff --git a/include/linux/io_uring_types.h b/include/linux/io_uring_types.h
index fe1c5d4ec56c..4fc68cd6b043 100644
--- a/include/linux/io_uring_types.h
+++ b/include/linux/io_uring_types.h
@@ -315,6 +315,11 @@ struct io_ring_ctx {
struct hlist_head waitid_list;
+#ifdef CONFIG_FUTEX
+ struct hlist_head futex_list;
+ struct io_alloc_cache futex_cache;
+#endif
+
const struct cred *sq_creds; /* cred used for __io_sq_thread() */
struct io_sq_data *sq_data; /* if using sq thread polling */
diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
index 683ac2b74721..4ddd7bdbbfb8 100644
--- a/include/uapi/linux/io_uring.h
+++ b/include/uapi/linux/io_uring.h
@@ -66,6 +66,7 @@ struct io_uring_sqe {
__u32 msg_ring_flags;
__u32 uring_cmd_flags;
__u32 waitid_flags;
+ __u32 futex_flags;
};
__u64 user_data; /* data to be passed back at completion time */
/* pack this to avoid bogus arm OABI complaints */
@@ -243,6 +244,8 @@ enum io_uring_op {
IORING_OP_SENDMSG_ZC,
IORING_OP_READ_MULTISHOT,
IORING_OP_WAITID,
+ IORING_OP_FUTEX_WAIT,
+ IORING_OP_FUTEX_WAKE,
/* this goes last, obviously */
IORING_OP_LAST,
diff --git a/io_uring/Makefile b/io_uring/Makefile
index 7bd64e442567..e5be47e4fc3b 100644
--- a/io_uring/Makefile
+++ b/io_uring/Makefile
@@ -10,3 +10,4 @@ obj-$(CONFIG_IO_URING) += io_uring.o xattr.o nop.o fs.o splice.o \
cancel.o kbuf.o rsrc.o rw.o opdef.o \
notif.o waitid.o
obj-$(CONFIG_IO_WQ) += io-wq.o
+obj-$(CONFIG_FUTEX) += futex.o
diff --git a/io_uring/cancel.c b/io_uring/cancel.c
index eb77a51c5a79..3c19cccb1aec 100644
--- a/io_uring/cancel.c
+++ b/io_uring/cancel.c
@@ -16,6 +16,7 @@
#include "poll.h"
#include "timeout.h"
#include "waitid.h"
+#include "futex.h"
#include "cancel.h"
struct io_cancel {
@@ -124,6 +125,10 @@ int io_try_cancel(struct io_uring_task *tctx, struct io_cancel_data *cd,
if (ret != -ENOENT)
return ret;
+ ret = io_futex_cancel(ctx, cd, issue_flags);
+ if (ret != -ENOENT)
+ return ret;
+
spin_lock(&ctx->completion_lock);
if (!(cd->flags & IORING_ASYNC_CANCEL_FD))
ret = io_timeout_cancel(ctx, cd);
diff --git a/io_uring/cancel.h b/io_uring/cancel.h
index fc98622e6166..c0a8e7c520b6 100644
--- a/io_uring/cancel.h
+++ b/io_uring/cancel.h
@@ -1,4 +1,6 @@
// SPDX-License-Identifier: GPL-2.0
+#ifndef IORING_CANCEL_H
+#define IORING_CANCEL_H
#include <linux/io_uring_types.h>
@@ -22,3 +24,5 @@ void init_hash_table(struct io_hash_table *table, unsigned size);
int io_sync_cancel(struct io_ring_ctx *ctx, void __user *arg);
bool io_cancel_req_match(struct io_kiocb *req, struct io_cancel_data *cd);
+
+#endif
diff --git a/io_uring/futex.c b/io_uring/futex.c
new file mode 100644
index 000000000000..4278302d212c
--- /dev/null
+++ b/io_uring/futex.c
@@ -0,0 +1,230 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/fs.h>
+#include <linux/file.h>
+#include <linux/io_uring.h>
+
+#include <uapi/linux/io_uring.h>
+
+#include "../kernel/futex/futex.h"
+#include "io_uring.h"
+#include "rsrc.h"
+#include "futex.h"
+
+struct io_futex {
+ struct file *file;
+ u32 __user *uaddr;
+ unsigned long futex_val;
+ unsigned long futex_mask;
+ u32 futex_flags;
+};
+
+struct io_futex_data {
+ union {
+ struct futex_q q;
+ struct io_cache_entry cache;
+ };
+ struct io_kiocb *req;
+};
+
+void io_futex_cache_init(struct io_ring_ctx *ctx)
+{
+ io_alloc_cache_init(&ctx->futex_cache, IO_NODE_ALLOC_CACHE_MAX,
+ sizeof(struct io_futex_data));
+}
+
+static void io_futex_cache_entry_free(struct io_cache_entry *entry)
+{
+ kfree(container_of(entry, struct io_futex_data, cache));
+}
+
+void io_futex_cache_free(struct io_ring_ctx *ctx)
+{
+ io_alloc_cache_free(&ctx->futex_cache, io_futex_cache_entry_free);
+}
+
+static void io_futex_complete(struct io_kiocb *req, struct io_tw_state *ts)
+{
+ struct io_futex_data *ifd = req->async_data;
+ struct io_ring_ctx *ctx = req->ctx;
+
+ io_tw_lock(ctx, ts);
+ if (!io_alloc_cache_put(&ctx->futex_cache, &ifd->cache))
+ kfree(ifd);
+ req->async_data = NULL;
+ hlist_del_init(&req->hash_node);
+ io_req_task_complete(req, ts);
+}
+
+static bool __io_futex_cancel(struct io_ring_ctx *ctx, struct io_kiocb *req)
+{
+ struct io_futex_data *ifd = req->async_data;
+
+ /* futex wake already done or in progress */
+ if (!futex_unqueue(&ifd->q))
+ return false;
+
+ hlist_del_init(&req->hash_node);
+ io_req_set_res(req, -ECANCELED, 0);
+ req->io_task_work.func = io_futex_complete;
+ io_req_task_work_add(req);
+ return true;
+}
+
+int io_futex_cancel(struct io_ring_ctx *ctx, struct io_cancel_data *cd,
+ unsigned int issue_flags)
+{
+ struct hlist_node *tmp;
+ struct io_kiocb *req;
+ int nr = 0;
+
+ if (cd->flags & (IORING_ASYNC_CANCEL_FD|IORING_ASYNC_CANCEL_FD_FIXED))
+ return -ENOENT;
+
+ io_ring_submit_lock(ctx, issue_flags);
+ hlist_for_each_entry_safe(req, tmp, &ctx->futex_list, hash_node) {
+ if (req->cqe.user_data != cd->data &&
+ !(cd->flags & IORING_ASYNC_CANCEL_ANY))
+ continue;
+ if (__io_futex_cancel(ctx, req))
+ nr++;
+ if (!(cd->flags & IORING_ASYNC_CANCEL_ALL))
+ break;
+ }
+ io_ring_submit_unlock(ctx, issue_flags);
+
+ if (nr)
+ return nr;
+
+ return -ENOENT;
+}
+
+bool io_futex_remove_all(struct io_ring_ctx *ctx, struct task_struct *task,
+ bool cancel_all)
+{
+ struct hlist_node *tmp;
+ struct io_kiocb *req;
+ bool found = false;
+
+ lockdep_assert_held(&ctx->uring_lock);
+
+ hlist_for_each_entry_safe(req, tmp, &ctx->futex_list, hash_node) {
+ if (!io_match_task_safe(req, task, cancel_all))
+ continue;
+ __io_futex_cancel(ctx, req);
+ found = true;
+ }
+
+ return found;
+}
+
+int io_futex_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
+{
+ struct io_futex *iof = io_kiocb_to_cmd(req, struct io_futex);
+ u32 flags;
+
+ if (unlikely(sqe->fd || sqe->len || sqe->buf_index || sqe->file_index))
+ return -EINVAL;
+
+ iof->uaddr = u64_to_user_ptr(READ_ONCE(sqe->addr));
+ iof->futex_val = READ_ONCE(sqe->addr2);
+ iof->futex_mask = READ_ONCE(sqe->addr3);
+ flags = READ_ONCE(sqe->futex_flags);
+
+ if (flags & ~FUTEX2_VALID_MASK)
+ return -EINVAL;
+
+ iof->futex_flags = futex2_to_flags(flags);
+ if (!futex_flags_valid(iof->futex_flags))
+ return -EINVAL;
+
+ if (!futex_validate_input(iof->futex_flags, iof->futex_val) ||
+ !futex_validate_input(iof->futex_flags, iof->futex_mask))
+ return -EINVAL;
+
+ return 0;
+}
+
+static void io_futex_wake_fn(struct wake_q_head *wake_q, struct futex_q *q)
+{
+ struct io_futex_data *ifd = container_of(q, struct io_futex_data, q);
+ struct io_kiocb *req = ifd->req;
+
+ if (unlikely(!__futex_wake_mark(q)))
+ return;
+
+ io_req_set_res(req, 0, 0);
+ req->io_task_work.func = io_futex_complete;
+ io_req_task_work_add(req);
+}
+
+static struct io_futex_data *io_alloc_ifd(struct io_ring_ctx *ctx)
+{
+ struct io_cache_entry *entry;
+
+ entry = io_alloc_cache_get(&ctx->futex_cache);
+ if (entry)
+ return container_of(entry, struct io_futex_data, cache);
+
+ return kmalloc(sizeof(struct io_futex_data), GFP_NOWAIT);
+}
+
+int io_futex_wait(struct io_kiocb *req, unsigned int issue_flags)
+{
+ struct io_futex *iof = io_kiocb_to_cmd(req, struct io_futex);
+ struct io_ring_ctx *ctx = req->ctx;
+ struct io_futex_data *ifd = NULL;
+ struct futex_hash_bucket *hb;
+ int ret;
+
+ if (!iof->futex_mask) {
+ ret = -EINVAL;
+ goto done;
+ }
+
+ io_ring_submit_lock(ctx, issue_flags);
+ ifd = io_alloc_ifd(ctx);
+ if (!ifd) {
+ ret = -ENOMEM;
+ goto done_unlock;
+ }
+
+ req->async_data = ifd;
+ ifd->q = futex_q_init;
+ ifd->q.bitset = iof->futex_mask;
+ ifd->q.wake = io_futex_wake_fn;
+ ifd->req = req;
+
+ ret = futex_wait_setup(iof->uaddr, iof->futex_val,
+ futex2_to_flags(iof->futex_flags), &ifd->q, &hb);
+ if (!ret) {
+ hlist_add_head(&req->hash_node, &ctx->futex_list);
+ io_ring_submit_unlock(ctx, issue_flags);
+
+ futex_queue(&ifd->q, hb);
+ return IOU_ISSUE_SKIP_COMPLETE;
+ }
+
+done_unlock:
+ io_ring_submit_unlock(ctx, issue_flags);
+done:
+ if (ret < 0)
+ req_set_fail(req);
+ io_req_set_res(req, ret, 0);
+ kfree(ifd);
+ return IOU_OK;
+}
+
+int io_futex_wake(struct io_kiocb *req, unsigned int issue_flags)
+{
+ struct io_futex *iof = io_kiocb_to_cmd(req, struct io_futex);
+ int ret;
+
+ ret = futex_wake(iof->uaddr, futex2_to_flags(iof->futex_flags),
+ iof->futex_val, iof->futex_mask);
+ if (ret < 0)
+ req_set_fail(req);
+ io_req_set_res(req, ret, 0);
+ return IOU_OK;
+}
diff --git a/io_uring/futex.h b/io_uring/futex.h
new file mode 100644
index 000000000000..ddc9e0d73c52
--- /dev/null
+++ b/io_uring/futex.h
@@ -0,0 +1,34 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include "cancel.h"
+
+int io_futex_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe);
+int io_futex_wait(struct io_kiocb *req, unsigned int issue_flags);
+int io_futex_wake(struct io_kiocb *req, unsigned int issue_flags);
+
+#if defined(CONFIG_FUTEX)
+int io_futex_cancel(struct io_ring_ctx *ctx, struct io_cancel_data *cd,
+ unsigned int issue_flags);
+bool io_futex_remove_all(struct io_ring_ctx *ctx, struct task_struct *task,
+ bool cancel_all);
+void io_futex_cache_init(struct io_ring_ctx *ctx);
+void io_futex_cache_free(struct io_ring_ctx *ctx);
+#else
+static inline int io_futex_cancel(struct io_ring_ctx *ctx,
+ struct io_cancel_data *cd,
+ unsigned int issue_flags)
+{
+ return 0;
+}
+static inline bool io_futex_remove_all(struct io_ring_ctx *ctx,
+ struct task_struct *task, bool cancel_all)
+{
+ return false;
+}
+static inline void io_futex_cache_init(struct io_ring_ctx *ctx)
+{
+}
+static inline void io_futex_cache_free(struct io_ring_ctx *ctx)
+{
+}
+#endif
diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index 2dff4772bf14..f4198258159d 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -93,6 +93,7 @@
#include "net.h"
#include "notif.h"
#include "waitid.h"
+#include "futex.h"
#include "timeout.h"
#include "poll.h"
@@ -330,6 +331,7 @@ static __cold struct io_ring_ctx *io_ring_ctx_alloc(struct io_uring_params *p)
sizeof(struct async_poll));
io_alloc_cache_init(&ctx->netmsg_cache, IO_ALLOC_CACHE_MAX,
sizeof(struct io_async_msghdr));
+ io_futex_cache_init(ctx);
init_completion(&ctx->ref_comp);
xa_init_flags(&ctx->personalities, XA_FLAGS_ALLOC1);
mutex_init(&ctx->uring_lock);
@@ -350,6 +352,9 @@ static __cold struct io_ring_ctx *io_ring_ctx_alloc(struct io_uring_params *p)
ctx->submit_state.free_list.next = NULL;
INIT_WQ_LIST(&ctx->locked_free_list);
INIT_HLIST_HEAD(&ctx->waitid_list);
+#ifdef CONFIG_FUTEX
+ INIT_HLIST_HEAD(&ctx->futex_list);
+#endif
INIT_DELAYED_WORK(&ctx->fallback_work, io_fallback_req_func);
INIT_WQ_LIST(&ctx->submit_state.compl_reqs);
return ctx;
@@ -2894,6 +2899,7 @@ static __cold void io_ring_ctx_free(struct io_ring_ctx *ctx)
io_eventfd_unregister(ctx);
io_alloc_cache_free(&ctx->apoll_cache, io_apoll_cache_free);
io_alloc_cache_free(&ctx->netmsg_cache, io_netmsg_cache_free);
+ io_futex_cache_free(ctx);
io_destroy_buffers(ctx);
mutex_unlock(&ctx->uring_lock);
if (ctx->sq_creds)
@@ -3306,6 +3312,7 @@ static __cold bool io_uring_try_cancel_requests(struct io_ring_ctx *ctx,
mutex_lock(&ctx->uring_lock);
ret |= io_poll_remove_all(ctx, task, cancel_all);
ret |= io_waitid_remove_all(ctx, task, cancel_all);
+ ret |= io_futex_remove_all(ctx, task, cancel_all);
mutex_unlock(&ctx->uring_lock);
ret |= io_kill_timeouts(ctx, task, cancel_all);
if (task)
diff --git a/io_uring/opdef.c b/io_uring/opdef.c
index aadcbf7136b0..31a3a421e94d 100644
--- a/io_uring/opdef.c
+++ b/io_uring/opdef.c
@@ -34,6 +34,7 @@
#include "cancel.h"
#include "rw.h"
#include "waitid.h"
+#include "futex.h"
static int io_no_issue(struct io_kiocb *req, unsigned int issue_flags)
{
@@ -444,6 +445,22 @@ const struct io_issue_def io_issue_defs[] = {
.prep = io_waitid_prep,
.issue = io_waitid,
},
+ [IORING_OP_FUTEX_WAIT] = {
+#if defined(CONFIG_FUTEX)
+ .prep = io_futex_prep,
+ .issue = io_futex_wait,
+#else
+ .prep = io_eopnotsupp_prep,
+#endif
+ },
+ [IORING_OP_FUTEX_WAKE] = {
+#if defined(CONFIG_FUTEX)
+ .prep = io_futex_prep,
+ .issue = io_futex_wake,
+#else
+ .prep = io_eopnotsupp_prep,
+#endif
+ },
};
const struct io_cold_def io_cold_defs[] = {
@@ -670,6 +687,12 @@ const struct io_cold_def io_cold_defs[] = {
.name = "WAITID",
.async_size = sizeof(struct io_waitid_async),
},
+ [IORING_OP_FUTEX_WAIT] = {
+ .name = "FUTEX_WAIT",
+ },
+ [IORING_OP_FUTEX_WAKE] = {
+ .name = "FUTEX_WAKE",
+ },
};
const char *io_uring_get_opcode(u8 opcode)
--
2.40.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 4/8] io_uring: add support for futex wake and wait
2023-09-21 18:29 ` [PATCH 4/8] io_uring: add support for futex wake and wait Jens Axboe
@ 2023-09-27 9:05 ` Peter Zijlstra
2023-09-27 12:05 ` Jens Axboe
0 siblings, 1 reply; 21+ messages in thread
From: Peter Zijlstra @ 2023-09-27 9:05 UTC (permalink / raw)
To: Jens Axboe; +Cc: io-uring, linux-kernel, andres, tglx
On Thu, Sep 21, 2023 at 12:29:04PM -0600, Jens Axboe wrote:
> +int io_futex_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
> +{
> + struct io_futex *iof = io_kiocb_to_cmd(req, struct io_futex);
> + u32 flags;
> +
> + if (unlikely(sqe->fd || sqe->len || sqe->buf_index || sqe->file_index))
> + return -EINVAL;
> +
> + iof->uaddr = u64_to_user_ptr(READ_ONCE(sqe->addr));
> + iof->futex_val = READ_ONCE(sqe->addr2);
> + iof->futex_mask = READ_ONCE(sqe->addr3);
> + flags = READ_ONCE(sqe->futex_flags);
> +
> + if (flags & ~FUTEX2_VALID_MASK)
> + return -EINVAL;
> +
> + iof->futex_flags = futex2_to_flags(flags);
So prep does the flags conversion..
> + if (!futex_flags_valid(iof->futex_flags))
> + return -EINVAL;
> +
> + if (!futex_validate_input(iof->futex_flags, iof->futex_val) ||
> + !futex_validate_input(iof->futex_flags, iof->futex_mask))
> + return -EINVAL;
> +
> + return 0;
> +}
> +int io_futex_wait(struct io_kiocb *req, unsigned int issue_flags)
> +{
> + struct io_futex *iof = io_kiocb_to_cmd(req, struct io_futex);
> + struct io_ring_ctx *ctx = req->ctx;
> + struct io_futex_data *ifd = NULL;
> + struct futex_hash_bucket *hb;
> + int ret;
> +
> + if (!iof->futex_mask) {
> + ret = -EINVAL;
> + goto done;
> + }
> +
> + io_ring_submit_lock(ctx, issue_flags);
> + ifd = io_alloc_ifd(ctx);
> + if (!ifd) {
> + ret = -ENOMEM;
> + goto done_unlock;
> + }
> +
> + req->async_data = ifd;
> + ifd->q = futex_q_init;
> + ifd->q.bitset = iof->futex_mask;
> + ifd->q.wake = io_futex_wake_fn;
> + ifd->req = req;
> +
> + ret = futex_wait_setup(iof->uaddr, iof->futex_val,
> + futex2_to_flags(iof->futex_flags), &ifd->q, &hb);
But then wait and..
> + if (!ret) {
> + hlist_add_head(&req->hash_node, &ctx->futex_list);
> + io_ring_submit_unlock(ctx, issue_flags);
> +
> + futex_queue(&ifd->q, hb);
> + return IOU_ISSUE_SKIP_COMPLETE;
> + }
> +
> +done_unlock:
> + io_ring_submit_unlock(ctx, issue_flags);
> +done:
> + if (ret < 0)
> + req_set_fail(req);
> + io_req_set_res(req, ret, 0);
> + kfree(ifd);
> + return IOU_OK;
> +}
> +
> +int io_futex_wake(struct io_kiocb *req, unsigned int issue_flags)
> +{
> + struct io_futex *iof = io_kiocb_to_cmd(req, struct io_futex);
> + int ret;
> +
> + ret = futex_wake(iof->uaddr, futex2_to_flags(iof->futex_flags),
... wake do it both again?
Also, I think we want wake to have wake do:
'FLAGS_STRICT | iof->futex_flags'
See 43adf8449510 ("futex: FLAGS_STRICT"), I'm thinking that waking 0
futexes should honour that request by waking 0, not 1 :-)
> + iof->futex_val, iof->futex_mask);
> + if (ret < 0)
> + req_set_fail(req);
> + io_req_set_res(req, ret, 0);
> + return IOU_OK;
> +}
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 4/8] io_uring: add support for futex wake and wait
2023-09-27 9:05 ` Peter Zijlstra
@ 2023-09-27 12:05 ` Jens Axboe
0 siblings, 0 replies; 21+ messages in thread
From: Jens Axboe @ 2023-09-27 12:05 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: io-uring, linux-kernel, andres, tglx
On 9/27/23 3:05 AM, Peter Zijlstra wrote:
> On Thu, Sep 21, 2023 at 12:29:04PM -0600, Jens Axboe wrote:
>
>> +int io_futex_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
>> +{
>> + struct io_futex *iof = io_kiocb_to_cmd(req, struct io_futex);
>> + u32 flags;
>> +
>> + if (unlikely(sqe->fd || sqe->len || sqe->buf_index || sqe->file_index))
>> + return -EINVAL;
>> +
>> + iof->uaddr = u64_to_user_ptr(READ_ONCE(sqe->addr));
>> + iof->futex_val = READ_ONCE(sqe->addr2);
>> + iof->futex_mask = READ_ONCE(sqe->addr3);
>> + flags = READ_ONCE(sqe->futex_flags);
>> +
>> + if (flags & ~FUTEX2_VALID_MASK)
>> + return -EINVAL;
>> +
>> + iof->futex_flags = futex2_to_flags(flags);
>
> So prep does the flags conversion..
>
>> + if (!futex_flags_valid(iof->futex_flags))
>> + return -EINVAL;
>> +
>> + if (!futex_validate_input(iof->futex_flags, iof->futex_val) ||
>> + !futex_validate_input(iof->futex_flags, iof->futex_mask))
>> + return -EINVAL;
>> +
>> + return 0;
>> +}
>
>> +int io_futex_wait(struct io_kiocb *req, unsigned int issue_flags)
>> +{
>> + struct io_futex *iof = io_kiocb_to_cmd(req, struct io_futex);
>> + struct io_ring_ctx *ctx = req->ctx;
>> + struct io_futex_data *ifd = NULL;
>> + struct futex_hash_bucket *hb;
>> + int ret;
>> +
>> + if (!iof->futex_mask) {
>> + ret = -EINVAL;
>> + goto done;
>> + }
>> +
>> + io_ring_submit_lock(ctx, issue_flags);
>> + ifd = io_alloc_ifd(ctx);
>> + if (!ifd) {
>> + ret = -ENOMEM;
>> + goto done_unlock;
>> + }
>> +
>> + req->async_data = ifd;
>> + ifd->q = futex_q_init;
>> + ifd->q.bitset = iof->futex_mask;
>> + ifd->q.wake = io_futex_wake_fn;
>> + ifd->req = req;
>> +
>> + ret = futex_wait_setup(iof->uaddr, iof->futex_val,
>> + futex2_to_flags(iof->futex_flags), &ifd->q, &hb);
>
> But then wait and..
>
>> + if (!ret) {
>> + hlist_add_head(&req->hash_node, &ctx->futex_list);
>> + io_ring_submit_unlock(ctx, issue_flags);
>> +
>> + futex_queue(&ifd->q, hb);
>> + return IOU_ISSUE_SKIP_COMPLETE;
>> + }
>> +
>> +done_unlock:
>> + io_ring_submit_unlock(ctx, issue_flags);
>> +done:
>> + if (ret < 0)
>> + req_set_fail(req);
>> + io_req_set_res(req, ret, 0);
>> + kfree(ifd);
>> + return IOU_OK;
>> +}
>> +
>> +int io_futex_wake(struct io_kiocb *req, unsigned int issue_flags)
>> +{
>> + struct io_futex *iof = io_kiocb_to_cmd(req, struct io_futex);
>> + int ret;
>> +
>> + ret = futex_wake(iof->uaddr, futex2_to_flags(iof->futex_flags),
>
> ... wake do it both again?
Oops good catch, yes just the prep side should do it of course. I'll fix
that up.
> Also, I think we want wake to have wake do:
>
> 'FLAGS_STRICT | iof->futex_flags'
>
> See 43adf8449510 ("futex: FLAGS_STRICT"), I'm thinking that waking 0
> futexes should honour that request by waking 0, not 1 :-)
Thanks for the pointer, yeah agree that sounds sane. Most syscalls that
take an number/size that is zero will indeed return zero. I'll add a
test case for that too.
--
Jens Axboe
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 5/8] futex: add wake_data to struct futex_q
2023-09-21 18:29 [PATCHSET v5] Add io_uring futex/futexv support Jens Axboe
` (3 preceding siblings ...)
2023-09-21 18:29 ` [PATCH 4/8] io_uring: add support for futex wake and wait Jens Axboe
@ 2023-09-21 18:29 ` Jens Axboe
2023-09-21 18:29 ` [PATCH 6/8] futex: make futex_parse_waitv() available as a helper Jens Axboe
` (2 subsequent siblings)
7 siblings, 0 replies; 21+ messages in thread
From: Jens Axboe @ 2023-09-21 18:29 UTC (permalink / raw)
To: io-uring, linux-kernel; +Cc: peterz, andres, tglx, Jens Axboe
With handling multiple futex_q for waitv, we cannot easily go from the
futex_q to data related to that request or queue. Add a wake_data
argument that belongs to the wake handler assigned.
Signed-off-by: Jens Axboe <[email protected]>
---
kernel/futex/futex.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/kernel/futex/futex.h b/kernel/futex/futex.h
index 33835b81e0c3..76f6c2e0f539 100644
--- a/kernel/futex/futex.h
+++ b/kernel/futex/futex.h
@@ -148,6 +148,7 @@ typedef void (futex_wake_fn)(struct wake_q_head *wake_q, struct futex_q *q);
* @task: the task waiting on the futex
* @lock_ptr: the hash bucket lock
* @wake: the wake handler for this queue
+ * @wake_data: data associated with the wake handler
* @key: the key the futex is hashed on
* @pi_state: optional priority inheritance state
* @rt_waiter: rt_waiter storage for use with requeue_pi
@@ -173,6 +174,7 @@ struct futex_q {
struct task_struct *task;
spinlock_t *lock_ptr;
futex_wake_fn *wake;
+ void *wake_data;
union futex_key key;
struct futex_pi_state *pi_state;
struct rt_mutex_waiter *rt_waiter;
--
2.40.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 6/8] futex: make futex_parse_waitv() available as a helper
2023-09-21 18:29 [PATCHSET v5] Add io_uring futex/futexv support Jens Axboe
` (4 preceding siblings ...)
2023-09-21 18:29 ` [PATCH 5/8] futex: add wake_data to struct futex_q Jens Axboe
@ 2023-09-21 18:29 ` Jens Axboe
2023-09-21 18:29 ` [PATCH 7/8] futex: make the vectored futex operations available Jens Axboe
2023-09-21 18:29 ` [PATCH 8/8] io_uring: add support for vectored futex waits Jens Axboe
7 siblings, 0 replies; 21+ messages in thread
From: Jens Axboe @ 2023-09-21 18:29 UTC (permalink / raw)
To: io-uring, linux-kernel; +Cc: peterz, andres, tglx, Jens Axboe
To make it more generically useful, augment it with allowing the caller
to pass in the wake handler and wake data. Convert the futex_waitv()
syscall, passing in the default handlers.
Signed-off-by: Jens Axboe <[email protected]>
---
kernel/futex/futex.h | 5 +++++
kernel/futex/syscalls.c | 16 +++++++++++-----
2 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/kernel/futex/futex.h b/kernel/futex/futex.h
index 76f6c2e0f539..6b6a6b3da103 100644
--- a/kernel/futex/futex.h
+++ b/kernel/futex/futex.h
@@ -361,6 +361,11 @@ struct futex_vector {
struct futex_q q;
};
+extern int futex_parse_waitv(struct futex_vector *futexv,
+ struct futex_waitv __user *uwaitv,
+ unsigned int nr_futexes, futex_wake_fn *wake,
+ void *wake_data);
+
extern int futex_wait_multiple(struct futex_vector *vs, unsigned int count,
struct hrtimer_sleeper *to);
diff --git a/kernel/futex/syscalls.c b/kernel/futex/syscalls.c
index 2b5cafdfdc50..4b6da9116aa6 100644
--- a/kernel/futex/syscalls.c
+++ b/kernel/futex/syscalls.c
@@ -184,12 +184,15 @@ SYSCALL_DEFINE6(futex, u32 __user *, uaddr, int, op, u32, val,
* @futexv: Kernel side list of waiters to be filled
* @uwaitv: Userspace list to be parsed
* @nr_futexes: Length of futexv
+ * @wake: Wake to call when futex is woken
+ * @wake_data: Data for the wake handler
*
* Return: Error code on failure, 0 on success
*/
-static int futex_parse_waitv(struct futex_vector *futexv,
- struct futex_waitv __user *uwaitv,
- unsigned int nr_futexes)
+int futex_parse_waitv(struct futex_vector *futexv,
+ struct futex_waitv __user *uwaitv,
+ unsigned int nr_futexes, futex_wake_fn *wake,
+ void *wake_data)
{
struct futex_waitv aux;
unsigned int i;
@@ -214,6 +217,8 @@ static int futex_parse_waitv(struct futex_vector *futexv,
futexv[i].w.val = aux.val;
futexv[i].w.uaddr = aux.uaddr;
futexv[i].q = futex_q_init;
+ futexv[i].q.wake = wake;
+ futexv[i].q.wake_data = wake_data;
}
return 0;
@@ -306,7 +311,8 @@ SYSCALL_DEFINE5(futex_waitv, struct futex_waitv __user *, waiters,
goto destroy_timer;
}
- ret = futex_parse_waitv(futexv, waiters, nr_futexes);
+ ret = futex_parse_waitv(futexv, waiters, nr_futexes, futex_wake_mark,
+ NULL);
if (!ret)
ret = futex_wait_multiple(futexv, nr_futexes, timeout ? &to : NULL);
@@ -421,7 +427,7 @@ SYSCALL_DEFINE4(futex_requeue,
if (!waiters)
return -EINVAL;
- ret = futex_parse_waitv(futexes, waiters, 2);
+ ret = futex_parse_waitv(futexes, waiters, 2, futex_wake_mark, NULL);
if (ret)
return ret;
--
2.40.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 7/8] futex: make the vectored futex operations available
2023-09-21 18:29 [PATCHSET v5] Add io_uring futex/futexv support Jens Axboe
` (5 preceding siblings ...)
2023-09-21 18:29 ` [PATCH 6/8] futex: make futex_parse_waitv() available as a helper Jens Axboe
@ 2023-09-21 18:29 ` Jens Axboe
2023-09-21 18:29 ` [PATCH 8/8] io_uring: add support for vectored futex waits Jens Axboe
7 siblings, 0 replies; 21+ messages in thread
From: Jens Axboe @ 2023-09-21 18:29 UTC (permalink / raw)
To: io-uring, linux-kernel; +Cc: peterz, andres, tglx, Jens Axboe
Rename unqueue_multiple() as futex_unqueue_multiple(), and make both
that and futex_wait_multiple_setup() available for external users. This
is in preparation for wiring up vectored waits in io_uring.
Signed-off-by: Jens Axboe <[email protected]>
---
kernel/futex/futex.h | 5 +++++
kernel/futex/waitwake.c | 10 +++++-----
2 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/kernel/futex/futex.h b/kernel/futex/futex.h
index 6b6a6b3da103..8b195d06f4e8 100644
--- a/kernel/futex/futex.h
+++ b/kernel/futex/futex.h
@@ -366,6 +366,11 @@ extern int futex_parse_waitv(struct futex_vector *futexv,
unsigned int nr_futexes, futex_wake_fn *wake,
void *wake_data);
+extern int futex_wait_multiple_setup(struct futex_vector *vs, int count,
+ int *woken);
+
+extern int futex_unqueue_multiple(struct futex_vector *v, int count);
+
extern int futex_wait_multiple(struct futex_vector *vs, unsigned int count,
struct hrtimer_sleeper *to);
diff --git a/kernel/futex/waitwake.c b/kernel/futex/waitwake.c
index 6fcf5f723719..61b112897a84 100644
--- a/kernel/futex/waitwake.c
+++ b/kernel/futex/waitwake.c
@@ -372,7 +372,7 @@ void futex_wait_queue(struct futex_hash_bucket *hb, struct futex_q *q,
}
/**
- * unqueue_multiple - Remove various futexes from their hash bucket
+ * futex_unqueue_multiple - Remove various futexes from their hash bucket
* @v: The list of futexes to unqueue
* @count: Number of futexes in the list
*
@@ -382,7 +382,7 @@ void futex_wait_queue(struct futex_hash_bucket *hb, struct futex_q *q,
* - >=0 - Index of the last futex that was awoken;
* - -1 - No futex was awoken
*/
-static int unqueue_multiple(struct futex_vector *v, int count)
+int futex_unqueue_multiple(struct futex_vector *v, int count)
{
int ret = -1, i;
@@ -410,7 +410,7 @@ static int unqueue_multiple(struct futex_vector *v, int count)
* - 0 - Success
* - <0 - -EFAULT, -EWOULDBLOCK or -EINVAL
*/
-static int futex_wait_multiple_setup(struct futex_vector *vs, int count, int *woken)
+int futex_wait_multiple_setup(struct futex_vector *vs, int count, int *woken)
{
struct futex_hash_bucket *hb;
bool retry = false;
@@ -472,7 +472,7 @@ static int futex_wait_multiple_setup(struct futex_vector *vs, int count, int *wo
* was woken, we don't return error and return this index to
* userspace
*/
- *woken = unqueue_multiple(vs, i);
+ *woken = futex_unqueue_multiple(vs, i);
if (*woken >= 0)
return 1;
@@ -557,7 +557,7 @@ int futex_wait_multiple(struct futex_vector *vs, unsigned int count,
__set_current_state(TASK_RUNNING);
- ret = unqueue_multiple(vs, count);
+ ret = futex_unqueue_multiple(vs, count);
if (ret >= 0)
return ret;
--
2.40.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 8/8] io_uring: add support for vectored futex waits
2023-09-21 18:29 [PATCHSET v5] Add io_uring futex/futexv support Jens Axboe
` (6 preceding siblings ...)
2023-09-21 18:29 ` [PATCH 7/8] futex: make the vectored futex operations available Jens Axboe
@ 2023-09-21 18:29 ` Jens Axboe
2023-09-26 6:41 ` Jens Axboe
7 siblings, 1 reply; 21+ messages in thread
From: Jens Axboe @ 2023-09-21 18:29 UTC (permalink / raw)
To: io-uring, linux-kernel; +Cc: peterz, andres, tglx, Jens Axboe
This adds support for IORING_OP_FUTEX_WAITV, which allows registering a
notification for a number of futexes at once. If one of the futexes are
woken, then the request will complete with the index of the futex that got
woken as the result. This is identical to what the normal vectored futex
waitv operation does.
Use like IORING_OP_FUTEX_WAIT, except sqe->addr must now contain the a
pointer to a struct futex_waitv array, and sqe->off must now contain the
number of elements in that array.
For cancelations, FUTEX_WAITV does not rely on the futex_unqueue()
return value as we're dealing with multiple futexes. Instead, a separate
per io_uring request atomic is used to claim ownership of the request.
Waiting on N futexes could be done with IORING_OP_FUTEX_WAIT as well,
but that punts a lot of the work to the application:
1) Application would need to submit N IORING_OP_FUTEX_WAIT requests,
rather than just a single IORING_OP_FUTEX_WAITV.
2) When one futex is woken, application would need to cancel the
remaining N-1 requests that didn't trigger.
While this is of course doable, having a single vectored futex wait
makes for much simpler application code.
Signed-off-by: Jens Axboe <[email protected]>
---
include/uapi/linux/io_uring.h | 1 +
io_uring/futex.c | 164 ++++++++++++++++++++++++++++++++--
io_uring/futex.h | 2 +
io_uring/opdef.c | 11 +++
4 files changed, 169 insertions(+), 9 deletions(-)
diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
index 4ddd7bdbbfb8..172472626f5b 100644
--- a/include/uapi/linux/io_uring.h
+++ b/include/uapi/linux/io_uring.h
@@ -246,6 +246,7 @@ enum io_uring_op {
IORING_OP_WAITID,
IORING_OP_FUTEX_WAIT,
IORING_OP_FUTEX_WAKE,
+ IORING_OP_FUTEX_WAITV,
/* this goes last, obviously */
IORING_OP_LAST,
diff --git a/io_uring/futex.c b/io_uring/futex.c
index 4278302d212c..0c07df8668aa 100644
--- a/io_uring/futex.c
+++ b/io_uring/futex.c
@@ -14,10 +14,15 @@
struct io_futex {
struct file *file;
- u32 __user *uaddr;
+ union {
+ u32 __user *uaddr;
+ struct futex_waitv __user *uwaitv;
+ };
unsigned long futex_val;
unsigned long futex_mask;
+ unsigned long futexv_owned;
u32 futex_flags;
+ unsigned int futex_nr;
};
struct io_futex_data {
@@ -44,6 +49,13 @@ void io_futex_cache_free(struct io_ring_ctx *ctx)
io_alloc_cache_free(&ctx->futex_cache, io_futex_cache_entry_free);
}
+static void __io_futex_complete(struct io_kiocb *req, struct io_tw_state *ts)
+{
+ req->async_data = NULL;
+ hlist_del_init(&req->hash_node);
+ io_req_task_complete(req, ts);
+}
+
static void io_futex_complete(struct io_kiocb *req, struct io_tw_state *ts)
{
struct io_futex_data *ifd = req->async_data;
@@ -52,22 +64,59 @@ static void io_futex_complete(struct io_kiocb *req, struct io_tw_state *ts)
io_tw_lock(ctx, ts);
if (!io_alloc_cache_put(&ctx->futex_cache, &ifd->cache))
kfree(ifd);
- req->async_data = NULL;
- hlist_del_init(&req->hash_node);
- io_req_task_complete(req, ts);
+ __io_futex_complete(req, ts);
}
-static bool __io_futex_cancel(struct io_ring_ctx *ctx, struct io_kiocb *req)
+static void io_futexv_complete(struct io_kiocb *req, struct io_tw_state *ts)
{
- struct io_futex_data *ifd = req->async_data;
+ struct io_futex *iof = io_kiocb_to_cmd(req, struct io_futex);
+ struct futex_vector *futexv = req->async_data;
+ struct io_ring_ctx *ctx = req->ctx;
+ int res = 0;
- /* futex wake already done or in progress */
- if (!futex_unqueue(&ifd->q))
+ io_tw_lock(ctx, ts);
+
+ res = futex_unqueue_multiple(futexv, iof->futex_nr);
+ if (res != -1)
+ io_req_set_res(req, res, 0);
+
+ kfree(req->async_data);
+ req->flags &= ~REQ_F_ASYNC_DATA;
+ __io_futex_complete(req, ts);
+}
+
+static bool io_futexv_claimed(struct io_futex *iof)
+{
+ return test_bit(0, &iof->futexv_owned);
+}
+
+static bool io_futexv_claim(struct io_futex *iof)
+{
+ if (test_bit(0, &iof->futexv_owned) ||
+ test_and_set_bit(0, &iof->futexv_owned))
return false;
+ return true;
+}
+
+static bool __io_futex_cancel(struct io_ring_ctx *ctx, struct io_kiocb *req)
+{
+ /* futex wake already done or in progress */
+ if (req->opcode == IORING_OP_FUTEX_WAIT) {
+ struct io_futex_data *ifd = req->async_data;
+
+ if (!futex_unqueue(&ifd->q))
+ return false;
+ req->io_task_work.func = io_futex_complete;
+ } else {
+ struct io_futex *iof = io_kiocb_to_cmd(req, struct io_futex);
+
+ if (!io_futexv_claim(iof))
+ return false;
+ req->io_task_work.func = io_futexv_complete;
+ }
hlist_del_init(&req->hash_node);
io_req_set_res(req, -ECANCELED, 0);
- req->io_task_work.func = io_futex_complete;
io_req_task_work_add(req);
return true;
}
@@ -146,6 +195,54 @@ int io_futex_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
return 0;
}
+static void io_futex_wakev_fn(struct wake_q_head *wake_q, struct futex_q *q)
+{
+ struct io_kiocb *req = q->wake_data;
+ struct io_futex *iof = io_kiocb_to_cmd(req, struct io_futex);
+
+ if (!io_futexv_claim(iof))
+ return;
+ if (unlikely(!__futex_wake_mark(q)))
+ return;
+
+ io_req_set_res(req, 0, 0);
+ req->io_task_work.func = io_futexv_complete;
+ io_req_task_work_add(req);
+}
+
+int io_futexv_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
+{
+ struct io_futex *iof = io_kiocb_to_cmd(req, struct io_futex);
+ struct futex_vector *futexv;
+ int ret;
+
+ /* No flags or mask supported for waitv */
+ if (unlikely(sqe->fd || sqe->buf_index || sqe->file_index ||
+ sqe->addr2 || sqe->addr3))
+ return -EINVAL;
+
+ iof->uaddr = u64_to_user_ptr(READ_ONCE(sqe->addr));
+ iof->futex_nr = READ_ONCE(sqe->len);
+ if (!iof->futex_nr || iof->futex_nr > FUTEX_WAITV_MAX)
+ return -EINVAL;
+
+ futexv = kcalloc(iof->futex_nr, sizeof(*futexv), GFP_KERNEL);
+ if (!futexv)
+ return -ENOMEM;
+
+ ret = futex_parse_waitv(futexv, iof->uwaitv, iof->futex_nr,
+ io_futex_wakev_fn, req);
+ if (ret) {
+ kfree(futexv);
+ return ret;
+ }
+
+ iof->futexv_owned = 0;
+ req->flags |= REQ_F_ASYNC_DATA;
+ req->async_data = futexv;
+ return 0;
+}
+
static void io_futex_wake_fn(struct wake_q_head *wake_q, struct futex_q *q)
{
struct io_futex_data *ifd = container_of(q, struct io_futex_data, q);
@@ -170,6 +267,55 @@ static struct io_futex_data *io_alloc_ifd(struct io_ring_ctx *ctx)
return kmalloc(sizeof(struct io_futex_data), GFP_NOWAIT);
}
+int io_futexv_wait(struct io_kiocb *req, unsigned int issue_flags)
+{
+ struct io_futex *iof = io_kiocb_to_cmd(req, struct io_futex);
+ struct futex_vector *futexv = req->async_data;
+ struct io_ring_ctx *ctx = req->ctx;
+ int ret, woken = -1;
+
+ io_ring_submit_lock(ctx, issue_flags);
+
+ ret = futex_wait_multiple_setup(futexv, iof->futex_nr, &woken);
+
+ /*
+ * The above call leaves us potentially non-running. This is fine
+ * for the sync syscall as it'll be blocking unless we already got
+ * one of the futexes woken, but it obviously won't work for an async
+ * invocation. Mark us runnable again.
+ */
+ __set_current_state(TASK_RUNNING);
+
+ /*
+ * We got woken while setting up, let that side do the completion
+ */
+ if (io_futexv_claimed(iof)) {
+skip:
+ io_ring_submit_unlock(ctx, issue_flags);
+ return IOU_ISSUE_SKIP_COMPLETE;
+ }
+
+ /*
+ * 0 return means that we successfully setup the waiters, and that
+ * nobody triggered a wakeup while we were doing so. < 0 or 1 return
+ * is either an error or we got a wakeup while setting up.
+ */
+ if (!ret) {
+ hlist_add_head(&req->hash_node, &ctx->futex_list);
+ goto skip;
+ }
+
+ io_ring_submit_unlock(ctx, issue_flags);
+ if (ret < 0)
+ req_set_fail(req);
+ else if (woken != -1)
+ ret = woken;
+ io_req_set_res(req, ret, 0);
+ kfree(futexv);
+ req->flags &= ~REQ_F_ASYNC_DATA;
+ return IOU_OK;
+}
+
int io_futex_wait(struct io_kiocb *req, unsigned int issue_flags)
{
struct io_futex *iof = io_kiocb_to_cmd(req, struct io_futex);
diff --git a/io_uring/futex.h b/io_uring/futex.h
index ddc9e0d73c52..0847e9e8a127 100644
--- a/io_uring/futex.h
+++ b/io_uring/futex.h
@@ -3,7 +3,9 @@
#include "cancel.h"
int io_futex_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe);
+int io_futexv_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe);
int io_futex_wait(struct io_kiocb *req, unsigned int issue_flags);
+int io_futexv_wait(struct io_kiocb *req, unsigned int issue_flags);
int io_futex_wake(struct io_kiocb *req, unsigned int issue_flags);
#if defined(CONFIG_FUTEX)
diff --git a/io_uring/opdef.c b/io_uring/opdef.c
index 31a3a421e94d..25a3515a177c 100644
--- a/io_uring/opdef.c
+++ b/io_uring/opdef.c
@@ -459,6 +459,14 @@ const struct io_issue_def io_issue_defs[] = {
.issue = io_futex_wake,
#else
.prep = io_eopnotsupp_prep,
+#endif
+ },
+ [IORING_OP_FUTEX_WAITV] = {
+#if defined(CONFIG_FUTEX)
+ .prep = io_futexv_prep,
+ .issue = io_futexv_wait,
+#else
+ .prep = io_eopnotsupp_prep,
#endif
},
};
@@ -693,6 +701,9 @@ const struct io_cold_def io_cold_defs[] = {
[IORING_OP_FUTEX_WAKE] = {
.name = "FUTEX_WAKE",
},
+ [IORING_OP_FUTEX_WAITV] = {
+ .name = "FUTEX_WAITV",
+ },
};
const char *io_uring_get_opcode(u8 opcode)
--
2.40.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 8/8] io_uring: add support for vectored futex waits
2023-09-21 18:29 ` [PATCH 8/8] io_uring: add support for vectored futex waits Jens Axboe
@ 2023-09-26 6:41 ` Jens Axboe
2023-09-26 15:25 ` Jens Axboe
0 siblings, 1 reply; 21+ messages in thread
From: Jens Axboe @ 2023-09-26 6:41 UTC (permalink / raw)
To: io-uring, linux-kernel; +Cc: peterz, andres, tglx
After discussing this one with Thomas yesterday at KR2023 I had this
nagging feeling that something was still amiss. Took a closer look at
it, and there is an issue with the odd case of
futex_wait_multiple_setup() returning 1. It does so if a wakeup was
triggered during setup. Which is fine, except then it also unqueues ALL
the futexes at that point, which is unlike the normal wakeup path on the
io_uring side.
It'd be nice to unify those and leave the cleanup to the caller, but
since we also re-loop in that setup handler if nobody was woken AND we
use the futex_unqueue_multiple() to see if we were woken to begin with,
I think it's cleaner to just note this fact in io_uring and deal with
it.
I'm folding in the below incremental for now. Has a few cleanups in
there too that I spotted while doing that, the important bit is the
->futexv_unqueued part.
diff --git a/io_uring/futex.c b/io_uring/futex.c
index 0c07df8668aa..8a2b0a260d5b 100644
--- a/io_uring/futex.c
+++ b/io_uring/futex.c
@@ -23,6 +23,7 @@ struct io_futex {
unsigned long futexv_owned;
u32 futex_flags;
unsigned int futex_nr;
+ bool futexv_unqueued;
};
struct io_futex_data {
@@ -71,25 +72,21 @@ static void io_futexv_complete(struct io_kiocb *req, struct io_tw_state *ts)
{
struct io_futex *iof = io_kiocb_to_cmd(req, struct io_futex);
struct futex_vector *futexv = req->async_data;
- struct io_ring_ctx *ctx = req->ctx;
- int res = 0;
- io_tw_lock(ctx, ts);
+ io_tw_lock(req->ctx, ts);
+
+ if (!iof->futexv_unqueued) {
+ int res = futex_unqueue_multiple(futexv, iof->futex_nr);
- res = futex_unqueue_multiple(futexv, iof->futex_nr);
- if (res != -1)
- io_req_set_res(req, res, 0);
+ if (res != -1)
+ io_req_set_res(req, res, 0);
+ }
kfree(req->async_data);
req->flags &= ~REQ_F_ASYNC_DATA;
__io_futex_complete(req, ts);
}
-static bool io_futexv_claimed(struct io_futex *iof)
-{
- return test_bit(0, &iof->futexv_owned);
-}
-
static bool io_futexv_claim(struct io_futex *iof)
{
if (test_bit(0, &iof->futexv_owned) ||
@@ -238,6 +235,7 @@ int io_futexv_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
}
iof->futexv_owned = 0;
+ iof->futexv_unqueued = 0;
req->flags |= REQ_F_ASYNC_DATA;
req->async_data = futexv;
return 0;
@@ -278,6 +276,18 @@ int io_futexv_wait(struct io_kiocb *req, unsigned int issue_flags)
ret = futex_wait_multiple_setup(futexv, iof->futex_nr, &woken);
+ /*
+ * We got woken while setting up, let that side do the completion.
+ * Note that futex_wait_multiple_setup() will have unqueued all
+ * the futexes in this case. Mark us as having done that already,
+ * since this is different from normal wakeup.
+ */
+ if (ret == 1) {
+ iof->futexv_unqueued = 1;
+ io_req_set_res(req, woken, 0);
+ goto skip;
+ }
+
/*
* The above call leaves us potentially non-running. This is fine
* for the sync syscall as it'll be blocking unless we already got
@@ -287,29 +297,23 @@ int io_futexv_wait(struct io_kiocb *req, unsigned int issue_flags)
__set_current_state(TASK_RUNNING);
/*
- * We got woken while setting up, let that side do the completion
+ * 0 return means that we successfully setup the waiters, and that
+ * nobody triggered a wakeup while we were doing so. If the wakeup
+ * happened post setup, the task_work will be run post this issue
+ * and under the submission lock.
*/
- if (io_futexv_claimed(iof)) {
+ if (!ret) {
+ hlist_add_head(&req->hash_node, &ctx->futex_list);
skip:
io_ring_submit_unlock(ctx, issue_flags);
return IOU_ISSUE_SKIP_COMPLETE;
}
/*
- * 0 return means that we successfully setup the waiters, and that
- * nobody triggered a wakeup while we were doing so. < 0 or 1 return
- * is either an error or we got a wakeup while setting up.
+ * Error case, ret is < 0. Mark the request as failed.
*/
- if (!ret) {
- hlist_add_head(&req->hash_node, &ctx->futex_list);
- goto skip;
- }
-
io_ring_submit_unlock(ctx, issue_flags);
- if (ret < 0)
- req_set_fail(req);
- else if (woken != -1)
- ret = woken;
+ req_set_fail(req);
io_req_set_res(req, ret, 0);
kfree(futexv);
req->flags &= ~REQ_F_ASYNC_DATA;
--
Jens Axboe
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 8/8] io_uring: add support for vectored futex waits
2023-09-26 6:41 ` Jens Axboe
@ 2023-09-26 15:25 ` Jens Axboe
0 siblings, 0 replies; 21+ messages in thread
From: Jens Axboe @ 2023-09-26 15:25 UTC (permalink / raw)
To: io-uring, linux-kernel; +Cc: peterz, andres, tglx
On 9/26/23 12:41 AM, Jens Axboe wrote:
> After discussing this one with Thomas yesterday at KR2023 I had this
> nagging feeling that something was still amiss. Took a closer look at
> it, and there is an issue with the odd case of
> futex_wait_multiple_setup() returning 1. It does so if a wakeup was
> triggered during setup. Which is fine, except then it also unqueues ALL
> the futexes at that point, which is unlike the normal wakeup path on the
> io_uring side.
>
> It'd be nice to unify those and leave the cleanup to the caller, but
> since we also re-loop in that setup handler if nobody was woken AND we
> use the futex_unqueue_multiple() to see if we were woken to begin with,
> I think it's cleaner to just note this fact in io_uring and deal with
> it.
>
> I'm folding in the below incremental for now. Has a few cleanups in
> there too that I spotted while doing that, the important bit is the
> ->futexv_unqueued part.
For completeness, here's the V2 of this patch. Did a few more cleanups
since changes were being made. This is what is in the io_uring-futex
branch as of earlier today. Didn't think it was worthwhile to spin a new
full patchset version for this and post it.
commit 61f8c9c0b48632ea1f14519dfae9176ffbf65050
Author: Jens Axboe <[email protected]>
Date: Mon Jun 12 19:04:32 2023 -0600
io_uring: add support for vectored futex waits
This adds support for IORING_OP_FUTEX_WAITV, which allows registering a
notification for a number of futexes at once. If one of the futexes are
woken, then the request will complete with the index of the futex that got
woken as the result. This is identical to what the normal vectored futex
waitv operation does.
Use like IORING_OP_FUTEX_WAIT, except sqe->addr must now contain the a
pointer to a struct futex_waitv array, and sqe->off must now contain the
number of elements in that array.
For cancelations, FUTEX_WAITV does not rely on the futex_unqueue()
return value as we're dealing with multiple futexes. Instead, a separate
per io_uring request atomic is used to claim ownership of the request.
Waiting on N futexes could be done with IORING_OP_FUTEX_WAIT as well,
but that punts a lot of the work to the application:
1) Application would need to submit N IORING_OP_FUTEX_WAIT requests,
rather than just a single IORING_OP_FUTEX_WAITV.
2) When one futex is woken, application would need to cancel the
remaining N-1 requests that didn't trigger.
While this is of course doable, having a single vectored futex wait
makes for much simpler application code.
Signed-off-by: Jens Axboe <[email protected]>
diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
index 4ddd7bdbbfb8..172472626f5b 100644
--- a/include/uapi/linux/io_uring.h
+++ b/include/uapi/linux/io_uring.h
@@ -246,6 +246,7 @@ enum io_uring_op {
IORING_OP_WAITID,
IORING_OP_FUTEX_WAIT,
IORING_OP_FUTEX_WAKE,
+ IORING_OP_FUTEX_WAITV,
/* this goes last, obviously */
IORING_OP_LAST,
diff --git a/io_uring/futex.c b/io_uring/futex.c
index 4278302d212c..ad6578c97ab8 100644
--- a/io_uring/futex.c
+++ b/io_uring/futex.c
@@ -14,10 +14,16 @@
struct io_futex {
struct file *file;
- u32 __user *uaddr;
+ union {
+ u32 __user *uaddr;
+ struct futex_waitv __user *uwaitv;
+ };
unsigned long futex_val;
unsigned long futex_mask;
+ unsigned long futexv_owned;
u32 futex_flags;
+ unsigned int futex_nr;
+ bool futexv_unqueued;
};
struct io_futex_data {
@@ -44,6 +50,13 @@ void io_futex_cache_free(struct io_ring_ctx *ctx)
io_alloc_cache_free(&ctx->futex_cache, io_futex_cache_entry_free);
}
+static void __io_futex_complete(struct io_kiocb *req, struct io_tw_state *ts)
+{
+ req->async_data = NULL;
+ hlist_del_init(&req->hash_node);
+ io_req_task_complete(req, ts);
+}
+
static void io_futex_complete(struct io_kiocb *req, struct io_tw_state *ts)
{
struct io_futex_data *ifd = req->async_data;
@@ -52,22 +65,56 @@ static void io_futex_complete(struct io_kiocb *req, struct io_tw_state *ts)
io_tw_lock(ctx, ts);
if (!io_alloc_cache_put(&ctx->futex_cache, &ifd->cache))
kfree(ifd);
- req->async_data = NULL;
- hlist_del_init(&req->hash_node);
- io_req_task_complete(req, ts);
+ __io_futex_complete(req, ts);
}
-static bool __io_futex_cancel(struct io_ring_ctx *ctx, struct io_kiocb *req)
+static void io_futexv_complete(struct io_kiocb *req, struct io_tw_state *ts)
{
- struct io_futex_data *ifd = req->async_data;
+ struct io_futex *iof = io_kiocb_to_cmd(req, struct io_futex);
+ struct futex_vector *futexv = req->async_data;
- /* futex wake already done or in progress */
- if (!futex_unqueue(&ifd->q))
+ io_tw_lock(req->ctx, ts);
+
+ if (!iof->futexv_unqueued) {
+ int res;
+
+ res = futex_unqueue_multiple(futexv, iof->futex_nr);
+ if (res != -1)
+ io_req_set_res(req, res, 0);
+ }
+
+ kfree(req->async_data);
+ req->flags &= ~REQ_F_ASYNC_DATA;
+ __io_futex_complete(req, ts);
+}
+
+static bool io_futexv_claim(struct io_futex *iof)
+{
+ if (test_bit(0, &iof->futexv_owned) ||
+ test_and_set_bit_lock(0, &iof->futexv_owned))
return false;
+ return true;
+}
+
+static bool __io_futex_cancel(struct io_ring_ctx *ctx, struct io_kiocb *req)
+{
+ /* futex wake already done or in progress */
+ if (req->opcode == IORING_OP_FUTEX_WAIT) {
+ struct io_futex_data *ifd = req->async_data;
+
+ if (!futex_unqueue(&ifd->q))
+ return false;
+ req->io_task_work.func = io_futex_complete;
+ } else {
+ struct io_futex *iof = io_kiocb_to_cmd(req, struct io_futex);
+
+ if (!io_futexv_claim(iof))
+ return false;
+ req->io_task_work.func = io_futexv_complete;
+ }
hlist_del_init(&req->hash_node);
io_req_set_res(req, -ECANCELED, 0);
- req->io_task_work.func = io_futex_complete;
io_req_task_work_add(req);
return true;
}
@@ -146,6 +193,55 @@ int io_futex_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
return 0;
}
+static void io_futex_wakev_fn(struct wake_q_head *wake_q, struct futex_q *q)
+{
+ struct io_kiocb *req = q->wake_data;
+ struct io_futex *iof = io_kiocb_to_cmd(req, struct io_futex);
+
+ if (!io_futexv_claim(iof))
+ return;
+ if (unlikely(!__futex_wake_mark(q)))
+ return;
+
+ io_req_set_res(req, 0, 0);
+ req->io_task_work.func = io_futexv_complete;
+ io_req_task_work_add(req);
+}
+
+int io_futexv_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
+{
+ struct io_futex *iof = io_kiocb_to_cmd(req, struct io_futex);
+ struct futex_vector *futexv;
+ int ret;
+
+ /* No flags or mask supported for waitv */
+ if (unlikely(sqe->fd || sqe->buf_index || sqe->file_index ||
+ sqe->addr2 || sqe->addr3))
+ return -EINVAL;
+
+ iof->uaddr = u64_to_user_ptr(READ_ONCE(sqe->addr));
+ iof->futex_nr = READ_ONCE(sqe->len);
+ if (!iof->futex_nr || iof->futex_nr > FUTEX_WAITV_MAX)
+ return -EINVAL;
+
+ futexv = kcalloc(iof->futex_nr, sizeof(*futexv), GFP_KERNEL);
+ if (!futexv)
+ return -ENOMEM;
+
+ ret = futex_parse_waitv(futexv, iof->uwaitv, iof->futex_nr,
+ io_futex_wakev_fn, req);
+ if (ret) {
+ kfree(futexv);
+ return ret;
+ }
+
+ iof->futexv_owned = 0;
+ iof->futexv_unqueued = 0;
+ req->flags |= REQ_F_ASYNC_DATA;
+ req->async_data = futexv;
+ return 0;
+}
+
static void io_futex_wake_fn(struct wake_q_head *wake_q, struct futex_q *q)
{
struct io_futex_data *ifd = container_of(q, struct io_futex_data, q);
@@ -170,6 +266,61 @@ static struct io_futex_data *io_alloc_ifd(struct io_ring_ctx *ctx)
return kmalloc(sizeof(struct io_futex_data), GFP_NOWAIT);
}
+int io_futexv_wait(struct io_kiocb *req, unsigned int issue_flags)
+{
+ struct io_futex *iof = io_kiocb_to_cmd(req, struct io_futex);
+ struct futex_vector *futexv = req->async_data;
+ struct io_ring_ctx *ctx = req->ctx;
+ int ret, woken = -1;
+
+ io_ring_submit_lock(ctx, issue_flags);
+
+ ret = futex_wait_multiple_setup(futexv, iof->futex_nr, &woken);
+
+ /*
+ * Error case, ret is < 0. Mark the request as failed.
+ */
+ if (unlikely(ret < 0)) {
+ io_ring_submit_unlock(ctx, issue_flags);
+ req_set_fail(req);
+ io_req_set_res(req, ret, 0);
+ kfree(futexv);
+ req->async_data = NULL;
+ req->flags &= ~REQ_F_ASYNC_DATA;
+ return IOU_OK;
+ }
+
+ /*
+ * 0 return means that we successfully setup the waiters, and that
+ * nobody triggered a wakeup while we were doing so. If the wakeup
+ * happened post setup, the task_work will be run post this issue and
+ * under the submission lock. 1 means We got woken while setting up,
+ * let that side do the completion. Note that
+ * futex_wait_multiple_setup() will have unqueued all the futexes in
+ * this case. Mark us as having done that already, since this is
+ * different from normal wakeup.
+ */
+ if (!ret) {
+ /*
+ * If futex_wait_multiple_setup() returns 0 for a
+ * successful setup, then the task state will not be
+ * runnable. This is fine for the sync syscall, as
+ * it'll be blocking unless we already got one of the
+ * futexes woken, but it obviously won't work for an
+ * async invocation. Mark us runnable again.
+ */
+ __set_current_state(TASK_RUNNING);
+ hlist_add_head(&req->hash_node, &ctx->futex_list);
+ } else {
+ iof->futexv_unqueued = 1;
+ if (woken != -1)
+ io_req_set_res(req, woken, 0);
+ }
+
+ io_ring_submit_unlock(ctx, issue_flags);
+ return IOU_ISSUE_SKIP_COMPLETE;
+}
+
int io_futex_wait(struct io_kiocb *req, unsigned int issue_flags)
{
struct io_futex *iof = io_kiocb_to_cmd(req, struct io_futex);
diff --git a/io_uring/futex.h b/io_uring/futex.h
index ddc9e0d73c52..0847e9e8a127 100644
--- a/io_uring/futex.h
+++ b/io_uring/futex.h
@@ -3,7 +3,9 @@
#include "cancel.h"
int io_futex_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe);
+int io_futexv_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe);
int io_futex_wait(struct io_kiocb *req, unsigned int issue_flags);
+int io_futexv_wait(struct io_kiocb *req, unsigned int issue_flags);
int io_futex_wake(struct io_kiocb *req, unsigned int issue_flags);
#if defined(CONFIG_FUTEX)
diff --git a/io_uring/opdef.c b/io_uring/opdef.c
index 31a3a421e94d..25a3515a177c 100644
--- a/io_uring/opdef.c
+++ b/io_uring/opdef.c
@@ -459,6 +459,14 @@ const struct io_issue_def io_issue_defs[] = {
.issue = io_futex_wake,
#else
.prep = io_eopnotsupp_prep,
+#endif
+ },
+ [IORING_OP_FUTEX_WAITV] = {
+#if defined(CONFIG_FUTEX)
+ .prep = io_futexv_prep,
+ .issue = io_futexv_wait,
+#else
+ .prep = io_eopnotsupp_prep,
#endif
},
};
@@ -693,6 +701,9 @@ const struct io_cold_def io_cold_defs[] = {
[IORING_OP_FUTEX_WAKE] = {
.name = "FUTEX_WAKE",
},
+ [IORING_OP_FUTEX_WAITV] = {
+ .name = "FUTEX_WAITV",
+ },
};
const char *io_uring_get_opcode(u8 opcode)
--
Jens Axboe
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 4/8] io_uring: add support for futex wake and wait
2023-09-28 17:25 [PATCHSET v6] Add io_uring futex/futexv support Jens Axboe
@ 2023-09-28 17:25 ` Jens Axboe
0 siblings, 0 replies; 21+ messages in thread
From: Jens Axboe @ 2023-09-28 17:25 UTC (permalink / raw)
To: io-uring, linux-kernel; +Cc: peterz, andres, tglx, Jens Axboe
Add support for FUTEX_WAKE/WAIT primitives.
IORING_OP_FUTEX_WAKE is mix of FUTEX_WAKE and FUTEX_WAKE_BITSET, as
it does support passing in a bitset.
Similary, IORING_OP_FUTEX_WAIT is a mix of FUTEX_WAIT and
FUTEX_WAIT_BITSET.
For both of them, they are using the futex2 interface.
FUTEX_WAKE is straight forward, as those can always be done directly from
the io_uring submission without needing async handling. For FUTEX_WAIT,
things are a bit more complicated. If the futex isn't ready, then we
rely on a callback via futex_queue->wake() when someone wakes up the
futex. From that calback, we queue up task_work with the original task,
which will post a CQE and wake it, if necessary.
Cancelations are supported, both from the application point-of-view,
but also to be able to cancel pending waits if the ring exits before
all events have occurred. The return value of futex_unqueue() is used
to gate who wins the potential race between cancelation and futex
wakeups. Whomever gets a 'ret == 1' return from that claims ownership
of the io_uring futex request.
This is just the barebones wait/wake support. PI or REQUEUE support is
not added at this point, unclear if we might look into that later.
Likewise, explicit timeouts are not supported either. It is expected
that users that need timeouts would do so via the usual io_uring
mechanism to do that using linked timeouts.
The SQE format is as follows:
`addr` Address of futex
`fd` futex2(2) FUTEX2_* flags
`futex_flags` io_uring specific command flags. None valid now.
`addr2` Value of futex
`addr3` Mask to wake/wait
Signed-off-by: Jens Axboe <[email protected]>
---
include/linux/io_uring_types.h | 5 +
include/uapi/linux/io_uring.h | 3 +
io_uring/Makefile | 1 +
io_uring/cancel.c | 5 +
io_uring/cancel.h | 4 +
io_uring/futex.c | 235 +++++++++++++++++++++++++++++++++
io_uring/futex.h | 34 +++++
io_uring/io_uring.c | 7 +
io_uring/opdef.c | 23 ++++
9 files changed, 317 insertions(+)
create mode 100644 io_uring/futex.c
create mode 100644 io_uring/futex.h
diff --git a/include/linux/io_uring_types.h b/include/linux/io_uring_types.h
index e178461fa513..990984614fca 100644
--- a/include/linux/io_uring_types.h
+++ b/include/linux/io_uring_types.h
@@ -321,6 +321,11 @@ struct io_ring_ctx {
struct hlist_head waitid_list;
+#ifdef CONFIG_FUTEX
+ struct hlist_head futex_list;
+ struct io_alloc_cache futex_cache;
+#endif
+
const struct cred *sq_creds; /* cred used for __io_sq_thread() */
struct io_sq_data *sq_data; /* if using sq thread polling */
diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
index 425f64eee44e..04f9fba38d4b 100644
--- a/include/uapi/linux/io_uring.h
+++ b/include/uapi/linux/io_uring.h
@@ -66,6 +66,7 @@ struct io_uring_sqe {
__u32 msg_ring_flags;
__u32 uring_cmd_flags;
__u32 waitid_flags;
+ __u32 futex_flags;
};
__u64 user_data; /* data to be passed back at completion time */
/* pack this to avoid bogus arm OABI complaints */
@@ -243,6 +244,8 @@ enum io_uring_op {
IORING_OP_SENDMSG_ZC,
IORING_OP_READ_MULTISHOT,
IORING_OP_WAITID,
+ IORING_OP_FUTEX_WAIT,
+ IORING_OP_FUTEX_WAKE,
/* this goes last, obviously */
IORING_OP_LAST,
diff --git a/io_uring/Makefile b/io_uring/Makefile
index 7bd64e442567..e5be47e4fc3b 100644
--- a/io_uring/Makefile
+++ b/io_uring/Makefile
@@ -10,3 +10,4 @@ obj-$(CONFIG_IO_URING) += io_uring.o xattr.o nop.o fs.o splice.o \
cancel.o kbuf.o rsrc.o rw.o opdef.o \
notif.o waitid.o
obj-$(CONFIG_IO_WQ) += io-wq.o
+obj-$(CONFIG_FUTEX) += futex.o
diff --git a/io_uring/cancel.c b/io_uring/cancel.c
index eb77a51c5a79..3c19cccb1aec 100644
--- a/io_uring/cancel.c
+++ b/io_uring/cancel.c
@@ -16,6 +16,7 @@
#include "poll.h"
#include "timeout.h"
#include "waitid.h"
+#include "futex.h"
#include "cancel.h"
struct io_cancel {
@@ -124,6 +125,10 @@ int io_try_cancel(struct io_uring_task *tctx, struct io_cancel_data *cd,
if (ret != -ENOENT)
return ret;
+ ret = io_futex_cancel(ctx, cd, issue_flags);
+ if (ret != -ENOENT)
+ return ret;
+
spin_lock(&ctx->completion_lock);
if (!(cd->flags & IORING_ASYNC_CANCEL_FD))
ret = io_timeout_cancel(ctx, cd);
diff --git a/io_uring/cancel.h b/io_uring/cancel.h
index fc98622e6166..c0a8e7c520b6 100644
--- a/io_uring/cancel.h
+++ b/io_uring/cancel.h
@@ -1,4 +1,6 @@
// SPDX-License-Identifier: GPL-2.0
+#ifndef IORING_CANCEL_H
+#define IORING_CANCEL_H
#include <linux/io_uring_types.h>
@@ -22,3 +24,5 @@ void init_hash_table(struct io_hash_table *table, unsigned size);
int io_sync_cancel(struct io_ring_ctx *ctx, void __user *arg);
bool io_cancel_req_match(struct io_kiocb *req, struct io_cancel_data *cd);
+
+#endif
diff --git a/io_uring/futex.c b/io_uring/futex.c
new file mode 100644
index 000000000000..eb4406ac46fb
--- /dev/null
+++ b/io_uring/futex.c
@@ -0,0 +1,235 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/fs.h>
+#include <linux/file.h>
+#include <linux/io_uring.h>
+
+#include <uapi/linux/io_uring.h>
+
+#include "../kernel/futex/futex.h"
+#include "io_uring.h"
+#include "rsrc.h"
+#include "futex.h"
+
+struct io_futex {
+ struct file *file;
+ u32 __user *uaddr;
+ unsigned long futex_val;
+ unsigned long futex_mask;
+ u32 futex_flags;
+};
+
+struct io_futex_data {
+ union {
+ struct futex_q q;
+ struct io_cache_entry cache;
+ };
+ struct io_kiocb *req;
+};
+
+void io_futex_cache_init(struct io_ring_ctx *ctx)
+{
+ io_alloc_cache_init(&ctx->futex_cache, IO_NODE_ALLOC_CACHE_MAX,
+ sizeof(struct io_futex_data));
+}
+
+static void io_futex_cache_entry_free(struct io_cache_entry *entry)
+{
+ kfree(container_of(entry, struct io_futex_data, cache));
+}
+
+void io_futex_cache_free(struct io_ring_ctx *ctx)
+{
+ io_alloc_cache_free(&ctx->futex_cache, io_futex_cache_entry_free);
+}
+
+static void io_futex_complete(struct io_kiocb *req, struct io_tw_state *ts)
+{
+ struct io_futex_data *ifd = req->async_data;
+ struct io_ring_ctx *ctx = req->ctx;
+
+ io_tw_lock(ctx, ts);
+ if (!io_alloc_cache_put(&ctx->futex_cache, &ifd->cache))
+ kfree(ifd);
+ req->async_data = NULL;
+ hlist_del_init(&req->hash_node);
+ io_req_task_complete(req, ts);
+}
+
+static bool __io_futex_cancel(struct io_ring_ctx *ctx, struct io_kiocb *req)
+{
+ struct io_futex_data *ifd = req->async_data;
+
+ /* futex wake already done or in progress */
+ if (!futex_unqueue(&ifd->q))
+ return false;
+
+ hlist_del_init(&req->hash_node);
+ io_req_set_res(req, -ECANCELED, 0);
+ req->io_task_work.func = io_futex_complete;
+ io_req_task_work_add(req);
+ return true;
+}
+
+int io_futex_cancel(struct io_ring_ctx *ctx, struct io_cancel_data *cd,
+ unsigned int issue_flags)
+{
+ struct hlist_node *tmp;
+ struct io_kiocb *req;
+ int nr = 0;
+
+ if (cd->flags & (IORING_ASYNC_CANCEL_FD|IORING_ASYNC_CANCEL_FD_FIXED))
+ return -ENOENT;
+
+ io_ring_submit_lock(ctx, issue_flags);
+ hlist_for_each_entry_safe(req, tmp, &ctx->futex_list, hash_node) {
+ if (req->cqe.user_data != cd->data &&
+ !(cd->flags & IORING_ASYNC_CANCEL_ANY))
+ continue;
+ if (__io_futex_cancel(ctx, req))
+ nr++;
+ if (!(cd->flags & IORING_ASYNC_CANCEL_ALL))
+ break;
+ }
+ io_ring_submit_unlock(ctx, issue_flags);
+
+ if (nr)
+ return nr;
+
+ return -ENOENT;
+}
+
+bool io_futex_remove_all(struct io_ring_ctx *ctx, struct task_struct *task,
+ bool cancel_all)
+{
+ struct hlist_node *tmp;
+ struct io_kiocb *req;
+ bool found = false;
+
+ lockdep_assert_held(&ctx->uring_lock);
+
+ hlist_for_each_entry_safe(req, tmp, &ctx->futex_list, hash_node) {
+ if (!io_match_task_safe(req, task, cancel_all))
+ continue;
+ __io_futex_cancel(ctx, req);
+ found = true;
+ }
+
+ return found;
+}
+
+int io_futex_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
+{
+ struct io_futex *iof = io_kiocb_to_cmd(req, struct io_futex);
+ u32 flags;
+
+ if (unlikely(sqe->len || sqe->futex_flags || sqe->buf_index ||
+ sqe->file_index))
+ return -EINVAL;
+
+ iof->uaddr = u64_to_user_ptr(READ_ONCE(sqe->addr));
+ iof->futex_val = READ_ONCE(sqe->addr2);
+ iof->futex_mask = READ_ONCE(sqe->addr3);
+ flags = READ_ONCE(sqe->fd);
+
+ if (flags & ~FUTEX2_VALID_MASK)
+ return -EINVAL;
+
+ iof->futex_flags = futex2_to_flags(flags);
+ if (!futex_flags_valid(iof->futex_flags))
+ return -EINVAL;
+
+ if (!futex_validate_input(iof->futex_flags, iof->futex_val) ||
+ !futex_validate_input(iof->futex_flags, iof->futex_mask))
+ return -EINVAL;
+
+ return 0;
+}
+
+static void io_futex_wake_fn(struct wake_q_head *wake_q, struct futex_q *q)
+{
+ struct io_futex_data *ifd = container_of(q, struct io_futex_data, q);
+ struct io_kiocb *req = ifd->req;
+
+ if (unlikely(!__futex_wake_mark(q)))
+ return;
+
+ io_req_set_res(req, 0, 0);
+ req->io_task_work.func = io_futex_complete;
+ io_req_task_work_add(req);
+}
+
+static struct io_futex_data *io_alloc_ifd(struct io_ring_ctx *ctx)
+{
+ struct io_cache_entry *entry;
+
+ entry = io_alloc_cache_get(&ctx->futex_cache);
+ if (entry)
+ return container_of(entry, struct io_futex_data, cache);
+
+ return kmalloc(sizeof(struct io_futex_data), GFP_NOWAIT);
+}
+
+int io_futex_wait(struct io_kiocb *req, unsigned int issue_flags)
+{
+ struct io_futex *iof = io_kiocb_to_cmd(req, struct io_futex);
+ struct io_ring_ctx *ctx = req->ctx;
+ struct io_futex_data *ifd = NULL;
+ struct futex_hash_bucket *hb;
+ int ret;
+
+ if (!iof->futex_mask) {
+ ret = -EINVAL;
+ goto done;
+ }
+
+ io_ring_submit_lock(ctx, issue_flags);
+ ifd = io_alloc_ifd(ctx);
+ if (!ifd) {
+ ret = -ENOMEM;
+ goto done_unlock;
+ }
+
+ req->async_data = ifd;
+ ifd->q = futex_q_init;
+ ifd->q.bitset = iof->futex_mask;
+ ifd->q.wake = io_futex_wake_fn;
+ ifd->req = req;
+
+ ret = futex_wait_setup(iof->uaddr, iof->futex_val, iof->futex_flags,
+ &ifd->q, &hb);
+ if (!ret) {
+ hlist_add_head(&req->hash_node, &ctx->futex_list);
+ io_ring_submit_unlock(ctx, issue_flags);
+
+ futex_queue(&ifd->q, hb);
+ return IOU_ISSUE_SKIP_COMPLETE;
+ }
+
+done_unlock:
+ io_ring_submit_unlock(ctx, issue_flags);
+done:
+ if (ret < 0)
+ req_set_fail(req);
+ io_req_set_res(req, ret, 0);
+ kfree(ifd);
+ return IOU_OK;
+}
+
+int io_futex_wake(struct io_kiocb *req, unsigned int issue_flags)
+{
+ struct io_futex *iof = io_kiocb_to_cmd(req, struct io_futex);
+ int ret;
+
+ /*
+ * Strict flags - ensure that waking 0 futexes yields a 0 result.
+ * See commit 43adf8449510 ("futex: FLAGS_STRICT") for details.
+ */
+ ret = futex_wake(iof->uaddr, FLAGS_STRICT | iof->futex_flags,
+ iof->futex_val, iof->futex_mask);
+ if (ret < 0)
+ req_set_fail(req);
+ io_req_set_res(req, ret, 0);
+ return IOU_OK;
+}
diff --git a/io_uring/futex.h b/io_uring/futex.h
new file mode 100644
index 000000000000..ddc9e0d73c52
--- /dev/null
+++ b/io_uring/futex.h
@@ -0,0 +1,34 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include "cancel.h"
+
+int io_futex_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe);
+int io_futex_wait(struct io_kiocb *req, unsigned int issue_flags);
+int io_futex_wake(struct io_kiocb *req, unsigned int issue_flags);
+
+#if defined(CONFIG_FUTEX)
+int io_futex_cancel(struct io_ring_ctx *ctx, struct io_cancel_data *cd,
+ unsigned int issue_flags);
+bool io_futex_remove_all(struct io_ring_ctx *ctx, struct task_struct *task,
+ bool cancel_all);
+void io_futex_cache_init(struct io_ring_ctx *ctx);
+void io_futex_cache_free(struct io_ring_ctx *ctx);
+#else
+static inline int io_futex_cancel(struct io_ring_ctx *ctx,
+ struct io_cancel_data *cd,
+ unsigned int issue_flags)
+{
+ return 0;
+}
+static inline bool io_futex_remove_all(struct io_ring_ctx *ctx,
+ struct task_struct *task, bool cancel_all)
+{
+ return false;
+}
+static inline void io_futex_cache_init(struct io_ring_ctx *ctx)
+{
+}
+static inline void io_futex_cache_free(struct io_ring_ctx *ctx)
+{
+}
+#endif
diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index 08c9ea46bb95..3c1c111d02cb 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -93,6 +93,7 @@
#include "net.h"
#include "notif.h"
#include "waitid.h"
+#include "futex.h"
#include "timeout.h"
#include "poll.h"
@@ -330,6 +331,7 @@ static __cold struct io_ring_ctx *io_ring_ctx_alloc(struct io_uring_params *p)
sizeof(struct async_poll));
io_alloc_cache_init(&ctx->netmsg_cache, IO_ALLOC_CACHE_MAX,
sizeof(struct io_async_msghdr));
+ io_futex_cache_init(ctx);
init_completion(&ctx->ref_comp);
xa_init_flags(&ctx->personalities, XA_FLAGS_ALLOC1);
mutex_init(&ctx->uring_lock);
@@ -350,6 +352,9 @@ static __cold struct io_ring_ctx *io_ring_ctx_alloc(struct io_uring_params *p)
ctx->submit_state.free_list.next = NULL;
INIT_WQ_LIST(&ctx->locked_free_list);
INIT_HLIST_HEAD(&ctx->waitid_list);
+#ifdef CONFIG_FUTEX
+ INIT_HLIST_HEAD(&ctx->futex_list);
+#endif
INIT_DELAYED_WORK(&ctx->fallback_work, io_fallback_req_func);
INIT_WQ_LIST(&ctx->submit_state.compl_reqs);
INIT_HLIST_HEAD(&ctx->cancelable_uring_cmd);
@@ -2895,6 +2900,7 @@ static __cold void io_ring_ctx_free(struct io_ring_ctx *ctx)
io_eventfd_unregister(ctx);
io_alloc_cache_free(&ctx->apoll_cache, io_apoll_cache_free);
io_alloc_cache_free(&ctx->netmsg_cache, io_netmsg_cache_free);
+ io_futex_cache_free(ctx);
io_destroy_buffers(ctx);
mutex_unlock(&ctx->uring_lock);
if (ctx->sq_creds)
@@ -3338,6 +3344,7 @@ static __cold bool io_uring_try_cancel_requests(struct io_ring_ctx *ctx,
mutex_lock(&ctx->uring_lock);
ret |= io_poll_remove_all(ctx, task, cancel_all);
ret |= io_waitid_remove_all(ctx, task, cancel_all);
+ ret |= io_futex_remove_all(ctx, task, cancel_all);
ret |= io_uring_try_cancel_uring_cmd(ctx, task, cancel_all);
mutex_unlock(&ctx->uring_lock);
ret |= io_kill_timeouts(ctx, task, cancel_all);
diff --git a/io_uring/opdef.c b/io_uring/opdef.c
index aadcbf7136b0..31a3a421e94d 100644
--- a/io_uring/opdef.c
+++ b/io_uring/opdef.c
@@ -34,6 +34,7 @@
#include "cancel.h"
#include "rw.h"
#include "waitid.h"
+#include "futex.h"
static int io_no_issue(struct io_kiocb *req, unsigned int issue_flags)
{
@@ -444,6 +445,22 @@ const struct io_issue_def io_issue_defs[] = {
.prep = io_waitid_prep,
.issue = io_waitid,
},
+ [IORING_OP_FUTEX_WAIT] = {
+#if defined(CONFIG_FUTEX)
+ .prep = io_futex_prep,
+ .issue = io_futex_wait,
+#else
+ .prep = io_eopnotsupp_prep,
+#endif
+ },
+ [IORING_OP_FUTEX_WAKE] = {
+#if defined(CONFIG_FUTEX)
+ .prep = io_futex_prep,
+ .issue = io_futex_wake,
+#else
+ .prep = io_eopnotsupp_prep,
+#endif
+ },
};
const struct io_cold_def io_cold_defs[] = {
@@ -670,6 +687,12 @@ const struct io_cold_def io_cold_defs[] = {
.name = "WAITID",
.async_size = sizeof(struct io_waitid_async),
},
+ [IORING_OP_FUTEX_WAIT] = {
+ .name = "FUTEX_WAIT",
+ },
+ [IORING_OP_FUTEX_WAKE] = {
+ .name = "FUTEX_WAKE",
+ },
};
const char *io_uring_get_opcode(u8 opcode)
--
2.40.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 4/8] io_uring: add support for futex wake and wait
2023-07-12 16:20 [PATCHSET v2 0/8] Add io_uring futex/futexv support Jens Axboe
@ 2023-07-12 16:20 ` Jens Axboe
2023-07-13 11:15 ` Peter Zijlstra
0 siblings, 1 reply; 21+ messages in thread
From: Jens Axboe @ 2023-07-12 16:20 UTC (permalink / raw)
To: io-uring, linux-kernel; +Cc: tglx, mingo, peterz, andres, Jens Axboe
Add support for FUTEX_WAKE/WAIT primitives.
IORING_OP_FUTEX_WAKE is mix of FUTEX_WAKE and FUTEX_WAKE_BITSET, as
it does support passing in a bitset.
Similary, IORING_OP_FUTEX_WAIT is a mix of FUTEX_WAIT and
FUTEX_WAIT_BITSET.
FUTEX_WAKE is straight forward, as those can always be done directly from
the io_uring submission without needing async handling. For FUTEX_WAIT,
things are a bit more complicated. If the futex isn't ready, then we
rely on a callback via futex_queue->wake() when someone wakes up the
futex. From that calback, we queue up task_work with the original task,
which will post a CQE and wake it, if necessary.
Cancelations are supported, both from the application point-of-view,
but also to be able to cancel pending waits if the ring exits before
all events have occurred.
This is just the barebones wait/wake support. PI or REQUEUE support is
not added at this point, unclear if we might look into that later.
Likewise, explicit timeouts are not supported either. It is expected
that users that need timeouts would do so via the usual io_uring
mechanism to do that using linked timeouts.
Signed-off-by: Jens Axboe <[email protected]>
---
include/linux/io_uring_types.h | 3 +
include/uapi/linux/io_uring.h | 3 +
io_uring/Makefile | 4 +-
io_uring/cancel.c | 5 +
io_uring/cancel.h | 4 +
io_uring/futex.c | 232 +++++++++++++++++++++++++++++++++
io_uring/futex.h | 34 +++++
io_uring/io_uring.c | 5 +
io_uring/opdef.c | 24 +++-
9 files changed, 312 insertions(+), 2 deletions(-)
create mode 100644 io_uring/futex.c
create mode 100644 io_uring/futex.h
diff --git a/include/linux/io_uring_types.h b/include/linux/io_uring_types.h
index f04ce513fadb..a7f03d8d879f 100644
--- a/include/linux/io_uring_types.h
+++ b/include/linux/io_uring_types.h
@@ -273,6 +273,9 @@ struct io_ring_ctx {
struct io_wq_work_list locked_free_list;
unsigned int locked_free_nr;
+ struct hlist_head futex_list;
+ struct io_alloc_cache futex_cache;
+
const struct cred *sq_creds; /* cred used for __io_sq_thread() */
struct io_sq_data *sq_data; /* if using sq thread polling */
diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
index 36f9c73082de..3bd2d765f593 100644
--- a/include/uapi/linux/io_uring.h
+++ b/include/uapi/linux/io_uring.h
@@ -65,6 +65,7 @@ struct io_uring_sqe {
__u32 xattr_flags;
__u32 msg_ring_flags;
__u32 uring_cmd_flags;
+ __u32 futex_flags;
};
__u64 user_data; /* data to be passed back at completion time */
/* pack this to avoid bogus arm OABI complaints */
@@ -235,6 +236,8 @@ enum io_uring_op {
IORING_OP_URING_CMD,
IORING_OP_SEND_ZC,
IORING_OP_SENDMSG_ZC,
+ IORING_OP_FUTEX_WAIT,
+ IORING_OP_FUTEX_WAKE,
/* this goes last, obviously */
IORING_OP_LAST,
diff --git a/io_uring/Makefile b/io_uring/Makefile
index 8cc8e5387a75..2e4779bc550c 100644
--- a/io_uring/Makefile
+++ b/io_uring/Makefile
@@ -7,5 +7,7 @@ obj-$(CONFIG_IO_URING) += io_uring.o xattr.o nop.o fs.o splice.o \
openclose.o uring_cmd.o epoll.o \
statx.o net.o msg_ring.o timeout.o \
sqpoll.o fdinfo.o tctx.o poll.o \
- cancel.o kbuf.o rsrc.o rw.o opdef.o notif.o
+ cancel.o kbuf.o rsrc.o rw.o opdef.o \
+ notif.o
obj-$(CONFIG_IO_WQ) += io-wq.o
+obj-$(CONFIG_FUTEX) += futex.o
diff --git a/io_uring/cancel.c b/io_uring/cancel.c
index 7b23607cf4af..3dba8ccb1cd8 100644
--- a/io_uring/cancel.c
+++ b/io_uring/cancel.c
@@ -15,6 +15,7 @@
#include "tctx.h"
#include "poll.h"
#include "timeout.h"
+#include "futex.h"
#include "cancel.h"
struct io_cancel {
@@ -119,6 +120,10 @@ int io_try_cancel(struct io_uring_task *tctx, struct io_cancel_data *cd,
if (ret != -ENOENT)
return ret;
+ ret = io_futex_cancel(ctx, cd, issue_flags);
+ if (ret != -ENOENT)
+ return ret;
+
spin_lock(&ctx->completion_lock);
if (!(cd->flags & IORING_ASYNC_CANCEL_FD))
ret = io_timeout_cancel(ctx, cd);
diff --git a/io_uring/cancel.h b/io_uring/cancel.h
index fc98622e6166..c0a8e7c520b6 100644
--- a/io_uring/cancel.h
+++ b/io_uring/cancel.h
@@ -1,4 +1,6 @@
// SPDX-License-Identifier: GPL-2.0
+#ifndef IORING_CANCEL_H
+#define IORING_CANCEL_H
#include <linux/io_uring_types.h>
@@ -22,3 +24,5 @@ void init_hash_table(struct io_hash_table *table, unsigned size);
int io_sync_cancel(struct io_ring_ctx *ctx, void __user *arg);
bool io_cancel_req_match(struct io_kiocb *req, struct io_cancel_data *cd);
+
+#endif
diff --git a/io_uring/futex.c b/io_uring/futex.c
new file mode 100644
index 000000000000..fa92ee795c83
--- /dev/null
+++ b/io_uring/futex.c
@@ -0,0 +1,232 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/fs.h>
+#include <linux/file.h>
+#include <linux/io_uring.h>
+
+#include <uapi/linux/io_uring.h>
+
+#include "../kernel/futex/futex.h"
+#include "io_uring.h"
+#include "rsrc.h"
+#include "futex.h"
+
+struct io_futex {
+ struct file *file;
+ u32 __user *uaddr;
+ int futex_op;
+ unsigned int futex_val;
+ unsigned int futex_flags;
+ unsigned int futex_mask;
+};
+
+struct io_futex_data {
+ union {
+ struct futex_q q;
+ struct io_cache_entry cache;
+ };
+ struct io_kiocb *req;
+};
+
+void io_futex_cache_init(struct io_ring_ctx *ctx)
+{
+ io_alloc_cache_init(&ctx->futex_cache, IO_NODE_ALLOC_CACHE_MAX,
+ sizeof(struct io_futex_data));
+}
+
+static void io_futex_cache_entry_free(struct io_cache_entry *entry)
+{
+ kfree(container_of(entry, struct io_futex_data, cache));
+}
+
+void io_futex_cache_free(struct io_ring_ctx *ctx)
+{
+ io_alloc_cache_free(&ctx->futex_cache, io_futex_cache_entry_free);
+}
+
+static void io_futex_complete(struct io_kiocb *req, struct io_tw_state *ts)
+{
+ struct io_futex_data *ifd = req->async_data;
+ struct io_ring_ctx *ctx = req->ctx;
+
+ io_tw_lock(ctx, ts);
+ if (!io_alloc_cache_put(&ctx->futex_cache, &ifd->cache))
+ kfree(ifd);
+ req->async_data = NULL;
+ hlist_del_init(&req->hash_node);
+ io_req_task_complete(req, ts);
+}
+
+static bool __io_futex_cancel(struct io_ring_ctx *ctx, struct io_kiocb *req)
+{
+ struct io_futex_data *ifd = req->async_data;
+
+ /* futex wake already done or in progress */
+ if (!futex_unqueue(&ifd->q))
+ return false;
+
+ hlist_del_init(&req->hash_node);
+ io_req_set_res(req, -ECANCELED, 0);
+ req->io_task_work.func = io_futex_complete;
+ io_req_task_work_add(req);
+ return true;
+}
+
+int io_futex_cancel(struct io_ring_ctx *ctx, struct io_cancel_data *cd,
+ unsigned int issue_flags)
+{
+ struct hlist_node *tmp;
+ struct io_kiocb *req;
+ int nr = 0;
+
+ if (cd->flags & (IORING_ASYNC_CANCEL_FD|IORING_ASYNC_CANCEL_FD_FIXED))
+ return -ENOENT;
+
+ io_ring_submit_lock(ctx, issue_flags);
+ hlist_for_each_entry_safe(req, tmp, &ctx->futex_list, hash_node) {
+ if (req->cqe.user_data != cd->data &&
+ !(cd->flags & IORING_ASYNC_CANCEL_ANY))
+ continue;
+ if (__io_futex_cancel(ctx, req))
+ nr++;
+ if (!(cd->flags & IORING_ASYNC_CANCEL_ALL))
+ break;
+ }
+ io_ring_submit_unlock(ctx, issue_flags);
+
+ if (nr)
+ return nr;
+
+ return -ENOENT;
+}
+
+bool io_futex_remove_all(struct io_ring_ctx *ctx, struct task_struct *task,
+ bool cancel_all)
+{
+ struct hlist_node *tmp;
+ struct io_kiocb *req;
+ bool found = false;
+
+ lockdep_assert_held(&ctx->uring_lock);
+
+ hlist_for_each_entry_safe(req, tmp, &ctx->futex_list, hash_node) {
+ if (!io_match_task_safe(req, task, cancel_all))
+ continue;
+ __io_futex_cancel(ctx, req);
+ found = true;
+ }
+
+ return found;
+}
+
+int io_futex_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
+{
+ struct io_futex *iof = io_kiocb_to_cmd(req, struct io_futex);
+
+ if (unlikely(sqe->addr2 || sqe->buf_index || sqe->addr3))
+ return -EINVAL;
+
+ iof->futex_op = READ_ONCE(sqe->fd);
+ iof->uaddr = u64_to_user_ptr(READ_ONCE(sqe->addr));
+ iof->futex_val = READ_ONCE(sqe->len);
+ iof->futex_mask = READ_ONCE(sqe->file_index);
+ iof->futex_flags = READ_ONCE(sqe->futex_flags);
+ if (iof->futex_flags & FUTEX_CMD_MASK)
+ return -EINVAL;
+
+ return 0;
+}
+
+static void io_futex_wake_fn(struct wake_q_head *wake_q, struct futex_q *q)
+{
+ struct io_futex_data *ifd = container_of(q, struct io_futex_data, q);
+ struct io_kiocb *req = ifd->req;
+
+ if (unlikely(!__futex_wake_mark(q)))
+ return;
+
+ io_req_set_res(req, 0, 0);
+ req->io_task_work.func = io_futex_complete;
+ io_req_task_work_add(req);
+}
+
+static struct io_futex_data *io_alloc_ifd(struct io_ring_ctx *ctx)
+{
+ struct io_cache_entry *entry;
+
+ entry = io_alloc_cache_get(&ctx->futex_cache);
+ if (entry)
+ return container_of(entry, struct io_futex_data, cache);
+
+ return kmalloc(sizeof(struct io_futex_data), GFP_NOWAIT);
+}
+
+int io_futex_wait(struct io_kiocb *req, unsigned int issue_flags)
+{
+ struct io_futex *iof = io_kiocb_to_cmd(req, struct io_futex);
+ struct io_ring_ctx *ctx = req->ctx;
+ struct io_futex_data *ifd = NULL;
+ struct futex_hash_bucket *hb;
+ unsigned int flags;
+ int ret;
+
+ if (!iof->futex_mask) {
+ ret = -EINVAL;
+ goto done;
+ }
+ if (!futex_op_to_flags(FUTEX_WAIT, iof->futex_flags, &flags)) {
+ ret = -ENOSYS;
+ goto done;
+ }
+
+ io_ring_submit_lock(ctx, issue_flags);
+ ifd = io_alloc_ifd(ctx);
+ if (!ifd) {
+ ret = -ENOMEM;
+ goto done_unlock;
+ }
+
+ req->async_data = ifd;
+ ifd->q = futex_q_init;
+ ifd->q.bitset = iof->futex_mask;
+ ifd->q.wake = io_futex_wake_fn;
+ ifd->req = req;
+
+ ret = futex_wait_setup(iof->uaddr, iof->futex_val, flags, &ifd->q, &hb);
+ if (!ret) {
+ hlist_add_head(&req->hash_node, &ctx->futex_list);
+ io_ring_submit_unlock(ctx, issue_flags);
+
+ futex_queue(&ifd->q, hb);
+ return IOU_ISSUE_SKIP_COMPLETE;
+ }
+
+done_unlock:
+ io_ring_submit_unlock(ctx, issue_flags);
+done:
+ if (ret < 0)
+ req_set_fail(req);
+ io_req_set_res(req, ret, 0);
+ kfree(ifd);
+ return IOU_OK;
+}
+
+int io_futex_wake(struct io_kiocb *req, unsigned int issue_flags)
+{
+ struct io_futex *iof = io_kiocb_to_cmd(req, struct io_futex);
+ unsigned int flags = 0;
+ int ret;
+
+ if (!futex_op_to_flags(FUTEX_WAKE, iof->futex_flags, &flags)) {
+ ret = -ENOSYS;
+ goto done;
+ }
+
+ ret = futex_wake(iof->uaddr, flags, iof->futex_val, iof->futex_mask);
+done:
+ if (ret < 0)
+ req_set_fail(req);
+ io_req_set_res(req, ret, 0);
+ return IOU_OK;
+}
diff --git a/io_uring/futex.h b/io_uring/futex.h
new file mode 100644
index 000000000000..ddc9e0d73c52
--- /dev/null
+++ b/io_uring/futex.h
@@ -0,0 +1,34 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include "cancel.h"
+
+int io_futex_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe);
+int io_futex_wait(struct io_kiocb *req, unsigned int issue_flags);
+int io_futex_wake(struct io_kiocb *req, unsigned int issue_flags);
+
+#if defined(CONFIG_FUTEX)
+int io_futex_cancel(struct io_ring_ctx *ctx, struct io_cancel_data *cd,
+ unsigned int issue_flags);
+bool io_futex_remove_all(struct io_ring_ctx *ctx, struct task_struct *task,
+ bool cancel_all);
+void io_futex_cache_init(struct io_ring_ctx *ctx);
+void io_futex_cache_free(struct io_ring_ctx *ctx);
+#else
+static inline int io_futex_cancel(struct io_ring_ctx *ctx,
+ struct io_cancel_data *cd,
+ unsigned int issue_flags)
+{
+ return 0;
+}
+static inline bool io_futex_remove_all(struct io_ring_ctx *ctx,
+ struct task_struct *task, bool cancel_all)
+{
+ return false;
+}
+static inline void io_futex_cache_init(struct io_ring_ctx *ctx)
+{
+}
+static inline void io_futex_cache_free(struct io_ring_ctx *ctx)
+{
+}
+#endif
diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index e8096d502a7c..67ff148bc394 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -92,6 +92,7 @@
#include "cancel.h"
#include "net.h"
#include "notif.h"
+#include "futex.h"
#include "timeout.h"
#include "poll.h"
@@ -314,6 +315,7 @@ static __cold struct io_ring_ctx *io_ring_ctx_alloc(struct io_uring_params *p)
sizeof(struct async_poll));
io_alloc_cache_init(&ctx->netmsg_cache, IO_ALLOC_CACHE_MAX,
sizeof(struct io_async_msghdr));
+ io_futex_cache_init(ctx);
init_completion(&ctx->ref_comp);
xa_init_flags(&ctx->personalities, XA_FLAGS_ALLOC1);
mutex_init(&ctx->uring_lock);
@@ -333,6 +335,7 @@ static __cold struct io_ring_ctx *io_ring_ctx_alloc(struct io_uring_params *p)
INIT_LIST_HEAD(&ctx->tctx_list);
ctx->submit_state.free_list.next = NULL;
INIT_WQ_LIST(&ctx->locked_free_list);
+ INIT_HLIST_HEAD(&ctx->futex_list);
INIT_DELAYED_WORK(&ctx->fallback_work, io_fallback_req_func);
INIT_WQ_LIST(&ctx->submit_state.compl_reqs);
return ctx;
@@ -2842,6 +2845,7 @@ static __cold void io_ring_ctx_free(struct io_ring_ctx *ctx)
io_eventfd_unregister(ctx);
io_alloc_cache_free(&ctx->apoll_cache, io_apoll_cache_free);
io_alloc_cache_free(&ctx->netmsg_cache, io_netmsg_cache_free);
+ io_futex_cache_free(ctx);
io_destroy_buffers(ctx);
mutex_unlock(&ctx->uring_lock);
if (ctx->sq_creds)
@@ -3254,6 +3258,7 @@ static __cold bool io_uring_try_cancel_requests(struct io_ring_ctx *ctx,
ret |= io_cancel_defer_files(ctx, task, cancel_all);
mutex_lock(&ctx->uring_lock);
ret |= io_poll_remove_all(ctx, task, cancel_all);
+ ret |= io_futex_remove_all(ctx, task, cancel_all);
mutex_unlock(&ctx->uring_lock);
ret |= io_kill_timeouts(ctx, task, cancel_all);
if (task)
diff --git a/io_uring/opdef.c b/io_uring/opdef.c
index 3b9c6489b8b6..c9f23c21a031 100644
--- a/io_uring/opdef.c
+++ b/io_uring/opdef.c
@@ -33,6 +33,7 @@
#include "poll.h"
#include "cancel.h"
#include "rw.h"
+#include "futex.h"
static int io_no_issue(struct io_kiocb *req, unsigned int issue_flags)
{
@@ -426,11 +427,26 @@ const struct io_issue_def io_issue_defs[] = {
.issue = io_sendmsg_zc,
#else
.prep = io_eopnotsupp_prep,
+#endif
+ },
+ [IORING_OP_FUTEX_WAIT] = {
+#if defined(CONFIG_FUTEX)
+ .prep = io_futex_prep,
+ .issue = io_futex_wait,
+#else
+ .prep = io_eopnotsupp_prep,
+#endif
+ },
+ [IORING_OP_FUTEX_WAKE] = {
+#if defined(CONFIG_FUTEX)
+ .prep = io_futex_prep,
+ .issue = io_futex_wake,
+#else
+ .prep = io_eopnotsupp_prep,
#endif
},
};
-
const struct io_cold_def io_cold_defs[] = {
[IORING_OP_NOP] = {
.name = "NOP",
@@ -648,6 +664,12 @@ const struct io_cold_def io_cold_defs[] = {
.fail = io_sendrecv_fail,
#endif
},
+ [IORING_OP_FUTEX_WAIT] = {
+ .name = "FUTEX_WAIT",
+ },
+ [IORING_OP_FUTEX_WAKE] = {
+ .name = "FUTEX_WAKE",
+ },
};
const char *io_uring_get_opcode(u8 opcode)
--
2.40.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 4/8] io_uring: add support for futex wake and wait
2023-07-12 16:20 ` [PATCH 4/8] io_uring: add support for futex wake and wait Jens Axboe
@ 2023-07-13 11:15 ` Peter Zijlstra
2023-07-13 16:25 ` Jens Axboe
2023-07-13 17:24 ` Peter Zijlstra
0 siblings, 2 replies; 21+ messages in thread
From: Peter Zijlstra @ 2023-07-13 11:15 UTC (permalink / raw)
To: Jens Axboe; +Cc: io-uring, linux-kernel, tglx, mingo, andres
On Wed, Jul 12, 2023 at 10:20:13AM -0600, Jens Axboe wrote:
> +int io_futex_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
> +{
> + struct io_futex *iof = io_kiocb_to_cmd(req, struct io_futex);
> +
> + if (unlikely(sqe->addr2 || sqe->buf_index || sqe->addr3))
> + return -EINVAL;
> +
> + iof->futex_op = READ_ONCE(sqe->fd);
> + iof->uaddr = u64_to_user_ptr(READ_ONCE(sqe->addr));
> + iof->futex_val = READ_ONCE(sqe->len);
> + iof->futex_mask = READ_ONCE(sqe->file_index);
> + iof->futex_flags = READ_ONCE(sqe->futex_flags);
> + if (iof->futex_flags & FUTEX_CMD_MASK)
> + return -EINVAL;
> +
> + return 0;
> +}
I'm a little confused on the purpose of iof->futex_op, it doesn't appear
to be used. Instead iof->futex_flags is used as the ~FUTEX_CMD_MASK part
of ops.
The latter actually makes sense since you encode the actual op in the
IOURING_OP_ space.
> +int io_futex_wait(struct io_kiocb *req, unsigned int issue_flags)
> +{
> + struct io_futex *iof = io_kiocb_to_cmd(req, struct io_futex);
> + struct io_ring_ctx *ctx = req->ctx;
> + struct io_futex_data *ifd = NULL;
> + struct futex_hash_bucket *hb;
> + unsigned int flags;
> + int ret;
> +
> + if (!iof->futex_mask) {
> + ret = -EINVAL;
> + goto done;
> + }
> + if (!futex_op_to_flags(FUTEX_WAIT, iof->futex_flags, &flags)) {
A little confusing since you then implement FUTEX_WAIT_BITSET, but using
FUTEX_WAIT ensures this goes -ENOSYS when setting FUTEX_CLOCK_REALTIME,
since you handle timeouts through the iouring thing.
Perhaps a comment?
> + ret = -ENOSYS;
> + goto done;
> + }
> +
> + io_ring_submit_lock(ctx, issue_flags);
> + ifd = io_alloc_ifd(ctx);
> + if (!ifd) {
> + ret = -ENOMEM;
> + goto done_unlock;
> + }
> +
> + req->async_data = ifd;
> + ifd->q = futex_q_init;
> + ifd->q.bitset = iof->futex_mask;
> + ifd->q.wake = io_futex_wake_fn;
> + ifd->req = req;
> +
> + ret = futex_wait_setup(iof->uaddr, iof->futex_val, flags, &ifd->q, &hb);
> + if (!ret) {
> + hlist_add_head(&req->hash_node, &ctx->futex_list);
> + io_ring_submit_unlock(ctx, issue_flags);
> +
> + futex_queue(&ifd->q, hb);
> + return IOU_ISSUE_SKIP_COMPLETE;
> + }
> +
> +done_unlock:
> + io_ring_submit_unlock(ctx, issue_flags);
> +done:
> + if (ret < 0)
> + req_set_fail(req);
> + io_req_set_res(req, ret, 0);
> + kfree(ifd);
> + return IOU_OK;
> +}
Other than that, I think these things are indeed transparant wrt the
existing futex interface. If we add a flag this shouldn't care.
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 4/8] io_uring: add support for futex wake and wait
2023-07-13 11:15 ` Peter Zijlstra
@ 2023-07-13 16:25 ` Jens Axboe
2023-07-13 17:24 ` Peter Zijlstra
1 sibling, 0 replies; 21+ messages in thread
From: Jens Axboe @ 2023-07-13 16:25 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: io-uring, linux-kernel, tglx, mingo, andres
On 7/13/23 5:15?AM, Peter Zijlstra wrote:
> On Wed, Jul 12, 2023 at 10:20:13AM -0600, Jens Axboe wrote:
>
>> +int io_futex_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
>> +{
>> + struct io_futex *iof = io_kiocb_to_cmd(req, struct io_futex);
>> +
>> + if (unlikely(sqe->addr2 || sqe->buf_index || sqe->addr3))
>> + return -EINVAL;
>> +
>> + iof->futex_op = READ_ONCE(sqe->fd);
>> + iof->uaddr = u64_to_user_ptr(READ_ONCE(sqe->addr));
>> + iof->futex_val = READ_ONCE(sqe->len);
>> + iof->futex_mask = READ_ONCE(sqe->file_index);
>> + iof->futex_flags = READ_ONCE(sqe->futex_flags);
>> + if (iof->futex_flags & FUTEX_CMD_MASK)
>> + return -EINVAL;
>> +
>> + return 0;
>> +}
>
> I'm a little confused on the purpose of iof->futex_op, it doesn't appear
> to be used. Instead iof->futex_flags is used as the ~FUTEX_CMD_MASK part
> of ops.
>
> The latter actually makes sense since you encode the actual op in the
> IOURING_OP_ space.
Yep, I think this is also a leftover from when I had it multiplexed a
bit more. The liburing side got fixed for that, but neglected this bit.
Good catch. I'll fold the below in.
>
>> +int io_futex_wait(struct io_kiocb *req, unsigned int issue_flags)
>> +{
>> + struct io_futex *iof = io_kiocb_to_cmd(req, struct io_futex);
>> + struct io_ring_ctx *ctx = req->ctx;
>> + struct io_futex_data *ifd = NULL;
>> + struct futex_hash_bucket *hb;
>> + unsigned int flags;
>> + int ret;
>> +
>> + if (!iof->futex_mask) {
>> + ret = -EINVAL;
>> + goto done;
>> + }
>> + if (!futex_op_to_flags(FUTEX_WAIT, iof->futex_flags, &flags)) {
>
> A little confusing since you then implement FUTEX_WAIT_BITSET, but using
> FUTEX_WAIT ensures this goes -ENOSYS when setting FUTEX_CLOCK_REALTIME,
> since you handle timeouts through the iouring thing.
>
> Perhaps a comment?
OK, will add a comment on that.
>> + ret = -ENOSYS;
>> + goto done;
>> + }
>> +
>> + io_ring_submit_lock(ctx, issue_flags);
>> + ifd = io_alloc_ifd(ctx);
>> + if (!ifd) {
>> + ret = -ENOMEM;
>> + goto done_unlock;
>> + }
>> +
>> + req->async_data = ifd;
>> + ifd->q = futex_q_init;
>> + ifd->q.bitset = iof->futex_mask;
>> + ifd->q.wake = io_futex_wake_fn;
>> + ifd->req = req;
>> +
>> + ret = futex_wait_setup(iof->uaddr, iof->futex_val, flags, &ifd->q, &hb);
>> + if (!ret) {
>> + hlist_add_head(&req->hash_node, &ctx->futex_list);
>> + io_ring_submit_unlock(ctx, issue_flags);
>> +
>> + futex_queue(&ifd->q, hb);
>> + return IOU_ISSUE_SKIP_COMPLETE;
>> + }
>> +
>> +done_unlock:
>> + io_ring_submit_unlock(ctx, issue_flags);
>> +done:
>> + if (ret < 0)
>> + req_set_fail(req);
>> + io_req_set_res(req, ret, 0);
>> + kfree(ifd);
>> + return IOU_OK;
>> +}
>
> Other than that, I think these things are indeed transparant wrt the
> existing futex interface. If we add a flag this shouldn't care.
Not sure I follow, what kind of flag do you want/need?
diff --git a/io_uring/futex.c b/io_uring/futex.c
index df65b8f3593f..bced11c87896 100644
--- a/io_uring/futex.c
+++ b/io_uring/futex.c
@@ -18,7 +18,6 @@ struct io_futex {
u32 __user *uaddr;
struct futex_waitv __user *uwaitv;
};
- int futex_op;
unsigned int futex_val;
unsigned int futex_flags;
unsigned int futex_mask;
@@ -173,10 +172,9 @@ int io_futex_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
{
struct io_futex *iof = io_kiocb_to_cmd(req, struct io_futex);
- if (unlikely(sqe->buf_index || sqe->addr3))
+ if (unlikely(sqe->fd || sqe->buf_index || sqe->addr3))
return -EINVAL;
- iof->futex_op = READ_ONCE(sqe->fd);
iof->uaddr = u64_to_user_ptr(READ_ONCE(sqe->addr));
iof->futex_val = READ_ONCE(sqe->len);
iof->futex_mask = READ_ONCE(sqe->file_index);
--
Jens Axboe
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 4/8] io_uring: add support for futex wake and wait
2023-07-13 11:15 ` Peter Zijlstra
2023-07-13 16:25 ` Jens Axboe
@ 2023-07-13 17:24 ` Peter Zijlstra
2023-07-14 14:52 ` Jens Axboe
1 sibling, 1 reply; 21+ messages in thread
From: Peter Zijlstra @ 2023-07-13 17:24 UTC (permalink / raw)
To: Jens Axboe; +Cc: io-uring, linux-kernel, tglx, mingo, andres
On Thu, Jul 13, 2023 at 01:15:13PM +0200, Peter Zijlstra wrote:
> On Wed, Jul 12, 2023 at 10:20:13AM -0600, Jens Axboe wrote:
>
> > +int io_futex_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
> > +{
> > + struct io_futex *iof = io_kiocb_to_cmd(req, struct io_futex);
> > +
> > + if (unlikely(sqe->addr2 || sqe->buf_index || sqe->addr3))
> > + return -EINVAL;
> > +
> > + iof->futex_op = READ_ONCE(sqe->fd);
> > + iof->uaddr = u64_to_user_ptr(READ_ONCE(sqe->addr));
> > + iof->futex_val = READ_ONCE(sqe->len);
> > + iof->futex_mask = READ_ONCE(sqe->file_index);
> > + iof->futex_flags = READ_ONCE(sqe->futex_flags);
> > + if (iof->futex_flags & FUTEX_CMD_MASK)
> > + return -EINVAL;
> > +
> > + return 0;
> > +}
>
> I'm a little confused on the purpose of iof->futex_op, it doesn't appear
> to be used. Instead iof->futex_flags is used as the ~FUTEX_CMD_MASK part
> of ops.
>
> The latter actually makes sense since you encode the actual op in the
> IOURING_OP_ space.
Futex is slowly getting back to me; I'm thinking these io_uring
interfaces should perhaps use the futex2 flags instead of the futex_op
encoded muck.
I'll try and send out a few patches tomorrow to clarify things a little
-- the futex2 work seems to have stalled somewhere halfway :/
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 4/8] io_uring: add support for futex wake and wait
2023-07-13 17:24 ` Peter Zijlstra
@ 2023-07-14 14:52 ` Jens Axboe
2023-07-14 15:08 ` Peter Zijlstra
0 siblings, 1 reply; 21+ messages in thread
From: Jens Axboe @ 2023-07-14 14:52 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: io-uring, linux-kernel, tglx, mingo, andres
On 7/13/23 11:24?AM, Peter Zijlstra wrote:
> On Thu, Jul 13, 2023 at 01:15:13PM +0200, Peter Zijlstra wrote:
>> On Wed, Jul 12, 2023 at 10:20:13AM -0600, Jens Axboe wrote:
>>
>>> +int io_futex_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
>>> +{
>>> + struct io_futex *iof = io_kiocb_to_cmd(req, struct io_futex);
>>> +
>>> + if (unlikely(sqe->addr2 || sqe->buf_index || sqe->addr3))
>>> + return -EINVAL;
>>> +
>>> + iof->futex_op = READ_ONCE(sqe->fd);
>>> + iof->uaddr = u64_to_user_ptr(READ_ONCE(sqe->addr));
>>> + iof->futex_val = READ_ONCE(sqe->len);
>>> + iof->futex_mask = READ_ONCE(sqe->file_index);
>>> + iof->futex_flags = READ_ONCE(sqe->futex_flags);
>>> + if (iof->futex_flags & FUTEX_CMD_MASK)
>>> + return -EINVAL;
>>> +
>>> + return 0;
>>> +}
>>
>> I'm a little confused on the purpose of iof->futex_op, it doesn't appear
>> to be used. Instead iof->futex_flags is used as the ~FUTEX_CMD_MASK part
>> of ops.
>>
>> The latter actually makes sense since you encode the actual op in the
>> IOURING_OP_ space.
>
> Futex is slowly getting back to me; I'm thinking these io_uring
> interfaces should perhaps use the futex2 flags instead of the futex_op
> encoded muck.
>
> I'll try and send out a few patches tomorrow to clarify things a little
> -- the futex2 work seems to have stalled somewhere halfway :/
Saw your series - I'll take a look. In terms of staging when we get
there, would it be possible to split your flags series into the bare
minimum and trivial, and then have that as a dependency for this series
and the rest of your series?
--
Jens Axboe
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 4/8] io_uring: add support for futex wake and wait
2023-07-14 14:52 ` Jens Axboe
@ 2023-07-14 15:08 ` Peter Zijlstra
2023-07-14 15:10 ` Jens Axboe
0 siblings, 1 reply; 21+ messages in thread
From: Peter Zijlstra @ 2023-07-14 15:08 UTC (permalink / raw)
To: Jens Axboe; +Cc: io-uring, linux-kernel, tglx, mingo, andres
On Fri, Jul 14, 2023 at 08:52:40AM -0600, Jens Axboe wrote:
> Saw your series - I'll take a look. In terms of staging when we get
> there, would it be possible to split your flags series into the bare
> minimum and trivial, and then have that as a dependency for this series
> and the rest of your series?
I think you only really need the first three patches, and I hope those
are the least controversial of the lot.
After those, I can implement the extra flags independently of the
io_uring thing and all interfaces should just have it work.
So yes :-)
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 4/8] io_uring: add support for futex wake and wait
2023-07-14 15:08 ` Peter Zijlstra
@ 2023-07-14 15:10 ` Jens Axboe
0 siblings, 0 replies; 21+ messages in thread
From: Jens Axboe @ 2023-07-14 15:10 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: io-uring, linux-kernel, tglx, mingo, andres
On 7/14/23 9:08?AM, Peter Zijlstra wrote:
> On Fri, Jul 14, 2023 at 08:52:40AM -0600, Jens Axboe wrote:
>> Saw your series - I'll take a look. In terms of staging when we get
>> there, would it be possible to split your flags series into the bare
>> minimum and trivial, and then have that as a dependency for this series
>> and the rest of your series?
>
> I think you only really need the first three patches, and I hope those
> are the least controversial of the lot.
That's what it looks like to me, and yes those look trivial and risk
free :-)
I'll just put those at the base of my series for now, and once they are
stamped, would be great if you could then stuff them in a stable branch
that I could pull in.
> After those, I can implement the extra flags independently of the
> io_uring thing and all interfaces should just have it work.
>
> So yes :-)
That's the plan then!
--
Jens Axboe
^ permalink raw reply [flat|nested] 21+ messages in thread