On Fri, Aug 13, 2021, Paolo Bonzini wrote: > On 12/08/21 19:46, Sean Christopherson wrote: > > > > if (iter->level == iter->min_level) > > > > return false; > > > > > > > > /* > > > > * Reread the SPTE before stepping down to avoid traversing into page > > > > * tables that are no longer linked from this entry. > > > > */ > > > > iter->old_spte = READ_ONCE(*rcu_dereference(iter->sptep)); \ > > > > ---> this is the code that is avoided > > > > child_pt = spte_to_child_pt(iter->old_spte, iter->level); / > > > > if (!child_pt) > > > > return false; > > > Ah, right - so I agree with Ben that it's not too important. > > Ya. There is a measurable performance improvement, but it's really only > > meaningful when there aren't many SPTEs to zap, otherwise the cost of zapping > > completely dominates the time. > > I don't understand. When try_step_down is called by tdp_iter_next, all it > does is really just the READ_ONCE, because spte_to_child_pt will see a > non-present PTE and return immediately. Why do two, presumably cache hot, > reads cause a measurable performance improvement? It's entirely possible my measurements were bad and/or noisy. Ah, and my kernel was running with CONFIG_PROVE_RCU=y, which makes the rcu_dereference() quite a bit more expensive.
