Re: [PATCH] KVM: x86/mmu: Ensure NX huge page recovery thread is alive before waking

[Sean Christopherson]

On Fri, Jan 24, 2025, Sean Christopherson wrote:
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index a45ae60e84ab..74c20dbb92da 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -7120,6 +7120,19 @@ static void mmu_destroy_caches(void)
>  	kmem_cache_destroy(mmu_page_header_cache);
>  }
>  
> +static void kvm_wake_nx_recovery_thread(struct kvm *kvm)
> +{
> +	/*
> +	 * The NX recovery thread is spawned on-demand at the first KVM_RUN and
> +	 * may not be valid even though the VM is globally visible.  Do nothing,
> +	 * as such a VM can't have any possible NX huge pages.
> +	 */
> +	struct vhost_task *nx_thread = READ_ONCE(kvm->arch.nx_huge_page_recovery_thread);
> +
> +	if (nx_thread)
> +		vhost_task_wake(nx_thread);

As mentioned in the original thread[*], I belatedly realized there's a race with
this approach.  If vhost_task_start() completes and kvm_nx_huge_page_recovery_worker()
runs before a parameter change, but the parameter change runs before the WRITE_ONCE(),
then the worker will run with stale params and could end up sleeping for far longer
than userspace wants.

I assume we could address that by taking kvm->arch.nx_once.mutex in this helper
instead of using the lockless approach.  I don't think that would lead to any
deadlocks?

[*] https://lore.kernel.org/all/Z5QsBXJ7rkJFDtmK@xxxxxxxxxx