Re: [PATCH A] KVM: Simplify kvm->tlbs_dirty handling

[Paolo Bonzini <pbonzini@redhat.com>]

Il 19/02/2014 01:44, Takuya Yoshikawa ha scritto:
> (2014/02/18 18:07), Paolo Bonzini wrote:
>> Il 18/02/2014 09:22, Takuya Yoshikawa ha scritto:
>>> When this was introduced, kvm_flush_remote_tlbs() could be called
>>> without holding mmu_lock.  It is now acknowledged that the function
>>> must be called before releasing mmu_lock, and all callers have already
>>> been changed to do so.
>>>
>>> There is no need to use smp_mb() and cmpxchg() any more.
>>>
>>> Signed-off-by: Takuya Yoshikawa <yoshikawa_takuya_b1@lab.ntt.co.jp>
>>
>> I prefer this patch, and in fact we can make it even simpler:
>>
>> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
>> index ed1cc89..9816b68 100644
>> --- a/include/linux/kvm_host.h
>> +++ b/include/linux/kvm_host.h
>> @@ -401,7 +401,9 @@ struct kvm {
>>       unsigned long mmu_notifier_seq;
>>       long mmu_notifier_count;
>>   #endif
>> +    /* Protected by mmu_lock */
>>       bool tlbs_dirty;
>> +
>>       struct list_head devices;
>>   };
>>
>> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
>> index 51744da..f5668a4 100644
>> --- a/virt/kvm/kvm_main.c
>> +++ b/virt/kvm/kvm_main.c
>> @@ -188,12 +188,6 @@ void kvm_flush_remote_tlbs(struct kvm *kvm)
>>   {
>>       if (make_all_cpus_request(kvm, KVM_REQ_TLB_FLUSH))
>>           ++kvm->stat.remote_tlb_flush;
>> -    /*
>> -     * tlbs_dirty is used only for optimizing x86's shadow paging
>> code with
>> -     * mmu notifiers in mind, see the note on sync_page().  Since it is
>> -     * always protected with mmu_lock there, should
>> kvm_flush_remote_tlbs()
>> -     * be called before releasing mmu_lock, this is safe.
>> -     */
>>       kvm->tlbs_dirty = false;
>>   }
>>   EXPORT_SYMBOL_GPL(kvm_flush_remote_tlbs);
>>
>>
>> What do you think?
>
> I agree.
>
> But please check Xiao's comment and my reply to it.

Yes, I'll wait some time in case Xiao can get back to his patch set, and 
then make up my mind on what patch to apply.

In any case, if Xiao patches go in a change will be needed.  For patch A 
it would be reverted, for patch B it would edit the comment.  So the two 
patches aren't a lot different in this respect.

Paolo