All of lore.kernel.org
 help / color / mirror / Atom feed
From: Torsten Duwe <duwe@lst.de>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Tom Musta <tommusta@gmail.com>,
	linux-kernel@vger.kernel.org, Paul Mackerras <paulus@samba.org>,
	Anton Blanchard <anton@samba.org>,
	Scott Wood <scottwood@freescale.com>,
	"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
	linuxppc-dev@lists.ozlabs.org, Ingo Molnar <mingo@kernel.org>
Subject: Re: [PATCH] Convert powerpc simple spinlocks into ticket locks
Date: Fri, 7 Feb 2014 18:08:45 +0100	[thread overview]
Message-ID: <20140207170845.GD2107@lst.de> (raw)
In-Reply-To: <20140207151847.GB3104@twins.programming.kicks-ass.net>

On Fri, Feb 07, 2014 at 04:18:47PM +0100, Peter Zijlstra wrote:
> On Fri, Feb 07, 2014 at 01:28:37PM +0100, Peter Zijlstra wrote:
> > Anyway, you can do a version with lwarx/stwcx if you're looking get rid
> > of lharx.
> 
> the below seems to compile into relatively ok asm. It can be done better
> if you write the entire thing by hand though.

[...]

> 
> static inline unsigned int xadd(unsigned int *v, unsigned int i)
> {
> 	int t, ret;
> 	
> 	__asm__ __volatile__ (
> "1:	lwarx	%0, 0, %4\n"
> "	mr	%1, %0\n"
> "	add	%0, %3, %0\n"
> "	stwcx.	%0, %0, %4\n"
> "	bne-	1b\n"
> 	: "=&r" (t), "=&r" (ret), "+m" (*v)
> 	: "r" (i), "r" (v)
> 	: "cc");
> 
> 	return ret;
> }
> 
I don't like this xadd thing -- it's so x86 ;)
x86 has its LOCK prefix, ppc has ll/sc.
That should be reflected somehow IMHO.

Maybe if xadd became mandatory for some kernel library.

> 
> void ticket_unlock(tickets_t *lock)
> {
> 	ticket_t tail = lock->tail + 1;
> 
> 	/*
> 	 * The store is save against the xadd for it will make the ll/sc fail
> 	 * and try again. Aside from that PowerISA guarantees single-copy
> 	 * atomicy for half-word writes.
> 	 *
> 	 * And since only the lock owner will ever write the tail, we're good.
> 	 */
> 	smp_store_release(&lock->tail, tail);
> }

Yeah, let's try that on top of v2 (just posted).
First, I want to see v2 work as nicely as v1 --
compiling a debug kernel takes a while...

	Torsten

WARNING: multiple messages have this Message-ID (diff)
From: Torsten Duwe <duwe@lst.de>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Scott Wood <scottwood@freescale.com>,
	linux-kernel@vger.kernel.org, Paul Mackerras <paulus@samba.org>,
	Anton Blanchard <anton@samba.org>, Tom Musta <tommusta@gmail.com>,
	"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
	linuxppc-dev@lists.ozlabs.org, Ingo Molnar <mingo@kernel.org>
Subject: Re: [PATCH] Convert powerpc simple spinlocks into ticket locks
Date: Fri, 7 Feb 2014 18:08:45 +0100	[thread overview]
Message-ID: <20140207170845.GD2107@lst.de> (raw)
In-Reply-To: <20140207151847.GB3104@twins.programming.kicks-ass.net>

On Fri, Feb 07, 2014 at 04:18:47PM +0100, Peter Zijlstra wrote:
> On Fri, Feb 07, 2014 at 01:28:37PM +0100, Peter Zijlstra wrote:
> > Anyway, you can do a version with lwarx/stwcx if you're looking get rid
> > of lharx.
> 
> the below seems to compile into relatively ok asm. It can be done better
> if you write the entire thing by hand though.

[...]

