linux-kernel - Re: [PATCH v2 2/3] mutex: Queue mutex spinners with MCS lock to reduce cacheline contention

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]

Message-ID: <1366086275.22463.25.camel@buesod1.americas.hpqcorp.net>
Date:	Mon, 15 Apr 2013 21:24:35 -0700
From:	Davidlohr Bueso <davidlohr.bueso@...com>
To:	Waiman Long <Waiman.Long@...com>
Cc:	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>,
	"H. Peter Anvin" <hpa@...or.com>,
	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
	David Howells <dhowells@...hat.com>,
	Dave Jones <davej@...hat.com>,
	Clark Williams <williams@...hat.com>,
	Peter Zijlstra <peterz@...radead.org>,
	linux-kernel@...r.kernel.org, x86@...nel.org,
	linux-arch@...r.kernel.org,
	"Chandramouleeswaran, Aswin" <aswin@...com>,
	"Norton, Scott J" <scott.norton@...com>,
	Rik van Riel <riel@...hat.com>
Subject: Re: [PATCH v2 2/3] mutex: Queue mutex spinners with MCS lock to
 reduce cacheline contention

On Mon, 2013-04-15 at 10:37 -0400, Waiman Long wrote:
[...]
> +typedef struct mspin_node {
> +	struct mspin_node *next;
> +	int		   locked;	/* 1 if lock acquired */
> +} mspin_node_t;
> +
> +typedef mspin_node_t	*mspin_lock_t;

I think we could do without the typedefs, specially mspin_lock_t.

> +
> +#define	MLOCK(mutex)	((mspin_lock_t *)&((mutex)->spin_mlock))
> +
> +static noinline void mspin_lock(mspin_lock_t *lock,  mspin_node_t *node)
> +{
> +	mspin_node_t *prev;
> +
> +	/* Init node */
> +	node->locked = 0;
> +	node->next   = NULL;
> +
> +	prev = xchg(lock, node);
> +	if (likely(prev == NULL)) {
> +		/* Lock acquired */
> +		node->locked = 1;
> +		return;
> +	}
> +	ACCESS_ONCE(prev->next) = node;
> +	smp_wmb();
> +	/* Wait until the lock holder passes the lock down */
> +	while (!ACCESS_ONCE(node->locked))
> +		arch_mutex_cpu_relax();
> +}
> +
> +static void mspin_unlock(mspin_lock_t *lock,  mspin_node_t *node)
> +{
> +	mspin_node_t *next = ACCESS_ONCE(node->next);
> +
> +	if (likely(!next)) {
> +		/*
> +		 * Release the lock by setting it to NULL
> +		 */
> +		if (cmpxchg(lock, node, NULL) == node)
> +			return;
> +		/* Wait until the next pointer is set */
> +		while (!(next = ACCESS_ONCE(node->next)))
> +			arch_mutex_cpu_relax();
> +	}
> +	barrier();
> +	ACCESS_ONCE(next->locked) = 1;
> +	smp_wmb();

Do we really need the compiler barrier call? The CPUs can reorder
anyway. I assume the smp_wbm() call makes sure no there's no funny
business before the next lock is acquired, might be worth commenting.

Thanks,
Davidlohr

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/