All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stephan Diestelhorst <stephan.diestelhorst@amd.com>
To: <xen-devel@lists.xensource.com>
Cc: Jeremy Fitzhardinge <jeremy@goop.org>,
	Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>,
	Nick Piggin <npiggin@kernel.dk>, KVM <kvm@vger.kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	the arch/x86 maintainers <x86@kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Marcelo Tosatti <mtosatti@redhat.com>,
	Andi Kleen <andi@firstfloor.org>, Avi Kivity <avi@redhat.com>,
	Jan Beulich <JBeulich@suse.com>, "H. Peter Anvin" <hpa@zytor.com>,
	Ingo Molnar <mingo@elte.hu>,
	Linus Torvalds <torvalds@linux-foundation.org>
Subject: Re: [Xen-devel] [PATCH 00/10] [PATCH RFC V2] Paravirtualized ticketlocks
Date: Mon, 10 Oct 2011 13:00:50 +0200	[thread overview]
Message-ID: <2523929.AGG4U997NO@d-allen> (raw)
In-Reply-To: <4E8DE7F1.3050108@goop.org>

On Thursday 06 October 2011, 13:40:01 Jeremy Fitzhardinge wrote:
> On 10/06/2011 07:04 AM, Stephan Diestelhorst wrote:
> > On Wednesday 28 September 2011, 14:49:56 Linus Torvalds wrote:
> >> Which certainly should *work*, but from a conceptual standpoint, isn't
> >> it just *much* nicer to say "we actually know *exactly* what the upper
> >> bits were".
> > Well, we really do NOT want atomicity here. What we really rather want
> > is sequentiality: free the lock, make the update visible, and THEN
> > check if someone has gone sleeping on it.
> >
> > Atomicity only conveniently enforces that the three do not happen in a
> > different order (with the store becoming visible after the checking
> > load).
> >
> > This does not have to be atomic, since spurious wakeups are not a
> > problem, in particular not with the FIFO-ness of ticket locks.
> >
> > For that the fence, additional atomic etc. would be IMHO much cleaner
> > than the crazy overflow logic.
> 
> All things being equal I'd prefer lock-xadd just because its easier to
> analyze the concurrency for, crazy overflow tests or no.  But if
> add+mfence turned out to be a performance win, then that would obviously
> tip the scales.
> 
> However, it looks like locked xadd is also has better performance:  on
> my Sandybridge laptop (2 cores, 4 threads), the add+mfence is 20% slower
> than locked xadd, so that pretty much settles it unless you think
> there'd be a dramatic difference on an AMD system.

Indeed, the fences are usually slower than locked RMWs, in particular,
if you do not need to add an instruction. I originally missed that
amazing stunt the GCC pulled off with replacing the branch with carry
flag magic. It seems that two twisted minds have found each other
here :)

One of my concerns was adding a branch in here... so that is settled,
and if everybody else feels like this is easier to reason about...
go ahead :) (I'll keep my itch to myself then.)

Stephan
-- 
Stephan Diestelhorst, AMD Operating System Research Center
stephan.diestelhorst@amd.com, Tel. +49 (0)351 448 356 719

Advanced Micro Devices GmbH
Einsteinring 24
85609 Aschheim
Germany

Geschaeftsfuehrer: Alberto Bozzo;
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632, WEEE-Reg-Nr: DE 12919551 


WARNING: multiple messages have this Message-ID (diff)
From: Stephan Diestelhorst <stephan.diestelhorst@amd.com>
To: xen-devel@lists.xensource.com
Cc: Jeremy Fitzhardinge <jeremy@goop.org>,
	Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>,
	Nick Piggin <npiggin@kernel.dk>, KVM <kvm@vger.kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	the arch/x86 maintainers <x86@kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Marcelo Tosatti <mtosatti@redhat.com>,
	Andi Kleen <andi@firstfloor.org>, Avi Kivity <avi@redhat.com>,
	Jan Beulich <JBeulich@suse.com>, "H. Peter Anvin" <hpa@zytor.com>,
	Ingo Molnar <mingo@elte.hu>,
	Linus Torvalds <torvalds@linux-foundation.org>
Subject: Re: [Xen-devel] [PATCH 00/10] [PATCH RFC V2] Paravirtualized ticketlocks
Date: Mon, 10 Oct 2011 13:00:50 +0200	[thread overview]
Message-ID: <2523929.AGG4U997NO@d-allen> (raw)
In-Reply-To: <4E8DE7F1.3050108@goop.org>

On Thursday 06 October 2011, 13:40:01 Jeremy Fitzhardinge wrote:
> On 10/06/2011 07:04 AM, Stephan Diestelhorst wrote:
> > On Wednesday 28 September 2011, 14:49:56 Linus Torvalds wrote:
> >> Which certainly should *work*, but from a conceptual standpoint, isn't
> >> it just *much* nicer to say "we actually know *exactly* what the upper
> >> bits were".
> > Well, we really do NOT want atomicity here. What we really rather want
> > is sequentiality: free the lock, make the update visible, and THEN
> > check if someone has gone sleeping on it.
> >
> > Atomicity only conveniently enforces that the three do not happen in a
> > different order (with the store becoming visible after the checking
> > load).
> >
> > This does not have to be atomic, since spurious wakeups are not a
> > problem, in particular not with the FIFO-ness of ticket locks.
> >
> > For that the fence, additional atomic etc. would be IMHO much cleaner
> > than the crazy overflow logic.
> 
> All things being equal I'd prefer lock-xadd just because its easier to
> analyze the concurrency for, crazy overflow tests or no.  But if
> add+mfence turned out to be a performance win, then that would obviously
> tip the scales.
> 
> However, it looks like locked xadd is also has better performance:  on
> my Sandybridge laptop (2 cores, 4 threads), the add+mfence is 20% slower
> than locked xadd, so that pretty much settles it unless you think
> there'd be a dramatic difference on an AMD system.

Indeed, the fences are usually slower than locked RMWs, in particular,
if you do not need to add an instruction. I originally missed that
amazing stunt the GCC pulled off with replacing the branch with carry
flag magic. It seems that two twisted minds have found each other
here :)

