[2/7] futex: factor out the futex wake handling
Commit Message
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.
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
kernel/futex/futex.h | 4 ++++
kernel/futex/requeue.c | 3 ++-
kernel/futex/waitwake.c | 6 +++---
3 files changed, 9 insertions(+), 4 deletions(-)
Comments
On Tue, Jul 11, 2023 at 06:47:00PM -0600, Jens Axboe wrote:
> 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.
Urgh... so if I understand things right, you're going to replace this
with a custom wake function that does *NOT* put the task on the wake_q.
The wake_q will thus be empty and the task does not get woken up. I'm
presuming someone gets a notification instead somewhere somehow.
I might've been nice to mention some of this somewhere ...
> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> ---
> kernel/futex/futex.h | 4 ++++
> kernel/futex/requeue.c | 3 ++-
> kernel/futex/waitwake.c | 6 +++---
> 3 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/futex/futex.h b/kernel/futex/futex.h
> index d2949fca37d1..8eaf1a5ce967 100644
> --- a/kernel/futex/futex.h
> +++ b/kernel/futex/futex.h
> @@ -69,6 +69,9 @@ 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
> @@ -98,6 +101,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;
On 7/12/23 2:58?AM, Peter Zijlstra wrote:
> On Tue, Jul 11, 2023 at 06:47:00PM -0600, Jens Axboe wrote:
>> 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.
>
> Urgh... so if I understand things right, you're going to replace this
> with a custom wake function that does *NOT* put the task on the wake_q.
>
> The wake_q will thus be empty and the task does not get woken up. I'm
> presuming someone gets a notification instead somewhere somehow.
Right. The custom wake function is responsible for waking the task, if
that is what needs to happen. On the io_uring side, the callback would
queue work for the original task to post a completion event, which would
then also wake it up obviously.
> I might've been nice to mention some of this somewhere ...
I'll expand the commit message a bit, it is a bit light.
@@ -69,6 +69,9 @@ 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
@@ -98,6 +101,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;
@@ -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),
@@ -591,7 +592,7 @@ int futex_requeue(u32 __user *uaddr1, unsigned int flags, u32 __user *uaddr2,
/* 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;
@@ -174,7 +174,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;
}
@@ -289,7 +289,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;
}
@@ -303,7 +303,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;
}