futex: Move futex_queue() into futex_wait_setup()
authorPeter Zijlstra <peterz@infradead.org>
Wed, 16 Apr 2025 16:29:03 +0000 (18:29 +0200)
committerPeter Zijlstra <peterz@infradead.org>
Sat, 3 May 2025 10:02:05 +0000 (12:02 +0200)
futex_wait_setup() has a weird calling convention in order to return
hb to use as an argument to futex_queue().

Mostly such that requeue can have an extra test in between.

Reorder code a little to get rid of this and keep the hb usage inside
futex_wait_setup().

[bigeasy: fixes]

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lore.kernel.org/r/20250416162921.513656-4-bigeasy@linutronix.de
io_uring/futex.c
kernel/futex/futex.h
kernel/futex/requeue.c
kernel/futex/waitwake.c

index 0ea4820cd8ff8ca4c472c4250a78ec7734d427f6..e89c0897117ae5ceb98907220db029f7bcd6e694 100644 (file)
@@ -273,7 +273,6 @@ 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) {
@@ -295,12 +294,11 @@ int io_futex_wait(struct io_kiocb *req, unsigned int issue_flags)
        ifd->req = req;
 
        ret = futex_wait_setup(iof->uaddr, iof->futex_val, iof->futex_flags,
-                              &ifd->q, &hb);
+                              &ifd->q, NULL, NULL);
        if (!ret) {
                hlist_add_head(&req->hash_node, &ctx->futex_list);
                io_ring_submit_unlock(ctx, issue_flags);
 
-               futex_queue(&ifd->q, hb, NULL);
                return IOU_ISSUE_SKIP_COMPLETE;
        }
 
index 6b2f4c7eb720fdefc137bfd28db2a53d8fd99761..16aafd0113442b1bea53f18009ab45ef70c0076d 100644 (file)
@@ -219,9 +219,9 @@ static inline int futex_match(union futex_key *key1, union futex_key *key2)
 }
 
 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);
+                           struct futex_q *q, union futex_key *key2,
+                           struct task_struct *task);
+extern void futex_do_wait(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);
 
