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

[Anshuman Khandual <khandual@linux.vnet.ibm.com>]

On 09/14/2016 05:42 AM, Paul Mackerras wrote:
> On Wed, Sep 14, 2016 at 09:57:48AM +1000, Michael Ellerman wrote:
>> > Anshuman Khandual <khandual@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.
>> > 
>> > It looks like if CMA is not configured we will just fail instantly.
>> > 
>> > 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:
>> > 
>> > 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.
> You're right.  I'll fix it.

Thanks Paul, so I will not be sending follow up patch on this.