On 07/17/2013 06:15 PM, Gleb Natapov wrote:
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.
True, we can not fix that. I thought to fix the inconsistency of lock,want pair. But NMI could happen after the first OR condition also. /me thinks again But if NMI handler do not
take locks we shouldn't worry.
Okay. Thanks for the reviews. 'll spin the next version with all the suggested changes. -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
