All of lore.kernel.org
 help / color / mirror / Atom feed
From: "André Almeida" <andrealmeid@collabora.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: linux-kernel@vger.kernel.org, kernel@collabora.com,
	bigeasy@linutronix.de, krisman@collabora.com,
	linux-api@vger.kernel.org, libc-alpha@sourceware.org,
	mingo@redhat.com, rostedt@goodmis.org, dvhart@infradead.org,
	mtk.manpages@gmail.com, dave@stgolabs.net, arnd@arndb.de,
	tglx@linutronix.de
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;
> +}

	

  parent reply	other threads:[~2021-09-15 16:29 UTC|newest]

Thread overview: 27+ 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
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 15:13 ` [PATCH 00/20] futex: splitup and waitv syscall André Almeida
     [not found] ` <20210915141525.621568509@infradead.org>
2021-09-15 15:20   ` [PATCH 16/20] futex: Implement sys_futex_waitv() André Almeida
2021-09-15 15:37   ` Peter Zijlstra
2021-09-15 17:34     ` Paul Eggert
2021-09-16 14:49       ` Thomas Gleixner
2021-09-16 18:54         ` André Almeida
2021-09-15 18:47     ` Arnd Bergmann
2021-09-15 16:29   ` André Almeida [this message]
2021-09-15 18:24 ` [PATCH 00/20] futex: splitup and waitv syscall André Almeida

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

* 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=andrealmeid@collabora.com \
    --cc=arnd@arndb.de \
    --cc=bigeasy@linutronix.de \
    --cc=dave@stgolabs.net \
    --cc=dvhart@infradead.org \
    --cc=kernel@collabora.com \
    --cc=krisman@collabora.com \
    --cc=libc-alpha@sourceware.org \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.