2019-07-31 02:34:17

by Gabriel Krisman Bertazi

[permalink] [raw]
Subject: [PATCH RFC 1/2] futex: Split key setup from key queue locking and read

split the futex key setup from the queue locking and key reading. This
is useful to support the setup of multiple keys at the same time, like
what is done in futex_requeue() and what will be done for the
FUTEX_WAIT_MULTIPLE command.

Signed-off-by: Gabriel Krisman Bertazi <[email protected]>
---
kernel/futex.c | 71 +++++++++++++++++++++++++++++---------------------
1 file changed, 42 insertions(+), 29 deletions(-)

diff --git a/kernel/futex.c b/kernel/futex.c
index 6d50728ef2e7..91f3db335c57 100644
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -2631,6 +2631,39 @@ static void futex_wait_queue_me(struct futex_hash_bucket *hb, struct futex_q *q,
__set_current_state(TASK_RUNNING);
}

+static int __futex_wait_setup(u32 __user *uaddr, u32 val, unsigned int flags,
+ struct futex_q *q, struct futex_hash_bucket **hb)
+{
+
+ u32 uval;
+ int ret;
+
+retry_private:
+ *hb = queue_lock(q);
+
+ ret = get_futex_value_locked(&uval, uaddr);
+
+ if (ret) {
+ queue_unlock(*hb);
+
+ ret = get_user(uval, uaddr);
+ if (ret)
+ return ret;
+
+ if (!(flags & FLAGS_SHARED))
+ goto retry_private;
+
+ return 1;
+ }
+
+ if (uval != val) {
+ queue_unlock(*hb);
+ ret = -EWOULDBLOCK;
+ }
+
+ return ret;
+}
+
/**
* futex_wait_setup() - Prepare to wait on a futex
* @uaddr: the futex userspace address
@@ -2651,7 +2684,6 @@ static void futex_wait_queue_me(struct futex_hash_bucket *hb, struct futex_q *q,
static int futex_wait_setup(u32 __user *uaddr, u32 val, unsigned int flags,
struct futex_q *q, struct futex_hash_bucket **hb)
{
- u32 uval;
int ret;

/*
@@ -2672,38 +2704,19 @@ static int futex_wait_setup(u32 __user *uaddr, u32 val, unsigned int flags,
* absorb a wakeup if *uaddr does not match the desired values
* while the syscall executes.
*/
-retry:
- ret = get_futex_key(uaddr, flags & FLAGS_SHARED, &q->key, FUTEX_READ);
- if (unlikely(ret != 0))
- return ret;
-
-retry_private:
- *hb = queue_lock(q);
+ do {
+ ret = get_futex_key(uaddr, flags & FLAGS_SHARED,
+ &q->key, FUTEX_READ);
+ if (unlikely(ret != 0))
+ return ret;

- ret = get_futex_value_locked(&uval, uaddr);
+ ret = __futex_wait_setup(uaddr, val, flags, q, hb);

- if (ret) {
- queue_unlock(*hb);
-
- ret = get_user(uval, uaddr);
+ /* Drop key reference if retry or error. */
if (ret)
- goto out;
+ put_futex_key(&q->key);
+ } while (ret > 0);

- if (!(flags & FLAGS_SHARED))
- goto retry_private;
-
- put_futex_key(&q->key);
- goto retry;
- }
-
- if (uval != val) {
- queue_unlock(*hb);
- ret = -EWOULDBLOCK;
- }
-
-out:
- if (ret)
- put_futex_key(&q->key);
return ret;
}

--
2.20.1


2019-08-01 00:42:29

by Gabriel Krisman Bertazi

[permalink] [raw]
Subject: Re: [PATCH RFC 1/2] futex: Split key setup from key queue locking and read

Gabriel Krisman Bertazi <[email protected]> writes:

> Thomas Gleixner <[email protected]> writes:
>> What has this to do with futex_requeue()? Absolutely nothing unleass you
>> can reused that code there, which I doubt.
>
>I think it could be reused there

Though I admit to not having tried it out. I suppose I can just drop
the reference from the commit message when I submit the new version.


--
Gabriel Krisman Bertazi

2019-08-01 00:45:26

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH RFC 1/2] futex: Split key setup from key queue locking and read

On Wed, 31 Jul 2019, Gabriel Krisman Bertazi wrote:
> Thomas Gleixner <[email protected]> writes:
>
> > On Tue, 30 Jul 2019, Gabriel Krisman Bertazi wrote:
> >
> >> split the futex key setup from the queue locking and key reading. This
> >> is useful to support the setup of multiple keys at the same time, like
> >> what is done in futex_requeue() and what will be done for the
> >
> > What has this to do with futex_requeue()? Absolutely nothing unleass you
> > can reused that code there, which I doubt.
>
> futex_requeue is another place where more than one key is setup at a
> time. Just that. I think it could be reused there, but this change is
> out of scope for this patch.

No it can't. And if it could, then it would be definitely in scope of this
patch set to reuse functionality.

Thanks,

tglx

2019-08-01 01:18:17

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH RFC 1/2] futex: Split key setup from key queue locking and read

On Tue, 30 Jul 2019, Gabriel Krisman Bertazi wrote:

> split the futex key setup from the queue locking and key reading. This
> is useful to support the setup of multiple keys at the same time, like
> what is done in futex_requeue() and what will be done for the

What has this to do with futex_requeue()? Absolutely nothing unleass you
can reused that code there, which I doubt.

Thanks,

tglx

2019-08-01 01:21:30

by Gabriel Krisman Bertazi

[permalink] [raw]
Subject: Re: [PATCH RFC 1/2] futex: Split key setup from key queue locking and read

Thomas Gleixner <[email protected]> writes:

> On Tue, 30 Jul 2019, Gabriel Krisman Bertazi wrote:
>
>> split the futex key setup from the queue locking and key reading. This
>> is useful to support the setup of multiple keys at the same time, like
>> what is done in futex_requeue() and what will be done for the
>
> What has this to do with futex_requeue()? Absolutely nothing unleass you
> can reused that code there, which I doubt.

futex_requeue is another place where more than one key is setup at a
time. Just that. I think it could be reused there, but this change is
out of scope for this patch.

--
Gabriel Krisman Bertazi