Re: [RFC] KVM: PPC: Book3S HV: Fall back to same size HPT in allocation ioctl
From: Anshuman Khandual <hidden>
Date: 2016-09-13 06:26:29
On 09/12/2016 05:35 PM, Aneesh Kumar K.V wrote:
Anshuman Khandual [off-list ref] writes:quoted
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.
quoted hunk ↗ jump to hunk
doing work around here.quoted
Signed-off-by: Anshuman Khandual <redacted> --- - 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.