On 07/17/2013 03:35 PM, Raghavendra K T wrote:
On 07/17/2013 03:04 PM, Gleb Natapov wrote:
On Wed, Jul 17, 2013 at 12:12:35AM +0530, Raghavendra K T wrote:
I do not think it is very rare to get interrupt between
local_irq_restore() and halt() under load since any interrupt that
occurs between local_irq_save() and local_irq_restore() will be
delivered
immediately after local_irq_restore(). Of course the chance of no
other
random interrupt waking lock waiter is very low, but waiter can sleep
for much longer then needed and this will be noticeable in
performance.
Yes, I meant the entire thing. I did infact turned WARN on
w->lock==null before halt() [ though we can potentially have irq right
after that ], but did not hit so far.
Depends on your workload of course. To hit that you not only need to get
interrupt in there, but the interrupt handler needs to take contended
spinlock.
Yes. Agree.
BTW can NMI handler take spinlocks? If it can what happens if NMI is
delivered in a section protected by
local_irq_save()/local_irq_restore()?
Had another idea if NMI, halts are causing problem until I saw
PeterZ's reply similar to V2 of pvspinlock posted here:
https://lkml.org/lkml/2011/10/23/211
Instead of halt we started with a sleep hypercall in those
versions. Changed to halt() once Avi suggested to reuse existing
sleep.
If we use older hypercall with few changes like below:
kvm_pv_wait_for_kick_op(flags, vcpu, w->lock )
{
// a0 reserved for flags
if (!w->lock)
return;
DEFINE_WAIT
...
end_wait
}
How would this help if NMI takes lock in critical section. The thing
that may happen is that lock_waiting->want may have NMI lock value, but
lock_waiting->lock will point to non NMI lock. Setting of want and lock
have to be atomic.
True. so we are here
non NMI lock(a)
w->lock = NULL;
smp_wmb();
w->want = want;
NMI
<---------------------
NMI lock(b)
w->lock = NULL;
smp_wmb();
w->want = want;
smp_wmb();
w->lock = lock;
---------------------->
smp_wmb();
w->lock = lock;
so how about fixing like this?
again:
w->lock = NULL;
smp_wmb();
w->want = want;
smp_wmb();
w->lock = lock;
if (!lock || w->want != want) goto again;
Sorry,
I meant if (!w->lock || w->want !=want) here
[...]
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
