Re: [PATCH RFC V10 15/18] kvm : Paravirtual ticketlocks support for linux guests running on KVM hypervisor

[Gleb Natapov <gleb@redhat.com>]

On Wed, Jul 17, 2013 at 03:35:37PM +0530, Raghavendra K T wrote:
> >>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;
> 
NMI can happen after the if() but before halt and the same situation
we are trying to prevent with IRQs will occur. But if NMI handler do not
take locks we shouldn't worry.

--
			Gleb.