Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1349957

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

From Paolo Bonzini <pbonzini@redhat.com>
Newsgroups linux.kernel
Subject Re: [PATCH] KVM: Remove redundant smp_mb() in the kvm_mmu_commit_zap_page()
Date 2016-03-04 09:50 +0100
Message-ID <r8TAT-44r-15@gated-at.bofh.it> (permalink)
References <r8Nc6-7Fq-9@gated-at.bofh.it> <r8Sls-38B-13@gated-at.bofh.it> <r8Sls-38B-11@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On 04/03/2016 08:12, Lan Tianyu wrote:
> > >  	/*
> > > -	 * wmb: make sure everyone sees our modifications to the page tables
> > > -	 * rmb: make sure we see changes to vcpu->mode
> > 
> > You want to leave the comment explaining the memory barriers and tell that
> > kvm_flush_remote_tlbs() contains the smp_mb().
> 
> That sounds more reasonable. Will update. Thanks.

In fact, the reason for kvm_flush_remote_tlbs()'s barrier is exactly
what was in this comment.  So you can:

1) add a comment to kvm_flush_remote_tlbs like:

	/*
	 * 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.
	 * - powerpc: smp_mb in kvmppc_prepare_to_enter.
	 */

2) add a comment to vcpu_enter_guest and kvmppc_prepare_to_enter, saying
that the memory barrier also orders the write to mode from any reads
to the page tables done while the VCPU is running.  In other words, on
entry a single memory barrier achieves two purposes (write ->mode before
reading requests, write ->mode before reading page tables).

The same should be true in kvm_flush_remote_tlbs().  So you may investigate
removing the barrier from kvm_flush_remote_tlbs, because
kvm_make_all_cpus_request already includes a memory barrier.  Like
Thomas suggested, leave a comment in kvm_flush_remote_tlbs(),
saying which memory barrier you are relying on and for what.

And finally, the memory barrier in kvm_make_all_cpus_request can become
smp_mb__after_atomic, which is free on x86.

Of course, all this should be done in at least three separate patches.

Thanks!

Paolo

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH] KVM: Remove redundant smp_mb() in the kvm_mmu_commit_zap_page() Lan Tianyu <tianyu.lan@intel.com> - 2016-03-04 03:00 +0100
  Re: [PATCH] KVM: Remove redundant smp_mb() in the kvm_mmu_commit_zap_page() Lan Tianyu <tianyu.lan@intel.com> - 2016-03-04 08:30 +0100
    Re: [PATCH] KVM: Remove redundant smp_mb() in the  kvm_mmu_commit_zap_page() Paolo Bonzini <pbonzini@redhat.com> - 2016-03-04 09:50 +0100
      Re: [PATCH] KVM: Remove redundant smp_mb() in the kvm_mmu_commit_zap_page() Lan Tianyu <tianyu.lan@intel.com> - 2016-03-08 10:00 +0100
        Re: [PATCH] KVM: Remove redundant smp_mb() in the  kvm_mmu_commit_zap_page() Paolo Bonzini <pbonzini@redhat.com> - 2016-03-08 16:30 +0100
          Re: [PATCH] KVM: Remove redundant smp_mb() in the kvm_mmu_commit_zap_page() Lan Tianyu <tianyu.lan@intel.com> - 2016-03-09 08:40 +0100
            Re: [PATCH] KVM: Remove redundant smp_mb() in the  kvm_mmu_commit_zap_page() Paolo Bonzini <pbonzini@redhat.com> - 2016-03-10 18:10 +0100
              Re: [PATCH] KVM: Remove redundant smp_mb() in the  kvm_mmu_commit_zap_page() Xiao Guangrong <guangrong.xiao@linux.intel.com> - 2016-03-10 19:20 +0100
          Re: [PATCH] KVM: Remove redundant smp_mb() in the  kvm_mmu_commit_zap_page() Xiao Guangrong <guangrong.xiao@linux.intel.com> - 2016-03-10 15:50 +0100
            Re: [PATCH] KVM: Remove redundant smp_mb() in the  kvm_mmu_commit_zap_page() Paolo Bonzini <pbonzini@redhat.com> - 2016-03-10 16:30 +0100
              Re: [PATCH] KVM: Remove redundant smp_mb() in the  kvm_mmu_commit_zap_page() Paolo Bonzini <pbonzini@redhat.com> - 2016-03-10 16:40 +0100
                Re: [PATCH] KVM: Remove redundant smp_mb() in the  kvm_mmu_commit_zap_page() Xiao Guangrong <guangrong.xiao@linux.intel.com> - 2016-03-10 16:50 +0100
                Re: [PATCH] KVM: Remove redundant smp_mb() in the  kvm_mmu_commit_zap_page() Paolo Bonzini <pbonzini@redhat.com> - 2016-03-10 17:10 +0100
                Re: [PATCH] KVM: Remove redundant smp_mb() in the  kvm_mmu_commit_zap_page() Xiao Guangrong <guangrong.xiao@linux.intel.com> - 2016-03-10 19:20 +0100
              Re: [PATCH] KVM: Remove redundant smp_mb() in the kvm_mmu_commit_zap_page() Lan Tianyu <tianyu.lan@intel.com> - 2016-03-11 02:40 +0100
  Re: [PATCH] KVM: Remove redundant smp_mb() in the  kvm_mmu_commit_zap_page() Thomas Gleixner <tglx@linutronix.de> - 2016-03-04 08:30 +0100
  Re: [PATCH] KVM: Remove redundant smp_mb() in the  kvm_mmu_commit_zap_page() Paolo Bonzini <pbonzini@redhat.com> - 2016-03-04 09:10 +0100
    Re: [PATCH] KVM: Remove redundant smp_mb() in the  kvm_mmu_commit_zap_page() Xiao Guangrong <guangrong.xiao@linux.intel.com> - 2016-03-04 14:50 +0100

csiph-web