Hey,
Linux v3.12 (which is now in merge window phase) has the code for
making the spinlock use ticketlocks for everything. The relevant
git commit is commit 816434ec4a674fcdb3c2221a6dffdc8f34020550
Merge: f357a82 36bd621
Author: Linus Torvalds <torvalds@linux-foundation.org>
Date: Wed Sep 4 11:55:10 2013 -0700
Merge branch ''x86-spinlocks-for-linus'' of
git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
Pull x86 spinlock changes from Ingo Molnar:
"The biggest change here are paravirtualized ticket spinlocks (PV
spinlocks), which bring a nice speedup on various benchmarks.
Majority of that work was done by Jeremy Fitzhardinge and I would like
to thank him for doing this work. It had taken a year (or more) to actually
get it upstreamed. Now that it is, it means that the PV ticketlock
implementation is used for:
- Xen PV guests (it used to have a PV bytelock locking mechanism)
- KVM Linux guests.
Baremetal will still use ticketlocks.
What is missing is that the Xen PVHVM guests don''t use it. They
still end up using ticketlock, but not the PV part. The reason is b/c
yours didn''t get all of the bugs ironed out until today :-)
I have a branch in
git://git.kernel.org/pub/scm/linux/kernel/git/konrad/xen.git
devel/pvticketlock.v7
which fixes the bugs so that under Xen PVHVM the PV ticketlocks are also
utilized. There is a cruft of DEBUG patches that one can ignore if
CONFIG_XEN_DEBUG_SPIN is not set. Otherwise they cause the slowpath to be
executed most of the time to stress test the kicker/waiter logic.
I am planning on doing some more testing next week and making sure it all works
nicely.
We could merge it in v3.12 - as most of the patches are bug-fixes
(and two reverts) - see below. However, I am going to be -EBUSY for most
of the v3.12 cycle so won''t be able to help much if mysterious bugs
show up.
P.S.
The DEBUG code adds new hypercalls. The Xen parts are in
git://xenbits.xen.org/people/konradwilk/xen.git devel/pvticketlock.v7
The xenanalyze patch:
http://darnok.org/xen/xenanalyze.devel-pvticketlock.v7.patch
P.S.S.
The relavant patches that make PVHVM work on top of the above mentioned
git commit are:
218b57 xen/smp: Update pv_lock_ops before alternative code kicks in (PVHVM).
eb2a6eb Revert "xen: disable PV spinlocks on HVM"
15d09db Revert "xen/spinlock: Disable IRQ spinlock (PV) allocation on
PVHVM"
ee4c3a1 xen/smp: Update pv_lock_ops before alternative code kicks in.
1253858 xen/spinlock: Fix locking path engaging too soon.
On Fri, 2013-09-06 at 16:11 -0400, Konrad Rzeszutek Wilk wrote:> Hey, > > Linux v3.12 (which is now in merge window phase) has the code for > making the spinlock use ticketlocks for everything.Congrats!> Majority of that work was done by Jeremy Fitzhardinge and I would like > to thank him for doing this work. It had taken a year (or more) to actuallyWay more that a year, wasn''t it? This stuff has been floating around for 3 or 4 years or longer IIRC. BTW Attilio Rao did some upstreaming work and a bunch of measurement stuff too. Ian.
On Mon, Sep 09, 2013 at 11:39:52AM +0100, Ian Campbell wrote:> On Fri, 2013-09-06 at 16:11 -0400, Konrad Rzeszutek Wilk wrote: > > Hey, > > > > Linux v3.12 (which is now in merge window phase) has the code for > > making the spinlock use ticketlocks for everything. > > Congrats!Thank you. Albeit most of the credit goes to folks you and me mentioned and as well Raghavendra K T for tirelessly picking it up and upstreaming it.> > > Majority of that work was done by Jeremy Fitzhardinge and I would like > > to thank him for doing this work. It had taken a year (or more) to actually > > Way more that a year, wasn''t it? This stuff has been floating around for > 3 or 4 years or longer IIRC.Wow. Time does not exist for me. Everything that has not yet been done in my mind is "last year" :-)> > BTW Attilio Rao did some upstreaming work and a bunch of measurement > stuff too.And a nice Wiki entry: http://wiki.xen.org/wiki/Benchmarking_the_new_PV_ticketlock_implementation and an awesome blog entry: http://blog.xen.org/index.php/2012/05/11/benchmarking-the-new-pv-ticketlock-implementation/> > Ian. >
On lun, 2013-09-09 at 10:03 -0400, Konrad Rzeszutek Wilk wrote:> On Mon, Sep 09, 2013 at 11:39:52AM +0100, Ian Campbell wrote: > > > Congrats! >Huge to everyone that has been involved in it congrats from me too! :-)> Thank you. Albeit most of the credit goes to folks you and me mentioned > and as well Raghavendra K T for tirelessly picking it up and > upstreaming it. > > > > > > Majority of that work was done by Jeremy Fitzhardinge and I would like > > > to thank him for doing this work. It had taken a year (or more) to actually > > > > > > > BTW Attilio Rao did some upstreaming work and a bunch of measurement > > stuff too. > > And a nice Wiki entry: > > http://wiki.xen.org/wiki/Benchmarking_the_new_PV_ticketlock_implementation > > and an awesome blog entry: > http://blog.xen.org/index.php/2012/05/11/benchmarking-the-new-pv-ticketlock-implementation/ >Indeed. Still, the fact that the code finally hit upstream, certainly calls for another blog post! :-) For example, it would be nice to make an announcement that this is now upstreamed and also, if I understood your patches correctly, available for all the virtualization modes we support! Anyone up for it? Dario -- <<This happens because I choose it to happen!>> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Hi Konrad, Just to confirm: - PV ticketlocks is on by default and doesn''t neet a config option, to disable it at boot time it''s possible to use "xen_nopvspin" as a kernel commandline parameter - CONFIG_PARAVIRT_SPINLOCKS should be off, since that would enable the old bytelock pv-spinlock implementation which could now probably be removed ? Have a nice vacation ! -- Sander
On Wed, Sep 11, 2013 at 08:25:01PM +0200, Sander Eikelenboom wrote:> Hi Konrad, > > Just to confirm: > > - PV ticketlocks is on by default and doesn''t neet a config option, to disable it at boot time it''s possible to use "xen_nopvspin" as a kernel commandline parameterYes.> - CONFIG_PARAVIRT_SPINLOCKS should be off, since that would enable the old bytelock pv-spinlock implementation which could now probably be removed ?You still need CONFIG_PARAVIRT_SPINLOCKS=y to take advantage of it. The bytelock pvspinlock is gone. If you compile without CONFIG_PARAVIRT_SPINLOCKS then you will only use ticketlocks for all types of guests and baremtal. No ''PV'' variant. The CONFIG_PARAVIRT_SPINLOCKS=y enables the PV ticketlock for all guests.> > Have a nice vacation ! > > -- > Sander >
On Tue, Sep 10, 2013 at 07:41:04PM +0200, Dario Faggioli wrote:> On lun, 2013-09-09 at 10:03 -0400, Konrad Rzeszutek Wilk wrote: > > On Mon, Sep 09, 2013 at 11:39:52AM +0100, Ian Campbell wrote: > > > > > Congrats! > > > Huge to everyone that has been involved in it congrats from me too! :-) > > > Thank you. Albeit most of the credit goes to folks you and me mentioned > > and as well Raghavendra K T for tirelessly picking it up and > > upstreaming it. > > > > > > > > > Majority of that work was done by Jeremy Fitzhardinge and I would like > > > > to thank him for doing this work. It had taken a year (or more) to actually > > > > > > > > > > > BTW Attilio Rao did some upstreaming work and a bunch of measurement > > > stuff too. > > > > And a nice Wiki entry: > > > > http://wiki.xen.org/wiki/Benchmarking_the_new_PV_ticketlock_implementation > > > > and an awesome blog entry: > > http://blog.xen.org/index.php/2012/05/11/benchmarking-the-new-pv-ticketlock-implementation/ > > > Indeed. Still, the fact that the code finally hit upstream, certainly > calls for another blog post! :-)Ha!> > For example, it would be nice to make an announcement that this is now > upstreamed and also, if I understood your patches correctly, available > for all the virtualization modes we support!We still got two months to do it - as it is just now in v3.12-rc0 stage.> > Anyone up for it? > > Dario > > -- > <<This happens because I choose it to happen!>> (Raistlin Majere) > ----------------------------------------------------------------- > Dario Faggioli, Ph.D, http://about.me/dario.faggioli > Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) >
Wednesday, September 11, 2013, 8:41:29 PM, you wrote:> On Wed, Sep 11, 2013 at 08:25:01PM +0200, Sander Eikelenboom wrote: >> Hi Konrad, >> >> Just to confirm: >> >> - PV ticketlocks is on by default and doesn''t neet a config option, to disable it at boot time it''s possible to use "xen_nopvspin" as a kernel commandline parameter> Yes. >> - CONFIG_PARAVIRT_SPINLOCKS should be off, since that would enable the old bytelock pv-spinlock implementation which could now probably be removed ?> You still need CONFIG_PARAVIRT_SPINLOCKS=y to take advantage of it.> The bytelock pvspinlock is gone. If you compile without CONFIG_PARAVIRT_SPINLOCKS > then you will only use ticketlocks for all types of guests and baremtal. No > ''PV'' variant.> The CONFIG_PARAVIRT_SPINLOCKS=y enables the PV ticketlock for all guests.Ok so the KConfig (help) entry for CONFIG_PARAVIRT_SPINLOCKS should perhaps also be updated ? Since it for example still contains the "Unfortunately the downside is an up to 5% performance hit on native kernels, with various workloads." which afaik was one the things the PV ticketlock migitated, so it would be more feasible for distributions to have this option on for all general distro kernels ? -- Sander>> >> Have a nice vacation ! >> >> -- >> Sander >>
On Wed, Sep 11, 2013 at 09:05:05PM +0200, Sander Eikelenboom wrote:> > Wednesday, September 11, 2013, 8:41:29 PM, you wrote: > > > On Wed, Sep 11, 2013 at 08:25:01PM +0200, Sander Eikelenboom wrote: > >> Hi Konrad, > >> > >> Just to confirm: > >> > >> - PV ticketlocks is on by default and doesn''t neet a config option, to disable it at boot time it''s possible to use "xen_nopvspin" as a kernel commandline parameter > > > Yes. > >> - CONFIG_PARAVIRT_SPINLOCKS should be off, since that would enable the old bytelock pv-spinlock implementation which could now probably be removed ? > > > You still need CONFIG_PARAVIRT_SPINLOCKS=y to take advantage of it. > > > The bytelock pvspinlock is gone. If you compile without CONFIG_PARAVIRT_SPINLOCKS > > then you will only use ticketlocks for all types of guests and baremtal. No > > ''PV'' variant. > > > The CONFIG_PARAVIRT_SPINLOCKS=y enables the PV ticketlock for all guests. > > Ok so the KConfig (help) entry for CONFIG_PARAVIRT_SPINLOCKS should perhaps also be updated ?Yes!> > Since it for example still contains the "Unfortunately the downside is an up to 5% > performance hit on native kernels, with various workloads." which afaik was one the things the PV ticketlock migitated, > so it would be more feasible for distributions to have this option on for all general distro kernels ?Yikes. That is true - needs to be updated as the results were the opposite: Results: ====== setup: 32 core machine with 32 vcpu KVM guest (HT off) with 8GB RAM base = 3.11-rc patched = base + pvspinlock V12 +-----------------+----------------+--------+ dbench (Throughput in MB/sec. Higher is better) +-----------------+----------------+--------+ | base (stdev %)|patched(stdev%) | %gain | +-----------------+----------------+--------+ | 15035.3 (0.3) |15150.0 (0.6) | 0.8 | | 1470.0 (2.2) | 1713.7 (1.9) | 16.6 | | 848.6 (4.3) | 967.8 (4.3) | 14.0 | | 652.9 (3.5) | 685.3 (3.7) | 5.0 | +-----------------+----------------+--------+ (I don''t have the baremetal one - but it was pretty much 0%). Please if you get a chance - please do send a patch to x86 folks altering this. Thanks!
On 09/09/2013 03:39 AM, Ian Campbell wrote:> On Fri, 2013-09-06 at 16:11 -0400, Konrad Rzeszutek Wilk wrote: >> Hey, >> >> Linux v3.12 (which is now in merge window phase) has the code for >> making the spinlock use ticketlocks for everything. > Congrats!Yeah! Lots of good work to turn my stuff into something submittable. I got stalled by the wall of perf testing.>> Majority of that work was done by Jeremy Fitzhardinge and I would like >> to thank him for doing this work. It had taken a year (or more) to actually > Way more that a year, wasn''t it? This stuff has been floating around for > 3 or 4 years or longer IIRC.Not quite - the timestamps in my git tree are mid 2010-2011. I did the first pv spinlocks at the Xen summit with the paper, which was about 2008? 9? J> > BTW Attilio Rao did some upstreaming work and a bunch of measurement > stuff too. > > Ian. >
On mer, 2013-09-11 at 15:03 -0400, Konrad Rzeszutek Wilk wrote:> On Tue, Sep 10, 2013 at 07:41:04PM +0200, Dario Faggioli wrote: > > Indeed. Still, the fact that the code finally hit upstream, certainly > > calls for another blog post! :-) > > Ha! > > > > For example, it would be nice to make an announcement that this is now > > upstreamed and also, if I understood your patches correctly, available > > for all the virtualization modes we support! > > We still got two months to do it - as it is just now in v3.12-rc0 > stage. >Ok... Wait for an e-mail from me in 1 month time, reminding you to start putting it together then! :-P :-P Regards, Dario -- <<This happens because I choose it to happen!>> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel