Re: [PATCH v7 08/21] KVM: s390: vsie: Create helpers to alloc and free vsie_pages

[Janosch Frank <frankja@linux.ibm.com>]

On 9/18/26 5:09 PM, Christoph Schlameuss wrote:
> Encapsulate allocation of struct vsie_page and free into its own methods
> to allow this from multiple code paths.
> 
> Signed-off-by: Christoph Schlameuss <schlameuss@linux.ibm.com>
> ---
>   arch/s390/kvm/s390/vsie.c | 26 ++++++++++++++++++++++----
>   1 file changed, 22 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/s390/kvm/s390/vsie.c b/arch/s390/kvm/s390/vsie.c
> index afc2e56fcabf..e7005bade757 100644
> --- a/arch/s390/kvm/s390/vsie.c
> +++ b/arch/s390/kvm/s390/vsie.c
> @@ -1502,6 +1502,24 @@ static void put_vsie_page(struct vsie_page *vsie_page)
>   	clear_bit(VSIE_PAGE_IN_USE, &vsie_page->flags);
>   }
>   
> +static void free_vsie_page(struct vsie_page *vsie_page)
> +{
> +	free_page((unsigned long)vsie_page);
> +}
> +
> +static struct vsie_page *alloc_vsie_page(struct kvm *kvm)
> +{
> +	struct vsie_page *vsie_page;
> +
> +	vsie_page = (struct vsie_page *)__get_free_page(GFP_KERNEL_ACCOUNT | __GFP_ZERO | GFP_DMA);
> +	if (!vsie_page)
> +		return vsie_page;
> +
> +	/* Mark it as invalid until it resides in the tree. */
> +	vsie_page->scb_gpa = ULONG_MAX;

I'd appreciate a properly named constant in favor of the comment being 
repeated. This has been a thorn in my side for a while now.

#define VSIE_PAGE_INV ULONG_MAX

> +	return vsie_page;
> +}
> +
>   /*
>    * Get or create a vsie page for a scb address.
>    *
> @@ -1537,7 +1555,7 @@ static struct vsie_page *get_vsie_page(struct kvm *kvm, unsigned long addr)
>   
>   	mutex_lock(&kvm->arch.vsie.mutex);
>   	if (kvm->arch.vsie.page_count < nr_vcpus) {
> -		vsie_page = (void *)__get_free_page(GFP_KERNEL_ACCOUNT | __GFP_ZERO | GFP_DMA);

Can't we go the kzalloc route like Mike did for the other page 
allocations? Maybe we can then also get rid of the free function that 
you're introducing?

https://lore.kernel.org/kvm/20260906-s390-kvm-v2-0-2cf6434e6646@kernel.org/

> +		vsie_page = alloc_vsie_page(kvm);
>   		if (!vsie_page) {
>   			mutex_unlock(&kvm->arch.vsie.mutex);
>   			return ERR_PTR(-ENOMEM);
> @@ -1557,9 +1575,9 @@ static struct vsie_page *get_vsie_page(struct kvm *kvm, unsigned long addr)
>   		if (vsie_page->scb_gpa != ULONG_MAX)
>   			radix_tree_delete(&kvm->arch.vsie.addr_to_page,
>   					  vsie_page->scb_gpa >> SCB_ALIGNMENT_SHIFT);