linux-kernel - Re: [PATCH v4 1/3] locking/mutex: Add waiter parameter to mutex_optimistic

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 8 Aug 2016 19:26:59 +0200
From:	Peter Zijlstra <peterz@...radead.org>
To:	Waiman Long <Waiman.Long@....com>
Cc:	Ingo Molnar <mingo@...hat.com>, linux-kernel@...r.kernel.org,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Ding Tianhong <dingtianhong@...wei.com>,
	Jason Low <jason.low2@....com>,
	Davidlohr Bueso <dave@...olabs.net>,
	"Paul E. McKenney" <paulmck@...ibm.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Will Deacon <Will.Deacon@....com>,
	Tim Chen <tim.c.chen@...ux.intel.com>,
	Imre Deak <imre.deak@...el.com>,
	Waiman Long <Waiman.Long@...com>
Subject: Re: [PATCH v4 1/3] locking/mutex: Add waiter parameter to
 mutex_optimistic_spin()

On Mon, Jul 18, 2016 at 04:39:24PM -0400, Waiman Long wrote:
> @@ -302,22 +306,42 @@ static inline bool mutex_try_to_acquire(struct mutex *lock)
>   *
>   * Returns true when the lock was taken, otherwise false, indicating
>   * that we need to jump to the slowpath and sleep.
> + *
> + * The waiter flag is set to true if the spinner is a waiter in the wait
> + * queue. As the waiter has slept for a while, it should have priority to
> + * get the lock over the regular spinners. So going to wait at the end of
> + * the OSQ isn't fair to the waiter.

If the OSQ lock were a full FIFO it would in fact be fair, but its not
and things can drop out the middle and go (back) to sleep.

This has nothing to do with the end or not.

> Instead, it will spin on the lock
> + * directly and concurrently with the spinner at the head of the OSQ, if
> + * present. 

Note that this isn't starvation proof in any way.

>     There may be a bit more cacheline contention in this case.

This is relevant how ?

> + * The waiter also needs to set the lock to -1 instead of 0 on lock
> + * acquisition.

This is unrelated to the previous bits and thus should not be in the
same paragraph. Also, a 'why' would be more helpful.

>   */
>  static bool mutex_optimistic_spin(struct mutex *lock,
> -				  struct ww_acquire_ctx *ww_ctx, const bool use_ww_ctx)
> +				  struct ww_acquire_ctx *ww_ctx,
> +				  const bool use_ww_ctx, int waiter)
>  {