On Mon, Jul 20, 2015 at 11:34:21AM -0700, Andres Lagar-Cavilla wrote: > On Sun, Jul 19, 2015 at 5:31 AM, Vladimir Davydov <vdavydov@xxxxxxxxxxxxx> [...] > > +static int kvm_mmu_notifier_clear_young(struct mmu_notifier *mn, > > + struct mm_struct *mm, > > + unsigned long start, > > + unsigned long end) > > +{ > > + struct kvm *kvm = mmu_notifier_to_kvm(mn); > > + int young, idx; > > + > > > If you need to cut out another version please add comments as to the two > issues raised: > - This doesn't proactively flush TLBs -- not obvious if it should. > - This adversely affects performance in Pre_haswell Intel EPT. Oops, I stopped reading your e-mail in reply to the previous version of this patch as soon as I saw the Reviewed-by tag, so I missed your request for the comment, sorry about that. Here it goes (incremental): --- diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index ff4173ce6924..e69a5cb99571 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -397,6 +397,19 @@ static int kvm_mmu_notifier_clear_young(struct mmu_notifier *mn, idx = srcu_read_lock(&kvm->srcu); spin_lock(&kvm->mmu_lock); + /* + * Even though we do not flush TLB, this will still adversely + * affect performance on pre-Haswell Intel EPT, where there is + * no EPT Access Bit to clear so that we have to tear down EPT + * tables instead. If we find this unacceptable, we can always + * add a parameter to kvm_age_hva so that it effectively doesn't + * do anything on clear_young. + * + * Also note that currently we never issue secondary TLB flushes + * from clear_young, leaving this job up to the regular system + * cadence. If we find this inaccurate, we might come up with a + * more sophisticated heuristic later. + */ young = kvm_age_hva(kvm, start, end); spin_unlock(&kvm->mmu_lock); srcu_read_unlock(&kvm->srcu, idx); -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx"> email@xxxxxxxxx </a>