index b47bb764b352015026f163224b73c83c85a5f860..0e55975af515c9c432ebf187c95ca2c76c7097d1 100644 (file)
@@ -769,7 +769,6 @@ int futex_wait_requeue_pi(u32 __user *uaddr, unsigned int flags,
 {
        struct hrtimer_sleeper timeout, *to;
        struct rt_mutex_waiter rt_waiter;
-       struct futex_hash_bucket *hb;
        union futex_key key2 = FUTEX_KEY_INIT;
        struct futex_q q = futex_q_init;
        struct rt_mutex_base *pi_mutex;
@@ -805,29 +804,24 @@ int futex_wait_requeue_pi(u32 __user *uaddr, unsigned int flags,
         * Prepare to wait on uaddr. On success, it holds hb->lock and q
         * is initialized.
         */
-       ret = futex_wait_setup(uaddr, val, flags, &q, &hb);
+       ret = futex_wait_setup(uaddr, val, flags, &q, &key2, current);
        if (ret)
                goto out;
 
-       /*
-        * The check above which compares uaddrs is not sufficient for
-        * shared futexes. We need to compare the keys:
-        */
-       if (futex_match(&q.key, &key2)) {
-               futex_q_unlock(hb);
-               ret = -EINVAL;
-               goto out;
-       }
-
        /* Queue the futex_q, drop the hb lock, wait for wakeup. */
-       futex_wait_queue(hb, &q, to);
+       futex_do_wait(&q, to);
 
        switch (futex_requeue_pi_wakeup_sync(&q)) {
        case Q_REQUEUE_PI_IGNORE:
-               /* The waiter is still on uaddr1 */
-               spin_lock(&hb->lock);
-               ret = handle_early_requeue_pi_wakeup(hb, &q, to);
-               spin_unlock(&hb->lock);
+               {
+                       struct futex_hash_bucket *hb;
+
+                       hb = futex_hash(&q.key);
+                       /* The waiter is still on uaddr1 */
+                       spin_lock(&hb->lock);
+                       ret = handle_early_requeue_pi_wakeup(hb, &q, to);
+                       spin_unlock(&hb->lock);
+               }
                break;
 
        case Q_REQUEUE_PI_LOCKED:
index 25877d4f2f8f3fea76f62820d8a57739128a4fd2..6cf10701294b4da9473403b68e8ecd51e5dbf226 100644 (file)
@@ -339,18 +339,8 @@ static long futex_wait_restart(struct restart_block *restart);
  * @q:         the futex_q to queue up on
  * @timeout:   the prepared hrtimer_sleeper, or null for no timeout
  */
-void futex_wait_queue(struct futex_hash_bucket *hb, struct futex_q *q,
-                           struct hrtimer_sleeper *timeout)
+void futex_do_wait(struct futex_q *q, struct hrtimer_sleeper *timeout)
 {
-       /*
-        * The task state is guaranteed to be set before another task can
-        * wake it. set_current_state() is implemented using smp_store_mb() and
-        * futex_queue() calls spin_unlock() upon completion, both serializing
-        * access to the hash list and forcing another memory barrier.
-        */
-       set_current_state(TASK_INTERRUPTIBLE|TASK_FREEZABLE);
-       futex_queue(q, hb, current);
-
        /* Arm the timer */
        if (timeout)
                hrtimer_sleeper_start_expires(timeout, HRTIMER_MODE_ABS);
@@ -578,7 +568,8 @@ int futex_wait_multiple(struct futex_vector *vs, unsigned int count,
  * @val:       the expected value
  * @flags:     futex flags (FLAGS_SHARED, etc.)
  * @q:         the associated futex_q
- * @hb:                storage for hash_bucket pointer to be returned to caller
+ * @key2:      the second futex_key if used for requeue PI
+ * task:       Task queueing this futex
  *
  * Setup the futex_q and locate the hash_bucket.  Get the futex value and
  * compare it with the expected value.  Handle atomic faults internally.
@@ -589,8 +580,10 @@ int futex_wait_multiple(struct futex_vector *vs, unsigned int count,
  *  - <1 - -EFAULT or -EWOULDBLOCK (uaddr does not contain val) and hb is unlocked
  */
 int futex_wait_setup(u32 __user *uaddr, u32 val, unsigned int flags,
-                    struct futex_q *q, struct futex_hash_bucket **hb)
+                    struct futex_q *q, union futex_key *key2,
+                    struct task_struct *task)
 {
+       struct futex_hash_bucket *hb;
        u32 uval;
        int ret;
 
@@ -618,12 +611,12 @@ retry:
                return ret;
 
 retry_private:
-       *hb = futex_q_lock(q);
+       hb = futex_q_lock(q);
 
        ret = futex_get_value_locked(&uval, uaddr);
 
        if (ret) {
-               futex_q_unlock(*hb);
+               futex_q_unlock(hb);
 
                ret = get_user(uval, uaddr);
                if (ret)
@@ -636,10 +629,25 @@ retry_private:
        }
 
        if (uval != val) {
-               futex_q_unlock(*hb);
-               ret = -EWOULDBLOCK;
+               futex_q_unlock(hb);
+               return -EWOULDBLOCK;
        }
 
+       if (key2 && futex_match(&q->key, key2)) {
+               futex_q_unlock(hb);
+               return -EINVAL;
+       }
+
+       /*
+        * The task state is guaranteed to be set before another task can
+        * wake it. set_current_state() is implemented using smp_store_mb() and
+        * futex_queue() calls spin_unlock() upon completion, both serializing
+        * access to the hash list and forcing another memory barrier.
+        */
+       if (task == current)
+               set_current_state(TASK_INTERRUPTIBLE|TASK_FREEZABLE);
+       futex_queue(q, hb, task);
+
        return ret;
 }
 
@@ -647,7 +655,6 @@ int __futex_wait(u32 __user *uaddr, unsigned int flags, u32 val,
                 struct hrtimer_sleeper *to, u32 bitset)
 {
        struct futex_q q = futex_q_init;
-       struct futex_hash_bucket *hb;
        int ret;
 
        if (!bitset)
@@ -660,12 +667,12 @@ retry:
         * Prepare to wait on uaddr. On success, it holds hb->lock and q
         * is initialized.
         */
-       ret = futex_wait_setup(uaddr, val, flags, &q, &hb);
+       ret = futex_wait_setup(uaddr, val, flags, &q, NULL, current);
        if (ret)
                return ret;
 
        /* futex_queue and wait for wakeup, timeout, or a signal. */
-       futex_wait_queue(hb, &q, to);
+       futex_do_wait(&q, to);
 
        /* If we were woken (and unqueued), we succeeded, whatever. */
        if (!futex_unqueue(&q))