All of lore.kernel.org
 help / color / mirror / Atom feed
From: Srivatsa Vaddagiri <vatsa@in.ibm.com>
To: Nick Piggin <npiggin@suse.de>
Cc: Avi Kivity <avi@redhat.com>, Andi Kleen <andi@firstfloor.org>,
	Gleb Natapov <gleb@redhat.com>,
	linux-kernel@vger.kernel.org, kvm@vger.kernel.org, hpa@zytor.com,
	mingo@elte.hu, tglx@linutronix.de, mtosatti@redhat.com
Subject: Re: [PATCH] use unfair spinlock when running on hypervisor.
Date: Thu, 3 Jun 2010 17:34:50 +0530	[thread overview]
Message-ID: <20100603120450.GH4035@linux.vnet.ibm.com> (raw)
In-Reply-To: <20100603103855.GG6822@laptop>

On Thu, Jun 03, 2010 at 08:38:55PM +1000, Nick Piggin wrote:
> > Guest side:
> > 
> > static inline void spin_lock(spinlock_t *lock)
> > {
> > 	raw_spin_lock(&lock->rlock);
> > +       __get_cpu_var(gh_vcpu_ptr)->defer_preempt++;
> > }
> > 
> > static inline void spin_unlock(spinlock_t *lock)
> > {
> > +	__get_cpu_var(gh_vcpu_ptr)->defer_preempt--;
> >         raw_spin_unlock(&lock->rlock);
> > }
> > 
> > [similar changes to other spinlock variants]
> 
> Great, this is a nice way to improve it.
> 
> You might want to consider playing with first taking a ticket, and
> then if we fail to acquire the lock immediately, then increment
> defer_preempt before we start spinning.
>
> The downside of this would be if we waste all our slice on spinning
> and then preempted in the critical section. But with ticket locks
> you can easily see how many entries in the queue in front of you.
> So you could experiment with starting to defer preempt when we
> notice we are getting toward the head of the queue.

Mm - my goal is to avoid long spin times in the first place (because the 
owning vcpu was descheduled at an unfortunate time i.e while it was holding a
lock). From that sense, I am targetting preemption-defer of lock *holder*
rather than of lock acquirer. So ideally whenever somebody tries to grab a lock,
it should be free most of the time, it can be held only if the owner is
currently running - which means we won't have to spin too long for the lock.

> Have you also looked at how s390 checks if the owning vcpu is running
> and if so it spins, if not yields to the hypervisor. Something like
> turning it into an adaptive lock. This could be applicable as well.

I don't think even s390 does adaptive spinlocks. Also afaik s390 zVM does gang
scheduling of vcpus, which reduces the severity of this problem very much -
essentially lock acquirer/holder are run simultaneously on different cpus all
the time. Gang scheduling is on my list of things to look at much later
(although I have been warned that its a scalablility nightmare!).

- vatsa

  reply	other threads:[~2010-06-03 12:05 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-06-01  9:35 [PATCH] use unfair spinlock when running on hypervisor Gleb Natapov
2010-06-01 15:53 ` Andi Kleen
2010-06-01 16:24   ` Gleb Natapov
2010-06-01 16:38     ` Andi Kleen
2010-06-01 16:52       ` Avi Kivity
2010-06-01 17:27         ` Andi Kleen
2010-06-02  2:51           ` Avi Kivity
2010-06-02  5:26             ` Srivatsa Vaddagiri
2010-06-02  8:50             ` Andi Kleen
2010-06-02  9:00               ` Avi Kivity
2010-06-03  4:20                 ` Srivatsa Vaddagiri
2010-06-03  4:51                   ` Eric Dumazet
2010-06-03  5:38                     ` Srivatsa Vaddagiri
2010-06-03  8:52                   ` Andi Kleen
2010-06-03  9:26                     ` Srivatsa Vaddagiri
2010-06-03 10:22                     ` Nick Piggin
2010-06-03 10:38                   ` Nick Piggin
2010-06-03 12:04                     ` Srivatsa Vaddagiri [this message]
2010-06-03 12:38                       ` Nick Piggin
2010-06-03 12:58                         ` Srivatsa Vaddagiri
2010-06-03 13:04                           ` Srivatsa Vaddagiri
2010-06-03 13:45                           ` Nick Piggin
2010-06-03 14:48                             ` Srivatsa Vaddagiri
2010-06-03 15:17                         ` Andi Kleen
2010-06-03 15:35                           ` Nick Piggin
2010-06-03 17:25                             ` Andi Kleen
2010-06-01 17:39         ` Valdis.Kletnieks
2010-06-02  2:46           ` Avi Kivity
2010-06-02  7:39           ` H. Peter Anvin
2010-06-01 17:54         ` john cooper
2010-06-01 19:36           ` Andi Kleen
2010-06-03 11:06             ` David Woodhouse
2010-06-03 15:15               ` Andi Kleen
2010-06-01 21:39         ` Eric Dumazet

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=20100603120450.GH4035@linux.vnet.ibm.com \
    --to=vatsa@in.ibm.com \
    --cc=andi@firstfloor.org \
    --cc=avi@redhat.com \
    --cc=gleb@redhat.com \
    --cc=hpa@zytor.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=mtosatti@redhat.com \
    --cc=npiggin@suse.de \
    --cc=tglx@linutronix.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 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.