On 26/09/2018 09:58, Wei Yang wrote:
>
> +/* In case caller knows the rmap_head, just remove it by pte_list_remove() */
> +static void drop_spte_fast(struct kvm_rmap_head *rmap_head, u64 *sptep)
> +{
> + if (mmu_spte_clear_track_bits(sptep))
> + pte_list_remove(sptep, rmap_head);
> +}
> +
Here mmu_spte_clear_track_bits cannot return false, because
mmu_spte_clear_track_bits only returns false if
!is_shadow_present_pte(old_spte) and for_each_rmap_spte checks for that.
I think you should call this function pte_list_remove, and rename the
existing pte_list_remove to __pte_list_remove.
> @@ -1669,7 +1676,7 @@ static bool kvm_zap_rmapp(struct kvm *kvm, struct kvm_rmap_head *rmap_head)
> while ((sptep = rmap_get_first(rmap_head, &iter))) {
> rmap_printk("%s: spte %p %llx.\n", __func__, sptep, *sptep);
>
> - drop_spte(kvm, sptep);
> + drop_spte_fast(rmap_head, sptep);
> flush = true;
> }
>
> @@ -1705,7 +1712,7 @@ static int kvm_set_pte_rmapp(struct kvm *kvm, struct kvm_rmap_head *rmap_head,
> need_flush = 1;
>
> if (pte_write(*ptep)) {
> - drop_spte(kvm, sptep);
> + drop_spte_fast(rmap_head, sptep);
> goto restart;
> } else {
> new_spte = *sptep & ~PT64_BASE_ADDR_MASK;
... so this should probably move
mmu_spte_clear_track_bits(sptep)
from the "else" branch to before the "if", and call __pte_list_remove
directly.
Thanks,
Paolo
> @@ -5598,7 +5605,7 @@ static bool kvm_mmu_zap_collapsible_spte(struct kvm *kvm,
> if (sp->role.direct &&
> !kvm_is_reserved_pfn(pfn) &&
> PageTransCompoundMap(pfn_to_page(pfn))) {
> - drop_spte(kvm, sptep);
> + drop_spte_fast(rmap_head, sptep);
> need_tlb_flush = 1;
> goto restart;
> }
