[RFC] KVM: PPC: Book3S HV: Fall back to same size HPT in allocation ioctl

[Anshuman Khandual]

On 09/12/2016 05:35 PM, Aneesh Kumar K.V wrote:
> Anshuman Khandual <khandual at linux.vnet.ibm.com> writes:
> 
>> When the HPT size is explicitly passed on from the userspace, currently
>> the KVM_PPC_ALLOCATE_HTAB will try to allocate the requested size of HPT
>> from reserved CMA area and if that is not possible, the allocation just
>> fails. With the commit 572abd563befd56 ("KVM: PPC: Book3S HV: Don't fall
>> back to smaller HPT size in allocation ioctl"), it does not even try to
>> allocate the same order pages from the page allocator before failing for
>> good. Same order allocation should be attempted from the page allocator
>> as a fallback option when the CMA allocation attempt fails.
> 
> IMO we should fix the reason for these CMA allocation failure. We are just

IMHO, irrespective of the ability of CMA to satisfy allocation requests,
a fall back option from page allocator should always be there when the
guest is failing to start due to unavailability of memory. In my previous
response in this thread, also pointed out how there need to be a parity
between what we do for cases when ioctl calls specify size or not from
having a fallback option.

> doing work around here.
> 
>>
>> Signed-off-by: Anshuman Khandual <khandual at linux.vnet.ibm.com>
>> ---
>> - This change saves guests from failing to start after migration
>>
>>  arch/powerpc/kvm/book3s_64_mmu_hv.c | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c b/arch/powerpc/kvm/book3s_64_mmu_hv.c
>> index 05f09ae..0a30eb4 100644
>> --- a/arch/powerpc/kvm/book3s_64_mmu_hv.c
>> +++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c
>> @@ -78,6 +78,14 @@ long kvmppc_alloc_hpt(struct kvm *kvm, u32 *htab_orderp)
>>  			--order;
>>  	}
>>
>> +	/*
>> +	 * Fallback in case the userspace has provided a size via ioctl.
>> +	 * Try allocating the same order pages from the page allocator.
>> +	 */
>> +	if (!hpt && order > PPC_MIN_HPT_ORDER && htab_orderp)
>> +		hpt = __get_free_pages(GFP_KERNEL|__GFP_ZERO|__GFP_REPEAT|
>> +			__GFP_NOWARN, order - PAGE_SHIFT);
>> +
>>  	if (!hpt)
>>  		return -ENOMEM;
> 
> A better way to do that would be (not even compile tested) ?
> 
> diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c b/arch/powerpc/kvm/book3s_64_mmu_hv.c
> index 65b2b00d93d7..3f8995f27339 100644
> --- a/arch/powerpc/kvm/book3s_64_mmu_hv.c
> +++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c
> @@ -68,16 +68,18 @@ long kvmppc_alloc_hpt(struct kvm *kvm, u32 *htab_orderp)
>  		memset((void *)hpt, 0, (1ul << order));
>  		kvm->arch.hpt_cma_alloc = 1;
>  	}
> -
> -	/* Lastly try successively smaller sizes from the page allocator */
> -	/* Only do this if userspace didn't specify a size via ioctl */
> -	while (!hpt && order > PPC_MIN_HPT_ORDER && !htab_orderp) {
> +	/*
> +	 * Try successively smaller sizes from the page allocator.
> +	 * If a size was specified via an ioctl, we just try that
> +	 * specific size
> +	 */
> +-	while (!hpt && order > PPC_MIN_HPT_ORDER) {
>  		hpt = __get_free_pages(GFP_KERNEL|__GFP_ZERO|__GFP_REPEAT|
>  				       __GFP_NOWARN, order - PAGE_SHIFT);
> -		if (!hpt)
> -			--order;
> +		if (htab_orderp)
> +			break;
> +		--order;
>  	}
> -
>  	if (!hpt)
>  		return -ENOMEM;
> 

Initially thought about this way but then decided not to change this
existing code block instead just one more. But anything is fine, I
can just change this next time around.