Re: [PATCH] kvm: fix waitqueue_active without memory barrier in virt/kvm/async_pf.c

2015-10-09 Thread Kosuke Tatsukawa
Paolo Bonzini wrote: > On 09/10/2015 11:04, Kosuke Tatsukawa wrote: >> smp_store_mb() called from set_current_state(), which is called from >> prepare_to_wait() should prevent reordering such as below from >> happening. wait_event*() also calls set_current_state() inside. > > Ah, I missed that set

Re: [PATCH] kvm: fix waitqueue_active without memory barrier in virt/kvm/async_pf.c

2015-10-09 Thread Paolo Bonzini
On 09/10/2015 11:04, Kosuke Tatsukawa wrote: > smp_store_mb() called from set_current_state(), which is called from > prepare_to_wait() should prevent reordering such as below from > happening. wait_event*() also calls set_current_state() inside. Ah, I missed that set_current_state has a memory

Re: [PATCH] kvm: fix waitqueue_active without memory barrier in virt/kvm/async_pf.c

2015-10-09 Thread Paolo Bonzini
On 09/10/2015 10:50, Peter Zijlstra wrote: > Not having actually read or thought about the issue at hand, its > perfectly valid to pair an smp_mb() with either spin_lock() or > spin_unlock(). > > IOW. MB <-> {ACQUIRE, RELEASE} is a valid pairing. In this case it's an smp_mb() (store-load barrie

Re: [PATCH] kvm: fix waitqueue_active without memory barrier in virt/kvm/async_pf.c

2015-10-09 Thread Kosuke Tatsukawa
Paolo Bonzini wrote: > On 09/10/2015 02:35, Kosuke Tatsukawa wrote: >> async_pf_executekvm_vcpu_block >> >> spin_lock(&vcpu->async_pf.lock); >> if (waitqueue_active(&vcpu->wq)) >> /* The CPU might r

Re: [PATCH] kvm: fix waitqueue_active without memory barrier in virt/kvm/async_pf.c

2015-10-09 Thread Peter Zijlstra
On Fri, Oct 09, 2015 at 10:45:32AM +0200, Paolo Bonzini wrote: > So you need another smp_mb() after prepare_to_wait(). I'm not sure > if it's needed also for your original tty report, but I think it is > for https://lkml.org/lkml/2015/10/8/989 ("mei: fix waitqueue_active > without memory barrier i

Re: [PATCH] kvm: fix waitqueue_active without memory barrier in virt/kvm/async_pf.c

2015-10-09 Thread Paolo Bonzini
On 09/10/2015 02:35, Kosuke Tatsukawa wrote: > async_pf_executekvm_vcpu_block > > spin_lock(&vcpu->async_pf.lock); > if (waitqueue_active(&vcpu->wq)) > /* The CPU might reorder the test for >t

[PATCH] kvm: fix waitqueue_active without memory barrier in virt/kvm/async_pf.c

2015-10-08 Thread Kosuke Tatsukawa
async_pf_execute() seems to be missing a memory barrier which might cause the waker to not notice the waiter and miss sending a wake_up as in the following figure. async_pf_executekvm_vcpu_block sp