Hi,
This patchset adds support for first futex wake and wait, and then
futexv.
For both wait/wake/waitv, we support the bitset variant, as the
"normal" variants can be easily implemented on top of that.
PI and requeue are not supported through io_uring, just the above
mentioned parts. This may change in the future, but in the spirit
of keeping this small (and based on what people have been asking for),
this is what we currently have.
When I did these patches, I forgot that Pavel had previously posted a
futex variant for io_uring. The major thing that had been holding me
back from people asking about futexes and io_uring, is that I wanted
to do this what I consider the right way - no usage of io-wq or thread
offload, an actually async implementation that is efficient to use
and don't rely on a blocking thread for futex wait/waitv. This is what
this patchset attempts to do, while being minimally invasive on the
futex side. I believe the diffstat reflects that.
As far as I can recall, the first request for futex support with
io_uring came from Andres Freund, working on postgres. His aio rework
of postgres was one of the early adopters of io_uring, and futex
support was a natural extension for that. This is relevant from both
a usability point of view, as well as for effiency and performance.
In Andres's words, for the former:
"Futex wait support in io_uring makes it a lot easier to avoid deadlocks
in concurrent programs that have their own buffer pool: Obviously pages in
the application buffer pool have to be locked during IO. If the initiator
of IO A needs to wait for a held lock B, the holder of lock B might wait
for the IO A to complete. The ability to wait for a lock and IO
completions at the same time provides an efficient way to avoid such
deadlocks."
and in terms of effiency, even without unlocking the full potential yet,
Andres says:
"Futex wake support in io_uring is useful because it allows for more
efficient directed wakeups. For some "locks" postgres has queues
implemented in userspace, with wakeup logic that cannot easily be
implemented with FUTEX_WAKE_BITSET on a single "futex word" (imagine
waiting for journal flushes to have completed up to a certain point). Thus
a "lock release" sometimes need to wake up many processes in a row. A
quick-and-dirty conversion to doing these wakeups via io_uring lead to a
3% throughput increase, with 12% fewer context switches, albeit in a
fairly extreme workload."
Some basic io_uring futex support and test cases are available in the
liburing 'futex' branch:
https://git.kernel.dk/cgit/liburing/log/?h=futex
testing all of the variants. I originally wrote this code about a
month ago and Andres has been using it with postgres, and I'm not
aware of any bugs in it. That's not to say it's perfect, obviously,
and I welcome some feedback so we can move this forward and hash out
any potential issues.
In terms of testing, there's a functionality and beat-up test case
in liburing, and I've run all the ltp futex test cases as well to
ensure we didn't inadvertently break anything. It's also been in
linux-next for a long time and haven't heard any complaints.
include/linux/io_uring_types.h | 5 +
include/uapi/linux/io_uring.h | 4 +
io_uring/Makefile | 1 +
io_uring/cancel.c | 5 +
io_uring/cancel.h | 4 +
io_uring/futex.c | 386 +++++++++++++++++++++++++++++++++
io_uring/futex.h | 36 +++
io_uring/io_uring.c | 7 +
io_uring/opdef.c | 34 +++
kernel/futex/futex.h | 20 ++
kernel/futex/requeue.c | 3 +-
kernel/futex/syscalls.c | 18 +-
kernel/futex/waitwake.c | 49 +++--
13 files changed, 545 insertions(+), 27 deletions(-)
You can also find the code here:
https://git.kernel.dk/cgit/linux/log/?h=io_uring-futex
V6:
- Expand the two main commit messages describing command layout
- Fix double conversion of FUTEX2_* flags (Peter)
- Use FLAGS_STRICT for IORING_OP_FUTEX_WAKE, so we return 0 wakes
when the caller asked for 0 (Peter)
- Fix issue with IORING_OP_FUTEX_WAITV and futex_wait_multiple_setup()
doing futex_unqueue_multiple() if we had a wakeup while setting up.
- Cleanup IORING_OP_FUTEX_WAITV issue path
- Don't use sqe->futex_flags for the FUTEX2_* flags, reserve it for
future internal use.
- Add more liburing test cases.
- Rebase on current tree (for-6.7/io_uring + tip locking/core)
--
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
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
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 a
pointer to a struct futex_waitv array, and sqe->off must now contain the
number of elements in that array. As flags are passed in the futex_vector
array, and likewise for the value and futex address(es), sqe->addr2
and sqe->addr3 are also reserved for IORING_OP_FUTEX_WAITV.
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 | 169 ++++++++++++++++++++++++++++++++--
io_uring/futex.h | 2 +
io_uring/opdef.c | 11 +++
4 files changed, 174 insertions(+), 9 deletions(-)
diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
index 04f9fba38d4b..92be89a871fc 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 eb4406ac46fb..3c3575303c3d 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;
}
@@ -147,6 +194,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->futex_flags || 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);
@@ -171,6 +267,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)
--
2.40.1
On Thu, Sep 28, 2023 at 11:25:09AM -0600, Jens Axboe wrote:
> include/linux/io_uring_types.h | 5 +
> include/uapi/linux/io_uring.h | 4 +
> io_uring/Makefile | 1 +
> io_uring/cancel.c | 5 +
> io_uring/cancel.h | 4 +
> io_uring/futex.c | 386 +++++++++++++++++++++++++++++++++
> io_uring/futex.h | 36 +++
> io_uring/io_uring.c | 7 +
> io_uring/opdef.c | 34 +++
> kernel/futex/futex.h | 20 ++
> kernel/futex/requeue.c | 3 +-
> kernel/futex/syscalls.c | 18 +-
> kernel/futex/waitwake.c | 49 +++--
> 13 files changed, 545 insertions(+), 27 deletions(-)
Thanks for bearing with us on the futex2 thing!
Acked-by: Peter Zijlstra (Intel) <[email protected]>
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
On 9/29/23 1:53 AM, Peter Zijlstra wrote:
> On Thu, Sep 28, 2023 at 11:25:09AM -0600, Jens Axboe wrote:
>
>> include/linux/io_uring_types.h | 5 +
>> include/uapi/linux/io_uring.h | 4 +
>> io_uring/Makefile | 1 +
>> io_uring/cancel.c | 5 +
>> io_uring/cancel.h | 4 +
>> io_uring/futex.c | 386 +++++++++++++++++++++++++++++++++
>> io_uring/futex.h | 36 +++
>> io_uring/io_uring.c | 7 +
>> io_uring/opdef.c | 34 +++
>> kernel/futex/futex.h | 20 ++
>> kernel/futex/requeue.c | 3 +-
>> kernel/futex/syscalls.c | 18 +-
>> kernel/futex/waitwake.c | 49 +++--
>> 13 files changed, 545 insertions(+), 27 deletions(-)
>
> Thanks for bearing with us on the futex2 thing!
>
> Acked-by: Peter Zijlstra (Intel) <[email protected]>
Thanks Peter! Going with the futex2 interface was the right choice, the
old one was kinda wonky anyway. New one is definitely cleaner.
--
Jens Axboe