Thread (13 messages) flat view 13 messages, 5 authors, 2016-09-14

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

From: Anshuman Khandual <hidden>
Date: 2016-09-14 03:53:04

On 09/14/2016 05:27 AM, Michael Ellerman 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.
It looks like if CMA is not configured we will just fail instantly.
Right and also we have this fallback registered any way. I wonder
why we are still debating about the need of a fallback mechanism
when we already have got one.
So this does look like something we should fix.

But I think it is just a bug in commit 572abd563bef ("KVM: PPC: Book3S
HV: Don't fall back to smaller HPT size in allocation ioctl"), which did:
Hmm, I think its something the commit missed to accommodate for.
But maybe yes, its a bug in the commit.
quoted hunk ↗ jump to hunk
diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c b/arch/powerpc/kvm/book3s_64_mmu_hv.c
index 1f9c0a17f445..10722b1e38b5 100644
--- a/arch/powerpc/kvm/book3s_64_mmu_hv.c
+++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c
@@ -70,7 +70,8 @@ long kvmppc_alloc_hpt(struct kvm *kvm, u32 *htab_orderp)
        }

        /* Lastly try successively smaller sizes from the page allocator */
-       while (!hpt && order > PPC_MIN_HPT_ORDER) {
+       /* Only do this if userspace didn't specify a size via ioctl */
+       while (!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)

Instead of guarding the loop entry with !htab_orderp, it should have
allowed the loop to enter, but prevented it from iterating if the
allocation fails and htab_orderp != 0.
Right and thats what Aneesh's proposed patch (in the other thread) does.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help