From: "André Almeida via Libc-alpha" <libc-alpha@sourceware.org>
To: Peter Zijlstra <peterz@infradead.org>
Cc: dave@stgolabs.net, libc-alpha@sourceware.org,
linux-api@vger.kernel.org, bigeasy@linutronix.de,
linux-kernel@vger.kernel.org, rostedt@goodmis.org,
mingo@redhat.com, mtk.manpages@gmail.com, dvhart@infradead.org,
tglx@linutronix.de, kernel@collabora.com, krisman@collabora.com
Subject: Re: [PATCH 16/20] futex: Implement sys_futex_waitv()
Date: Wed, 15 Sep 2021 13:29:30 -0300 [thread overview]
Message-ID: <cbf1dbc3-6c38-0b53-8eb9-69530c8220b9@collabora.com> (raw)
In-Reply-To: <20210915141525.621568509@infradead.org>
Às 11:07 de 15/09/21, Peter Zijlstra escreveu:
> From: André Almeida <andrealmeid@collabora.com>
>
> Add support to wait on multiple futexes. This is the interface
> implemented by this syscall:
>
> futex_waitv(struct futex_waitv *waiters, unsigned int nr_futexes,
> unsigned int flags, struct timespec *timo)
>
> +/**
> + * futex_wait_multiple_setup - Prepare to wait and enqueue multiple futexes
> + * @vs: The futex list to wait on
> + * @count: The size of the list
> + * @awaken: Index of the last awoken futex, if any. Used to notify the
> + * caller that it can return this index to userspace (return parameter)
> + *
> + * Prepare multiple futexes in a single step and enqueue them. This may fail if
> + * the futex list is invalid or if any futex was already awoken. On success the
> + * task is ready to interruptible sleep.
> + *
> + * Return:
> + * - 1 - One of the futexes was awaken by another thread
> + * - 0 - Success
> + * - <0 - -EFAULT, -EWOULDBLOCK or -EINVAL
> + */
> +static int futex_wait_multiple_setup(struct futex_vector *vs, int count, int *awaken)
> +{
> + struct futex_hash_bucket *hb;
> + bool retry = false;
> + int ret, i;
> + u32 uval;
> +
> + /*
> + * Enqueuing multiple futexes is tricky, because we need to enqueue
> + * each futex in the list before dealing with the next one to avoid
> + * deadlocking on the hash bucket. But, before enqueuing, we need to
> + * make sure that current->state is TASK_INTERRUPTIBLE, so we don't
> + * absorb any awake events, which cannot be done before the
> + * get_futex_key of the next key, because it calls get_user_pages,
> + * which can sleep. Thus, we fetch the list of futexes keys in two
> + * steps, by first pinning all the memory keys in the futex key, and
> + * only then we read each key and queue the corresponding futex.
> + *
> + * Private futexes doesn't need to recalculate hash in retry, so skip
> + * get_futex_key() when retrying.
> + */
> +retry:
> + for (i = 0; i < count; i++) {
> + if ((vs[i].w.flags & FUTEX_PRIVATE_FLAG) && retry)
> + continue;
> +
> + ret = get_futex_key(u64_to_user_ptr(vs[i].w.uaddr),
> + !(vs[i].w.flags & FUTEX_PRIVATE_FLAG),
> + &vs[i].q.key, FUTEX_READ);
> +
> + if (unlikely(ret))
> + return ret;
> + }
> +
> + set_current_state(TASK_INTERRUPTIBLE);
> +
> + for (i = 0; i < count; i++) {
> + u32 __user *uaddr = (u32 __user *)(unsigned long)vs[i].w.uaddr;
> + struct futex_q *q = &vs[i].q;
> + u32 val = (u32)vs[i].w.val;
> +
> + hb = futex_q_lock(q);
> + ret = futex_get_value_locked(&uval, uaddr);
> +
> + if (!ret && uval == val) {
> + /*
> + * The bucket lock can't be held while dealing with the
> + * next futex. Queue each futex at this moment so hb can
> + * be unlocked.
> + */
> + futex_queue(q, hb);
> + continue;
> + }
> +
> + futex_q_unlock(hb);
> + __set_current_state(TASK_RUNNING);
> +
> + /*
> + * Even if something went wrong, if we find out that a futex
> + * was awaken, we don't return error and return this index to
> + * userspace
> + */
> + *awaken = unqueue_multiple(vs, i);
> + if (*awaken >= 0)
> + return 1;
> +
> + if (uval != val)
> + return -EWOULDBLOCK;
> +
> + if (ret) {
> + /*
> + * If we need to handle a page fault, we need to do so
> + * without any lock and any enqueued futex (otherwise
> + * we could lose some wakeup). So we do it here, after
> + * undoing all the work done so far. In success, we
> + * retry all the work.
> + */
> + if (get_user(uval, uaddr))
> + return -EFAULT;
> +
> + retry = true;
> + goto retry;
> + }
My bad again: the last two if's should be in the reserve order. If ret
!= 0, the user copy didn't succeed and the value wasn't copied to uval,
thus the comparison (uval != val) should happen only if ret == 0.
> + }
> +
> + return 0;
> +}
next prev parent reply other threads:[~2021-09-15 16:30 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-15 14:07 [PATCH 00/20] futex: splitup and waitv syscall Peter Zijlstra
2021-09-15 14:07 ` [PATCH 01/20] futex: Move to kernel/futex/ Peter Zijlstra
2021-09-15 14:07 ` [PATCH 02/20] futex: Split out syscalls Peter Zijlstra
2021-09-15 14:07 ` [PATCH 03/20] futex: Rename {,__}{,un}queue_me() Peter Zijlstra
2021-09-15 14:07 ` [PATCH 04/20] futex: Rename futex_wait_queue_me() Peter Zijlstra
2021-09-15 14:07 ` [PATCH 05/20] futex: Rename: queue_{,un}lock() Peter Zijlstra
2021-09-15 14:07 ` [PATCH 06/20] futex: Rename __unqueue_futex() Peter Zijlstra
2021-09-15 14:07 ` [PATCH 07/20] futex: Rename hash_futex() Peter Zijlstra
2021-09-15 15:17 ` André Almeida via Libc-alpha
2021-09-15 15:22 ` Peter Zijlstra
2021-09-15 14:07 ` [PATCH 08/20] futex: Rename: {get,cmpxchg}_futex_value_locked() Peter Zijlstra
2021-09-15 14:07 ` [PATCH 09/20] futex: Split out PI futex Peter Zijlstra
2021-09-15 14:07 ` [PATCH 10/20] futex: Rename: hb_waiter_{inc,dec,pending}() Peter Zijlstra
2021-09-15 14:07 ` [PATCH 11/20] futex: Rename: match_futex() Peter Zijlstra
2021-09-15 14:07 ` [PATCH 12/20] futex: Rename mark_wake_futex() Peter Zijlstra
2021-09-15 14:07 ` [PATCH 13/20] futex: Split out requeue Peter Zijlstra
2021-09-15 14:07 ` [PATCH 14/20] futex: Split out wait/wake Peter Zijlstra
2021-09-15 14:07 ` [PATCH 15/20] futex: Simplify double_lock_hb() Peter Zijlstra
2021-09-15 14:07 ` [PATCH 16/20] futex: Implement sys_futex_waitv() Peter Zijlstra
2021-09-15 15:20 ` André Almeida via Libc-alpha
2021-09-15 15:37 ` Peter Zijlstra
2021-09-15 17:34 ` Paul Eggert
2021-09-16 14:49 ` Thomas Gleixner via Libc-alpha
2021-09-16 18:54 ` André Almeida via Libc-alpha
2021-09-15 18:47 ` Arnd Bergmann
2021-09-15 16:29 ` André Almeida via Libc-alpha [this message]
2021-09-15 14:07 ` [PATCH 17/20] futex,x86: Wire up sys_futex_waitv() Peter Zijlstra
2021-09-15 14:07 ` [PATCH 18/20] futex,arm: " Peter Zijlstra
2021-09-15 14:07 ` [PATCH 19/20] selftests: futex: Add sys_futex_waitv() test Peter Zijlstra
2021-09-15 14:07 ` [PATCH 20/20] selftests: futex: Test sys_futex_waitv() timeout Peter Zijlstra
2021-09-15 15:13 ` [PATCH 00/20] futex: splitup and waitv syscall André Almeida via Libc-alpha
2021-09-15 18:24 ` André Almeida via Libc-alpha
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/libc/involved.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=cbf1dbc3-6c38-0b53-8eb9-69530c8220b9@collabora.com \
--to=libc-alpha@sourceware.org \
--cc=andrealmeid@collabora.com \
--cc=bigeasy@linutronix.de \
--cc=dave@stgolabs.net \
--cc=dvhart@infradead.org \
--cc=kernel@collabora.com \
--cc=krisman@collabora.com \
--cc=linux-api@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=mtk.manpages@gmail.com \
--cc=peterz@infradead.org \
--cc=rostedt@goodmis.org \
--cc=tglx@linutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).