Re: [Qemu-devel] [PATCH for-2.13] Clear mem_path if we fall back to anonymous RAM allocation

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2566 bytes --]

On Thu, Apr 19, 2018 at 06:30:37PM +0200, Greg Kurz wrote:
> On Thu, 19 Apr 2018 17:21:23 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > If the -mem-path option is set, we attempt to map the guest's RAM from a
> > file in the given path; it's usually used to back guest RAM with hugepages.
> > If we're unable to (e.g. not enough free hugepages) then we fall back to
> > allocating normal anonymous pages.  This behaviour can be surprising, but a
> > comment in allocate_system_memory_nonnuma() suggests it's legacy behaviour
> > we can't change.
> > 
> > What really isn't ok, though, is that in this case we leave mem_path set.
> > That means functions which attempt to determine the pagesize of main RAM
> > can erroneously think it is hugepage based on the requested path, even
> > though it's not.
> > 
> > This is particular bad for the pseries machine type.  KVM HV limitations
> > mean the guest can't use pagesizes larger than the host page size used to
> > back RAM.  That means that such a fallback, rather than merely giving
> > poorer performance that expected will cause the guest to freeze up early in
> 
> s/that expected/than expected/

Adjusted, thanks.

> 
> > boot as it attempts to use large page mappings that can't work.
> > 
> > This patch addresses the problem by clearing the mem_path variable when we
> > fall back to anonymous pages, meaning that subsequent attempts to
> > determine the RAM page size will get an accurate result.
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> >  numa.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > Paolo et al, as with my earlier patches adding some extensions to the
> > helpers for determining backing page sizes, if there are no objections
> > can I get an ack to merge this via my ppc tree?
> > 
> > diff --git a/numa.c b/numa.c
> > index 1116c90af9..78a869e598 100644
> > --- a/numa.c
> > +++ b/numa.c
> > @@ -469,6 +469,7 @@ static void allocate_system_memory_nonnuma(MemoryRegion *mr, Object *owner,
> >              /* Legacy behavior: if allocation failed, fall back to
> >               * regular RAM allocation.
> >               */
> > +            mem_path = NULL;
> >              memory_region_init_ram_nomigrate(mr, owner, name, ram_size, &error_fatal);
> >          }
> >  #else
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]