On 11/17/22 01:59, Robert Hoo wrote:
After break out, it immediately checks is_removed_spte(iter.old_spte)
and return, why not return here directly to avoid duplicated check and
another branch prediction?
/*
* Force the guest to retry the access if the upper level SPTEs
aren't
* in place, or if the target leaf SPTE is frozen by another
CPU.
*/
if (iter.level != fault->goal_level ||
is_removed_spte(iter.old_spte)) {
rcu_read_unlock();
return RET_PF_RETRY;
}
Good idea, more for readability than for efficiency. Another small
issue in David's patch is that
+ if (is_shadow_present_pte(iter.old_spte))
+ ret = tdp_mmu_split_huge_page(kvm, &iter, sp, true);
+ else
+ ret = tdp_mmu_link_sp(kvm, &iter, sp, true);
is assigning a 0/-EBUSY return value to ret, rather than RET_PF_* that
is assigned everywhere else in the function.
I sent a small patch to rectify both.
Paolo
