On 07/17/2013 03:04 PM, Gleb Natapov wrote:Sorry,On Wed, Jul 17, 2013 at 12:12:35AM +0530, Raghavendra K T wrote:Depends on your workload of course. To hit that you not only need to getI 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.
interrupt in there, but the interrupt handler needs to take contended
spinlock.
Yes. Agree.
How would this help if NMI takes lock in critical section. The thing
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
}
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;