Re: [PATCH 1/2] KVM: x86/mmu: Split NX hugepage recovery flow into TDP and non-TDP flow

[Sean Christopherson]

On Mon, Aug 12, 2024, Vipin Sharma wrote:
> -static void kvm_recover_nx_huge_pages(struct kvm *kvm)
> +/*
> + * Get the first shadow mmu page of desired type from the NX huge pages list.
> + * Return NULL if list doesn't have the needed page with in the first max pages.
> + */
> +struct kvm_mmu_page *kvm_mmu_possible_nx_huge_page(struct kvm *kvm, bool tdp_mmu,
> +						   ulong max)

My preference is "unsigned long" over "unlong".  Line lengths be damned, for this
case ;-).

>  {
> -	unsigned long nx_lpage_splits = kvm->stat.nx_lpage_splits;
> -	struct kvm_memory_slot *slot;
> -	int rcu_idx;
> -	struct kvm_mmu_page *sp;
> -	unsigned int ratio;
> -	LIST_HEAD(invalid_list);
> -	bool flush = false;
> -	ulong to_zap;
> +	struct kvm_mmu_page *sp = NULL;
> +	ulong i = 0;
>  
> -	rcu_idx = srcu_read_lock(&kvm->srcu);
> -	write_lock(&kvm->mmu_lock);
> +	/*
> +	 * We use a separate list instead of just using active_mmu_pages because
> +	 * the number of shadow pages that be replaced with an NX huge page is
> +	 * expected to be relatively small compared to the total number of shadow
> +	 * pages. And because the TDP MMU doesn't use active_mmu_pages.
> +	 */
> +	list_for_each_entry(sp, &kvm->arch.possible_nx_huge_pages, possible_nx_huge_page_link) {
> +		if (i++ >= max)
> +			break;
> +		if (is_tdp_mmu_page(sp) == tdp_mmu)
> +			return sp;
> +	}

This is silly and wasteful.  E.g. in the (unlikely) case there's one TDP MMU
page amongst hundreds/thousands of shadow MMU pages, this will walk the list
until @max, and then move on to the shadow MMU.

Why not just use separate lists?