> 
> static inline unsigned int xadd(unsigned int *v, unsigned int i)
> {
> 	int t, ret;
> 	
> 	__asm__ __volatile__ (
> "1:	lwarx	%0, 0, %4\n"
> "	mr	%1, %0\n"
> "	add	%0, %3, %0\n"
> "	stwcx.	%0, %0, %4\n"
> "	bne-	1b\n"
> 	: "=&r" (t), "=&r" (ret), "+m" (*v)
> 	: "r" (i), "r" (v)
> 	: "cc");
> 
> 	return ret;
> }
> 
I don't like this xadd thing -- it's so x86 ;)
x86 has its LOCK prefix, ppc has ll/sc.
That should be reflected somehow IMHO.

Maybe if xadd became mandatory for some kernel library.

> 
> void ticket_unlock(tickets_t *lock)
> {
> 	ticket_t tail = lock->tail + 1;
> 
> 	/*
> 	 * The store is save against the xadd for it will make the ll/sc fail
> 	 * and try again. Aside from that PowerISA guarantees single-copy
> 	 * atomicy for half-word writes.
> 	 *
> 	 * And since only the lock owner will ever write the tail, we're good.
> 	 */
> 	smp_store_release(&lock->tail, tail);
> }

Yeah, let's try that on top of v2 (just posted).
First, I want to see v2 work as nicely as v1 --
compiling a debug kernel takes a while...

	Torsten


  parent reply	other threads:[~2014-02-07 17:08 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-02-06 10:37 [PATCH] Convert powerpc simple spinlocks into ticket locks Torsten Duwe
2014-02-06 10:37 ` Torsten Duwe
2014-02-06 15:53 ` Benjamin Herrenschmidt
2014-02-06 15:53   ` Benjamin Herrenschmidt
2014-02-06 16:38 ` Peter Zijlstra
2014-02-06 16:38   ` Peter Zijlstra
2014-02-06 17:37   ` Torsten Duwe
2014-02-06 17:37     ` Torsten Duwe
2014-02-06 18:08     ` Peter Zijlstra
2014-02-06 18:08       ` Peter Zijlstra
2014-02-06 19:28       ` Tom Musta
2014-02-10  2:54         ` Benjamin Herrenschmidt
2014-02-10  2:54           ` Benjamin Herrenschmidt
2014-02-07  8:24       ` Torsten Duwe
2014-02-07  8:24         ` Torsten Duwe
2014-02-06 20:19     ` Scott Wood
2014-02-06 20:19       ` Scott Wood
2014-02-07  9:02       ` Torsten Duwe
2014-02-07  9:02         ` Torsten Duwe
2014-02-07 10:31         ` Peter Zijlstra
2014-02-07 10:31           ` Peter Zijlstra
2014-02-07 10:36           ` Peter Zijlstra
2014-02-07 10:36             ` Peter Zijlstra
2014-02-07 10:45           ` Peter Zijlstra
2014-02-07 10:45             ` Peter Zijlstra
2014-02-07 11:49             ` Torsten Duwe
2014-02-07 11:49               ` Torsten Duwe
2014-02-07 12:28               ` Peter Zijlstra
2014-02-07 12:28                 ` Peter Zijlstra
2014-02-07 15:18                 ` Peter Zijlstra
2014-02-07 15:18                   ` Peter Zijlstra
2014-02-07 15:43                   ` Peter Zijlstra
2014-02-07 15:43                     ` Peter Zijlstra
2014-02-07 17:08                   ` Torsten Duwe [this message]
2014-02-07 17:08                     ` Torsten Duwe
2014-02-07 17:19                     ` Peter Zijlstra
2014-02-07 17:19                       ` Peter Zijlstra
2014-02-07 15:51         ` Kumar Gala
2014-02-07 15:51           ` Kumar Gala
2014-02-07 16:10           ` Peter Zijlstra
2014-02-07 16:10             ` Peter Zijlstra
2014-02-10  3:05           ` Benjamin Herrenschmidt
2014-02-10  3:05             ` Benjamin Herrenschmidt
2014-02-10  3:02         ` Benjamin Herrenschmidt
2014-02-10  3:02           ` Benjamin Herrenschmidt

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=20140207170845.GD2107@lst.de \
    --to=duwe@lst.de \
    --cc=anton@samba.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mingo@kernel.org \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=paulus@samba.org \
    --cc=peterz@infradead.org \
    --cc=scottwood@freescale.com \
    --cc=tommusta@gmail.com \
    /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.