| Message ID | 20130509154433.d8b62a0f.yoshikawa_takuya_b1@lab.ntt.co.jp (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 05/09/2013 02:44 PM, Takuya Yoshikawa wrote: > Rather than clearing the ACC_WRITE_MASK bit of pte_access in the > "if (mmu_need_write_protect())" block not to call mark_page_dirty() in > the following if statement, simply moving the call into the appropriate > else block is better. > > Signed-off-by: Takuya Yoshikawa <yoshikawa_takuya_b1@lab.ntt.co.jp> > --- > arch/x86/kvm/mmu.c | 7 ++----- > 1 files changed, 2 insertions(+), 5 deletions(-) > > diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c > index 004cc87..08119a8 100644 > --- a/arch/x86/kvm/mmu.c > +++ b/arch/x86/kvm/mmu.c > @@ -2386,14 +2386,11 @@ static int set_spte(struct kvm_vcpu *vcpu, u64 *sptep, > pgprintk("%s: found shadow page for %llx, marking ro\n", > __func__, gfn); > ret = 1; > - pte_access &= ~ACC_WRITE_MASK; > spte &= ~(PT_WRITABLE_MASK | SPTE_MMU_WRITEABLE); > - } > + } else > + mark_page_dirty(vcpu->kvm, gfn); > } > > - if (pte_access & ACC_WRITE_MASK) > - mark_page_dirty(vcpu->kvm, gfn); > - > set_pte: > if (mmu_spte_update(sptep, spte)) > kvm_flush_remote_tlbs(vcpu->kvm); That function is really magic, and this change do no really help it. I had several patches posted some months ago to make these kind of code better understanding, but i am too tired to update them. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, May 09, 2013 at 06:16:55PM +0800, Xiao Guangrong wrote: > On 05/09/2013 02:44 PM, Takuya Yoshikawa wrote: > > Rather than clearing the ACC_WRITE_MASK bit of pte_access in the > > "if (mmu_need_write_protect())" block not to call mark_page_dirty() in > > the following if statement, simply moving the call into the appropriate > > else block is better. > > > > Signed-off-by: Takuya Yoshikawa <yoshikawa_takuya_b1@lab.ntt.co.jp> > > --- > > arch/x86/kvm/mmu.c | 7 ++----- > > 1 files changed, 2 insertions(+), 5 deletions(-) > > > > diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c > > index 004cc87..08119a8 100644 > > --- a/arch/x86/kvm/mmu.c > > +++ b/arch/x86/kvm/mmu.c > > @@ -2386,14 +2386,11 @@ static int set_spte(struct kvm_vcpu *vcpu, u64 *sptep, > > pgprintk("%s: found shadow page for %llx, marking ro\n", > > __func__, gfn); > > ret = 1; > > - pte_access &= ~ACC_WRITE_MASK; > > spte &= ~(PT_WRITABLE_MASK | SPTE_MMU_WRITEABLE); > > - } > > + } else > > + mark_page_dirty(vcpu->kvm, gfn); > > } > > > > - if (pte_access & ACC_WRITE_MASK) > > - mark_page_dirty(vcpu->kvm, gfn); > > - > > set_pte: > > if (mmu_spte_update(sptep, spte)) > > kvm_flush_remote_tlbs(vcpu->kvm); > > That function is really magic, and this change do no really help it. I had several > patches posted some months ago to make these kind of code better understanding, but > i am too tired to update them. Can you point me to them? Your work is really appreciated, I am sorry you feel this way. -- Gleb. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 05/09/2013 07:18 PM, Gleb Natapov wrote: > On Thu, May 09, 2013 at 06:16:55PM +0800, Xiao Guangrong wrote: >> On 05/09/2013 02:44 PM, Takuya Yoshikawa wrote: >>> Rather than clearing the ACC_WRITE_MASK bit of pte_access in the >>> "if (mmu_need_write_protect())" block not to call mark_page_dirty() in >>> the following if statement, simply moving the call into the appropriate >>> else block is better. >>> >>> Signed-off-by: Takuya Yoshikawa <yoshikawa_takuya_b1@lab.ntt.co.jp> >>> --- >>> arch/x86/kvm/mmu.c | 7 ++----- >>> 1 files changed, 2 insertions(+), 5 deletions(-) >>> >>> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c >>> index 004cc87..08119a8 100644 >>> --- a/arch/x86/kvm/mmu.c >>> +++ b/arch/x86/kvm/mmu.c >>> @@ -2386,14 +2386,11 @@ static int set_spte(struct kvm_vcpu *vcpu, u64 *sptep, >>> pgprintk("%s: found shadow page for %llx, marking ro\n", >>> __func__, gfn); >>> ret = 1; >>> - pte_access &= ~ACC_WRITE_MASK; >>> spte &= ~(PT_WRITABLE_MASK | SPTE_MMU_WRITEABLE); >>> - } >>> + } else >>> + mark_page_dirty(vcpu->kvm, gfn); >>> } >>> >>> - if (pte_access & ACC_WRITE_MASK) >>> - mark_page_dirty(vcpu->kvm, gfn); >>> - >>> set_pte: >>> if (mmu_spte_update(sptep, spte)) >>> kvm_flush_remote_tlbs(vcpu->kvm); >> >> That function is really magic, and this change do no really help it. I had several >> patches posted some months ago to make these kind of code better understanding, but >> i am too tired to update them. > Can you point me to them? Your work is really appreciated, I am sorry There are two patches about this set_spte cleanups: https://lkml.org/lkml/2013/1/23/125 https://lkml.org/lkml/2013/1/23/138 > you feel this way. It is not your fault, it is mine. Will update these patches when i finish the zap-all-page and zap-mmio-sp related things. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 09 May 2013 20:16:18 +0800 Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com> wrote: > >> That function is really magic, and this change do no really help it. I had several > >> patches posted some months ago to make these kind of code better understanding, but > >> i am too tired to update them. Thank you for reviewing my patches. But please don't express your personal frustrations on your pending work while reviewing patches from others. "too tired" made me think you would not send updated patches... If you want to explain possible conflicts with your patches, I can understand. Please say so in that casse. Takuya > > Can you point me to them? Your work is really appreciated, I am sorry > > There are two patches about this set_spte cleanups: > > https://lkml.org/lkml/2013/1/23/125 > https://lkml.org/lkml/2013/1/23/138 > > > you feel this way. > > It is not your fault, it is mine. > > Will update these patches when i finish the zap-all-page and zap-mmio-sp > related things. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c index 004cc87..08119a8 100644 --- a/arch/x86/kvm/mmu.c +++ b/arch/x86/kvm/mmu.c @@ -2386,14 +2386,11 @@ static int set_spte(struct kvm_vcpu *vcpu, u64 *sptep, pgprintk("%s: found shadow page for %llx, marking ro\n", __func__, gfn); ret = 1; - pte_access &= ~ACC_WRITE_MASK; spte &= ~(PT_WRITABLE_MASK | SPTE_MMU_WRITEABLE); - } + } else + mark_page_dirty(vcpu->kvm, gfn); } - if (pte_access & ACC_WRITE_MASK) - mark_page_dirty(vcpu->kvm, gfn); - set_pte: if (mmu_spte_update(sptep, spte)) kvm_flush_remote_tlbs(vcpu->kvm);
Rather than clearing the ACC_WRITE_MASK bit of pte_access in the "if (mmu_need_write_protect())" block not to call mark_page_dirty() in the following if statement, simply moving the call into the appropriate else block is better. Signed-off-by: Takuya Yoshikawa <yoshikawa_takuya_b1@lab.ntt.co.jp> --- arch/x86/kvm/mmu.c | 7 ++----- 1 files changed, 2 insertions(+), 5 deletions(-)