linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
To: xinhui <xinhui.pan@linux.vnet.ibm.com>,
	linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
	virtualization@lists.linux-foundation.org
Cc: paulus@samba.org, mpe@ellerman.id.au, peterz@infradead.org,
	mingo@redhat.com, paulmck@linux.vnet.ibm.com,
	waiman.long@hpe.com
Subject: Re: [PATCH v5 1/6] qspinlock: powerpc support qspinlock
Date: Fri, 03 Jun 2016 14:33:47 +1000	[thread overview]
Message-ID: <1464928427.26773.26.camel@kernel.crashing.org> (raw)
In-Reply-To: <57510353.1020209@linux.vnet.ibm.com>

On Fri, 2016-06-03 at 12:10 +0800, xinhui wrote:
> On 2016年06月03日 09:32, Benjamin Herrenschmidt wrote:
> > On Fri, 2016-06-03 at 11:32 +1000, Benjamin Herrenschmidt wrote:
> >> On Thu, 2016-06-02 at 17:22 +0800, Pan Xinhui wrote:
> >>>
> >>> Base code to enable qspinlock on powerpc. this patch add some
> >>> #ifdef
> >>> here and there. Although there is no paravirt related code, we
> can
> >>> successfully build a qspinlock kernel after apply this patch.
> >> This is missing the IO_SYNC stuff ... It means we'll fail to do a
> >> full
> >> sync to order vs MMIOs.
> >>
> >> You need to add that back in the unlock path.
> >
> > Well, and in the lock path as well...
> >
> Oh, yes. I missed IO_SYNC stuff.
> 
> thank you, Ben :)

Ok couple of other things that would be nice from my perspective (and
Michael's) if you can produce them:

 - Some benchmarks of the qspinlock alone, without the PV stuff,
   so we understand how much of the overhead is inherent to the
   qspinlock and how much is introduced by the PV bits.

 - For the above, can you show (or describe) where the qspinlock
   improves things compared to our current locks. While there's
   theory and to some extent practice on x86, it would be nice to
   validate the effects on POWER.

 - Comparative benchmark with the PV stuff in on a bare metal system
   to understand the overhead there.

 - Comparative benchmark with the PV stuff under pHyp and KVM

Spinlocks are fiddly and a critical piece of infrastructure, it's
important we fully understand the performance implications before we
decide to switch to a new model.

Cheers,
Ben.

  reply	other threads:[~2016-06-03  4:35 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-02  9:22 [PATCH v5 0/6] powerPC/pSeries use pv-qpsinlock as the default spinlock implemention Pan Xinhui
2016-06-02  9:22 ` Pan Xinhui
2016-06-02  9:22 ` [PATCH v5 1/6] qspinlock: powerpc support qspinlock Pan Xinhui
2016-06-03  1:32   ` Benjamin Herrenschmidt
2016-06-03  1:32     ` Benjamin Herrenschmidt
2016-06-03  4:10       ` xinhui
2016-06-03  4:33         ` Benjamin Herrenschmidt [this message]
2016-06-03  7:02           ` xinhui
2016-06-06 15:59           ` Peter Zijlstra
2016-06-06 21:41             ` Benjamin Herrenschmidt
2016-06-21 12:35               ` xinhui
2016-06-02  9:22 ` [PATCH v5 2/6] powerpc: pseries/Kconfig: Add qspinlock build config Pan Xinhui
2016-06-02  9:22 ` [PATCH v5 3/6] powerpc: lib/locks.c: Add cpu yield/wake helper function Pan Xinhui
2016-06-02  9:22 ` [PATCH v5 4/6] pv-qspinlock: powerpc support pv-qspinlock Pan Xinhui
2016-06-02  9:22 ` [PATCH v5 5/6] pv-qspinlock: use cmpxchg_release in __pv_queued_spin_unlock Pan Xinhui
2016-06-02  9:22 ` [PATCH v5 6/6] powerpc: pseries: Add pv-qspinlock build config/make Pan Xinhui
2016-06-02  9:26 [PATCH v5 0/6] powerPC/pSeries use pv-qpsinlock as the default spinlock implemention Pan Xinhui
2016-06-02  9:26 ` [PATCH v5 1/6] qspinlock: powerpc support qspinlock Pan Xinhui

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=1464928427.26773.26.camel@kernel.crashing.org \
    --to=benh@kernel.crashing.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mingo@redhat.com \
    --cc=mpe@ellerman.id.au \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=paulus@samba.org \
    --cc=peterz@infradead.org \
    --cc=virtualization@lists.linux-foundation.org \
    --cc=waiman.long@hpe.com \
    --cc=xinhui.pan@linux.vnet.ibm.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).