From: Srivatsa Vaddagiri <vatsa@linux.vnet.ibm.com>
To: Jeremy Fitzhardinge <jeremy@goop.org>
Cc: avi@redhat.com, Marcelo Tosatti <mtosatti@redhat.com>,
Gleb Natapov <gleb@redhat.com>,
linux-kernel@vger.kernel.org, npiggin@suse.de,
kvm@vger.kernel.org, bharata@in.ibm.com,
Balbir Singh <balbir@in.ibm.com>,
Jan Beulich <JBeulich@novell.com>
Subject: Re: [PATCH RFC 0/4] Paravirt-spinlock implementation for KVM guests (Version 0)
Date: Wed, 28 Jul 2010 20:17:04 +0530 [thread overview]
Message-ID: <20100728144704.GA27739@linux.vnet.ibm.com> (raw)
In-Reply-To: <4C4DC382.5010004@goop.org>
On Mon, Jul 26, 2010 at 10:18:58AM -0700, Jeremy Fitzhardinge wrote:
> >I tried to refactor Xen's spinlock
> >implementation to make it common for both Xen and KVM - but found that
> >few differences between Xen and KVM (Xen has the ability to block on a
> >particular event/irq for example) _and_ the fact that the guest kernel
> >can be compiled to support both Xen and KVM hypervisors (CONFIG_XEN and
> >CONFIG_KVM_GUEST can both be turned on) makes the "common" code a eye-sore.
> >There will have to be:
> >
> > if (xen) {
> > ...
> > } else if (kvm) {
> > ..
> > }
> >
> >or possibly:
> >
> > alternative(NOP, some_xen_specific_call, ....)
> >
> >type of code in the common implementation.
>
> No, that doesn't look like a good approach. It suggests the
> apparently commonality isn't really there.
>
> >For the time-being, I have made this KVM-specific only. At somepoint in future,
> >I hope this can be made common between Xen/KVM.
>
> Did you see the patch series I posted a couple of weeks ago to
> revamp pv spinlocks? Specifically, I dropped the notion of pv
> spinlocks in which the entire spinlock implementation is replaced,
> and added pv ticketlocks where the ticketlock algorithm is always
> used for the fastpath, but it calls out to pvop calls for the
> slowpath (a long spin, or unlocking a lock with waiters). It
> significantly reduces the amount of hypervisor-specific code.
Hmmm interesting - I will go thr' it in detail.
> You can see the current patches in
>
> git://git.kernel.org/pub/scm/linux/kernel/git/jeremy/xen.git
> xen/pvticketlock-git
[snip]
> That's not actually the real problem. It's *a* problem, but
> insignificant compared to the ticketlock-specific "next-in-line vcpu
> scheduler bunfight" problem - lock holder preemption is a misnomer.
> Fortunately the solutions to both are (nearly) the same.
>
> See Thomas Friebel's talk "Prevent Guests from Spinning Around
> (http://www.xen.org/files/xensummitboston08/LHP.pdf).
Yes I had seen Thomas's slides reporting huge degradation in performance with
tick spinlock.
> >b. Avoid preempting a lock-holder while its holding a (spin-) lock.
> >
> > In this scheme, guest OS can hint (set some flag in memory shared with
> > hypervisor) whenever its holding a lock and hypervisor could defer preempting
> > the guest vcpu when its holding a lock. With this scheme, we should never
> > have a lock-acquiring vcpu spin on a preempted vcpu to release its lock. If
> > ever it spins, its because somebody *currently running* is holding the lock -
> > and hence it won't have to spin-wait too long. IOW we are pro-actively
> > trying to prevent the LHP problem from occuring in the first place. This
> > should improve job turnaround time for some workloads. [1] has some
> > results based on this approach.
>
> This doesn't actually help the problem mentioned above, because it's
> not a problem with the lock holder getting preempted, but what
> happens once the lock has been released.
Good point. I agree that the latter problem needs more attention, given a
ticket-type implementation of spinlocks. Have you considered possible solutions
for unmodified guests, which have similar ticket-type lock implementations?
Not sure if that's important enough to investigate solutions like gang
scheduling ..
- vatsa
next prev parent reply other threads:[~2010-07-28 14:48 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-07-26 6:11 [PATCH RFC 0/4] Paravirt-spinlock implementation for KVM guests (Version 0) Srivatsa Vaddagiri
2010-07-26 6:13 ` [PATCH RFC 1/4] Debugfs support for reading an array of u32-type integers Srivatsa Vaddagiri
2010-07-26 6:14 ` [PATCH RFC 2/4] Add yield hypercall for KVM guests Srivatsa Vaddagiri
2010-07-26 17:19 ` Jeremy Fitzhardinge
2010-07-28 14:55 ` Srivatsa Vaddagiri
2010-08-02 8:40 ` Avi Kivity
2010-08-03 5:16 ` Srivatsa Vaddagiri
2010-08-03 5:33 ` Srivatsa Vaddagiri
2010-08-02 8:32 ` Avi Kivity
2010-08-02 14:42 ` Ryan Harper
2010-08-02 14:50 ` Avi Kivity
2010-08-02 15:08 ` Jeremy Fitzhardinge
2010-07-26 6:15 ` [PATCH RFC 3/4] Paravirtualized spinlock implementation " Srivatsa Vaddagiri
2010-08-02 8:48 ` Avi Kivity
2010-08-02 15:20 ` Jeremy Fitzhardinge
2010-08-03 6:59 ` Avi Kivity
2010-08-03 17:47 ` Jeremy Fitzhardinge
2010-08-02 8:53 ` Avi Kivity
2010-07-26 6:16 ` [PATCH RFC 4/4] Add yield hypercall support in Qemu Srivatsa Vaddagiri
2010-07-26 17:18 ` [PATCH RFC 0/4] Paravirt-spinlock implementation for KVM guests (Version 0) Jeremy Fitzhardinge
2010-07-28 14:47 ` Srivatsa Vaddagiri [this message]
2010-07-28 22:10 ` Konrad Rzeszutek Wilk
2010-07-28 22:42 ` Konrad Rzeszutek Wilk
2010-08-02 8:50 ` Avi Kivity
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=20100728144704.GA27739@linux.vnet.ibm.com \
--to=vatsa@linux.vnet.ibm.com \
--cc=JBeulich@novell.com \
--cc=avi@redhat.com \
--cc=balbir@in.ibm.com \
--cc=bharata@in.ibm.com \
--cc=gleb@redhat.com \
--cc=jeremy@goop.org \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mtosatti@redhat.com \
--cc=npiggin@suse.de \
/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