Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 20 May 2022 14:49:19 +0000
From:   Sean Christopherson <seanjc@google.com>
To:     Paolo Bonzini <pbonzini@redhat.com>
Cc:     Yajun Deng <yajun.deng@linux.dev>, vkuznets@redhat.com,
        wanpengli@tencent.com, jmattson@google.com, joro@8bytes.org,
        tglx@linutronix.de, mingo@redhat.com, bp@alien8.de,
        dave.hansen@linux.intel.com, hpa@zytor.com, x86@kernel.org,
        kvm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] KVM: x86: Move kzalloc out of atomic context on
 PREEMPT_RT
Message-ID: <YoeqbxPwOnOZx5oI@google.com>
References: <20220519090218.2230653-1-yajun.deng@linux.dev>
 <YoZeI6UeQbP3t1dF@google.com>
 <f7585471-43be-4b40-f398-dfd7dc937131@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <f7585471-43be-4b40-f398-dfd7dc937131@redhat.com>
Precedence: bulk

On Fri, May 20, 2022, Paolo Bonzini wrote:
> On 5/19/22 17:11, Sean Christopherson wrote:
> > AFAICT, kfree() is safe to call under a raw spinlock, so this?  Compile tested
> > only...
> 
> Freeing outside the lock is not complicated enough to check if it is:
> 
> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
> index 6aa1241a80b7..f849f7c9fbf2 100644
> --- a/arch/x86/kernel/kvm.c
> +++ b/arch/x86/kernel/kvm.c
> @@ -229,12 +229,15 @@ void kvm_async_pf_task_wake(u32 token)
>  		dummy->cpu = smp_processor_id();
>  		init_swait_queue_head(&dummy->wq);
>  		hlist_add_head(&dummy->link, &b->list);
> +		dummy = NULL;
>  	} else {
> -		kfree(dummy);
>  		apf_task_wake_one(n);
>  	}
>  	raw_spin_unlock(&b->lock);
> -	return;
> +
> +	/* A dummy token might be allocated and ultimately not used.  */
> +	if (dummy)
> +		kfree(dummy);
>  }
>  EXPORT_SYMBOL_GPL(kvm_async_pf_task_wake);
> 
> 
> I queued your patch with the above fixup.

Ha, I wrote it exactly that way, then grepped around found a few instances of kfree()
being called in side a raw spinlock, so changed it back :-)

100% agree it's not worth having to generate another patch if it turns out those
callers are wrong.