Re: [PATCH] KVM: Remove redundant smp_mb() in the kvm_mmu_commit_zap_page()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 03/11/2016 01:07 AM, Paolo Bonzini wrote:
On 09/03/2016 08:18, Lan Tianyu wrote:
How about the following comments.

Log for kvm_mmu_commit_zap_page()
	/*
	 * We need to make sure everyone sees our modifications to
	 * the page tables and see changes to vcpu->mode here.

Please mention that this pairs with vcpu_enter_guest and
walk_shadow_page_lockless_begin/end.

The
	 * barrier in the kvm_flush_remote_tlbs() helps us to achieve
	 * these. Otherwise, wait for all vcpus to exit guest mode
	 * and/or lockless shadow page table walks.
	 */
	kvm_flush_remote_tlbs(kvm);

The rest of the comment is okay, but please replace "Otherwise" with "In
addition, we need to".

Log for kvm_flush_remote_tlbs()
	/*
	 * We want to publish modifications to the page tables before
	 * reading mode. Pairs with a memory barrier in arch-specific
	 * code.
	 * - x86: smp_mb__after_srcu_read_unlock in vcpu_enter_guest.

... and smp_mb in walk_shadow_page_lockless_begin/end.

	 * - powerpc: smp_mb in kvmppc_prepare_to_enter.
	 */
  	 smp_mb__before_atomic();

The comment looks good, but the smp_mb__before_atomic() is not needed.
As mentioned in the reply to Guangrong, only a smp_load_acquire is required.

So the comment should say something like "There is already an smp_mb()
before kvm_make_all_cpus_request reads vcpu->mode.  We reuse that
barrier here.".

On top of this there is:

- the change to paging_tmpl.h that Guangrong posted, adding smp_wmb()
before each increment of vcpu->kvm->tlbs_dirty

Yes, please make it as a separated patch.


- the change to smp_mb__after_atomic() in kvm_make_all_cpus_request

- if you want :) you can also replace the store+mb in
walk_shadow_page_lockless_begin with smp_store_mb, and the mb+store in
walk_shadow_page_lockless_end with smp_store_release.

These changes are good to me.

TianYu, please CC me when you post the new version out.

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux