Re: [Qemu-devel] [Qemu-ppc] [PATCH for-2.13] Add host_memory_backend_pagesize() helper

[Greg Kurz <groug@kaod.org>]

On Thu, 29 Mar 2018 08:37:48 +0200
Greg Kurz <groug@kaod.org> wrote:

> On Thu, 29 Mar 2018 16:25:37 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > There are a couple places (one generic, one target specific) where we need
> > to get the host page size associated with a particular memory backend.  I
> > have some upcoming code which will add another place which wants this.  So,
> > for convenience, add a helper function to calculate this.
> > 
> > host_memory_backend_pagesize() returns the host pagesize for a given
> > HostMemoryBackend object, or for the default backend (-mem-path) if passed
> > NULL.
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---  
> 
[...]
> > diff --git a/exec.c b/exec.c
> > index c09bd93df3..04856c2402 100644
> > --- a/exec.c
> > +++ b/exec.c
> > @@ -1488,18 +1488,13 @@ void ram_block_dump(Monitor *mon)
> >   */
> >  static int find_max_supported_pagesize(Object *obj, void *opaque)
> >  {
> > -    char *mem_path;
> >      long *hpsize_min = opaque;
> >  
> >      if (object_dynamic_cast(obj, TYPE_MEMORY_BACKEND)) {
> > -        mem_path = object_property_get_str(obj, "mem-path", NULL);
> > -        if (mem_path) {
> > -            long hpsize = qemu_mempath_getpagesize(mem_path);  
> 
> ... ie, this code currently leaks mem_path. This should be fixed in 2.12...
> 

FYI, I've sent a series to fix various object_property_get_str()-based
leaks, including this one.

https://lists.nongnu.org/archive/html/qemu-devel/2018-03/msg07266.html

> > -            if (hpsize < *hpsize_min) {
> > -                *hpsize_min = hpsize;
> > -            }
> > -        } else {
> > -            *hpsize_min = getpagesize();
> > +        long hpsize = host_memory_backend_pagesize(MEMORY_BACKEND(obj));
> > +
> > +        if (hpsize < *hpsize_min) {
> > +            *hpsize_min = hpsize;
> >          }
> >      }
> >  
> > @@ -1509,15 +1504,9 @@ static int find_max_supported_pagesize(Object *obj, void *opaque)
> >  long qemu_getrampagesize(void)
> >  {
> >      long hpsize = LONG_MAX;
> > -    long mainrampagesize;
> > +    long mainrampagesize = host_memory_backend_pagesize(NULL);
> >      Object *memdev_root;
> >  
> > -    if (mem_path) {
> > -        mainrampagesize = qemu_mempath_getpagesize(mem_path);
> > -    } else {
> > -        mainrampagesize = getpagesize();
> > -    }
> > -
> >      /* it's possible we have memory-backend objects with
> >       * hugepage-backed RAM. these may get mapped into system
> >       * address space via -numa parameters or memory hotplug
> > diff --git a/include/sysemu/hostmem.h b/include/sysemu/hostmem.h
> > index 47bc9846ac..f474ef97f6 100644
> > --- a/include/sysemu/hostmem.h
> > +++ b/include/sysemu/hostmem.h
> > @@ -68,4 +68,6 @@ MemoryRegion *host_memory_backend_get_memory(HostMemoryBackend *backend,
> >  
> >  void host_memory_backend_set_mapped(HostMemoryBackend *backend, bool mapped);
> >  bool host_memory_backend_is_mapped(HostMemoryBackend *backend);
> > +long host_memory_backend_pagesize(HostMemoryBackend *memdev);
> > +
> >  #endif
> > diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> > index b329cd8173..0adcf18c9f 100644
> > --- a/target/ppc/kvm.c
> > +++ b/target/ppc/kvm.c
> > @@ -493,15 +493,7 @@ static void kvm_fixup_page_sizes(PowerPCCPU *cpu)
> >  bool kvmppc_is_mem_backend_page_size_ok(const char *obj_path)
> >  {
> >      Object *mem_obj = object_resolve_path(obj_path, NULL);
> > -    char *mempath = object_property_get_str(mem_obj, "mem-path", NULL);
> > -    long pagesize;
> > -
> > -    if (mempath) {
> > -        pagesize = qemu_mempath_getpagesize(mempath);
> > -        g_free(mempath);  
> 
> ... but this code is ok. No leak :)
> 
> > -    } else {
> > -        pagesize = getpagesize();
> > -    }
> > +    long pagesize = host_memory_backend_pagesize(MEMORY_BACKEND(mem_obj));
> >  
> >      return pagesize >= max_cpu_page_size;
> >  }  
> 
>