From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1752366AbcHHR1Y (ORCPT <rfc822;w@1wt.eu>);
	Mon, 8 Aug 2016 13:27:24 -0400
Received: from merlin.infradead.org ([205.233.59.134]:56656 "EHLO
	merlin.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1752128AbcHHR1W (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Mon, 8 Aug 2016 13:27:22 -0400
Date: Mon, 8 Aug 2016 19:26:59 +0200
From: Peter Zijlstra <peterz@infradead.org>
To: Waiman Long <Waiman.Long@hpe.com>
Cc: Ingo Molnar <mingo@redhat.com>, linux-kernel@vger.kernel.org,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Ding Tianhong <dingtianhong@huawei.com>,
        Jason Low <jason.low2@hpe.com>, Davidlohr Bueso <dave@stgolabs.net>,
        "Paul E. McKenney" <paulmck@us.ibm.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Will Deacon <Will.Deacon@arm.com>,
        Tim Chen <tim.c.chen@linux.intel.com>, Imre Deak <imre.deak@intel.com>,
        Waiman Long <Waiman.Long@hp.com>
Subject: Re: [PATCH v4 1/3] locking/mutex: Add waiter parameter to
 mutex_optimistic_spin()
Message-ID: <20160808172659.GX6862@twins.programming.kicks-ass.net>
References: <1468874366-56955-1-git-send-email-Waiman.Long@hpe.com>
 <1468874366-56955-2-git-send-email-Waiman.Long@hpe.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1468874366-56955-2-git-send-email-Waiman.Long@hpe.com>
User-Agent: Mutt/1.5.23.1 (2014-03-12)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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)
>  {