One of my concerns was adding a branch in here... so that is settled,
and if everybody else feels like this is easier to reason about...
go ahead :) (I'll keep my itch to myself then.)

Stephan
-- 
Stephan Diestelhorst, AMD Operating System Research Center
stephan.diestelhorst@amd.com, Tel. +49 (0)351 448 356 719

Advanced Micro Devices GmbH
Einsteinring 24
85609 Aschheim
Germany

Geschaeftsfuehrer: Alberto Bozzo;
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632, WEEE-Reg-Nr: DE 12919551 

  parent reply	other threads:[~2011-10-10 11:02 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-09-15  0:31 [PATCH 00/10] [PATCH RFC V2] Paravirtualized ticketlocks Jeremy Fitzhardinge
2011-09-15  0:31 ` [PATCH 01/10] x86/ticketlocks: remove obsolete comment Jeremy Fitzhardinge
2011-09-15  0:31 ` [PATCH 02/10] x86/spinlocks: replace pv spinlocks with pv ticketlocks Jeremy Fitzhardinge
2011-09-15  0:31 ` [PATCH 03/10] x86/ticketlock: don't inline _spin_unlock when using paravirt spinlocks Jeremy Fitzhardinge
2011-09-15  0:31 ` [PATCH 04/10] x86/ticketlock: collapse a layer of functions Jeremy Fitzhardinge
2011-09-15  0:31 ` [PATCH 05/10] xen/pvticketlock: Xen implementation for PV ticket locks Jeremy Fitzhardinge
2011-09-15  0:31 ` [PATCH 06/10] x86/pvticketlock: use callee-save for lock_spinning Jeremy Fitzhardinge
2011-09-15  0:31 ` [PATCH 07/10] x86/ticketlocks: when paravirtualizing ticket locks, increment by 2 Jeremy Fitzhardinge
2011-09-15  0:31   ` Jeremy Fitzhardinge
2011-09-15  0:31 ` [PATCH 08/10] x86/ticketlock: add slowpath logic Jeremy Fitzhardinge
2011-09-15  0:31 ` [PATCH 09/10] xen/pvticketlock: allow interrupts to be enabled while blocking Jeremy Fitzhardinge
2011-09-15  0:31 ` [PATCH 10/10] xen: enable PV ticketlocks on HVM Xen Jeremy Fitzhardinge
2011-09-27  9:34 ` [Xen-devel] [PATCH 00/10] [PATCH RFC V2] Paravirtualized ticketlocks Stephan Diestelhorst
2011-09-27  9:34   ` Stephan Diestelhorst
2011-09-27  9:34   ` Stephan Diestelhorst
2011-09-27 16:44   ` [Xen-devel] " Jeremy Fitzhardinge
2011-09-27 16:44     ` Jeremy Fitzhardinge
2011-09-28 13:58     ` [Xen-devel] " Stephan Diestelhorst
2011-09-28 16:44       ` Jeremy Fitzhardinge
2011-09-28 18:13         ` Stephan Diestelhorst
2011-09-28 15:38     ` Linus Torvalds
2011-09-28 15:55       ` Jan Beulich
2011-09-28 15:55         ` Jan Beulich
2011-09-28 16:10         ` Linus Torvalds
2011-09-28 16:47           ` Jeremy Fitzhardinge
2011-09-28 17:22             ` Linus Torvalds
2011-09-28 17:24               ` H. Peter Anvin
2011-09-28 17:50                 ` Jeremy Fitzhardinge
2011-09-28 18:08                   ` Stephan Diestelhorst
2011-09-28 18:27                     ` Jeremy Fitzhardinge
2011-09-28 18:49                     ` Linus Torvalds
2011-09-28 19:06                       ` Jeremy Fitzhardinge
2011-10-06 14:04                       ` Stephan Diestelhorst
2011-10-06 17:40                         ` Jeremy Fitzhardinge
2011-10-06 18:09                           ` Jeremy Fitzhardinge
2011-10-10  7:32                             ` Ingo Molnar
2011-10-10 19:51                               ` Jeremy Fitzhardinge
2011-10-10 11:00                           ` Stephan Diestelhorst [this message]
2011-10-10 11:00                             ` Stephan Diestelhorst
2011-10-10 14:01                             ` Stephan Diestelhorst
2011-10-10 14:01                               ` Stephan Diestelhorst
2011-10-10 19:44                               ` Jeremy Fitzhardinge

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=2523929.AGG4U997NO@d-allen \
    --to=stephan.diestelhorst@amd.com \
    --cc=JBeulich@suse.com \
    --cc=andi@firstfloor.org \
    --cc=avi@redhat.com \
    --cc=hpa@zytor.com \
    --cc=jeremy.fitzhardinge@citrix.com \
    --cc=jeremy@goop.org \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=mtosatti@redhat.com \
    --cc=npiggin@kernel.dk \
    --cc=peterz@infradead.org \
    --cc=torvalds@linux-foundation.org \
    --cc=x86@kernel.org \
    --cc=xen-devel@lists.xensource.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.