Re: [Qemu-devel] [PATCH v18 13/14] memory backend: fill memory backend ram fields

[Igor Mammedov]

On Wed, 19 Feb 2014 10:03:13 +0100
Paolo Bonzini <address@hidden> wrote:

>   19/02/2014 08:54, Hu Tao ha scritto:
> > Thus makes user control how to allocate memory for ram backend.
> >
> > Signed-off-by: Hu Tao <address@hidden>
> > ---
> >  backends/hostmem-ram.c  | 158 
> > ++++++++++++++++++++++++++++++++++++++++++++++++
> >  include/sysemu/sysemu.h |   2 +
> >  2 files changed, 160 insertions(+)
> >
> > diff --git a/backends/hostmem-ram.c b/backends/hostmem-ram.c
[...]

> >  static int
> >  ram_backend_memory_init(HostMemoryBackend *backend, Error **errp)
> >  {
> > +    HostMemoryBackendRam *ram_backend = MEMORY_BACKEND_RAM(backend);
> > +    int mode = ram_backend->policy;
> > +    void *p;
> > +    unsigned long maxnode;
> > +
> >      if (!memory_region_size(&backend->mr)) {
> >          memory_region_init_ram(&backend->mr, OBJECT(backend),
> >                                 object_get_canonical_path(OBJECT(backend)),
> >                                 backend->size);
> > +
> > +        p = memory_region_get_ram_ptr(&backend->mr);
> > +        maxnode = find_last_bit(ram_backend->host_nodes, MAX_NODES);
> > +
> > +        mode |= ram_backend->relative ? MPOL_F_RELATIVE_NODES :
> > +            MPOL_F_STATIC_NODES;
> > +        /* This is a workaround for a long standing bug in Linux'
> > +         * mbind implementation, which cuts off the last specified
> > +         * node. To stay compatible should this bug be fixed, we
> > +         * specify one more node and zero this one out.
> > +         */
> > +        if (syscall(SYS_mbind, p, backend->size, mode,
> > +                    ram_backend->host_nodes, maxnode + 2, 0)) {
> 
> This does not compile on non-Linux; also, does libnuma include the 
> workaround?  If so, this is a hint that we should be using libnuma 
> instead...
> 
> Finally, all this code should be in hostmem.c, not hostmem-ram.c, 
> because the same policies can be applied to hugepage-backed memory.
> 
> Currently host_memory_backend_get_memory is calling bc->memory_init. 
> Probably the call should be replaced by something like
I've pushed to github updated version of memdev, where
host_memory_backend_get_memory() is just convenience wrapper to get
access to memdev's internal MemoryRegion.

All initialization now is done in user_creatable->complete() method
which calls ram_backend_memory_init() so leaving it as is should be fine.

> 
> static void
> host_memory_backend_alloc(HostMemoryBackend *backend, Error **errp)
> {
>      Error *local_err = NULL;
>      bc->memory_init(backend, &local_err);
>      if (local_err != NULL) {
>          error_propagate(errp, local_err);
>          return;
>      }
> 
>      ... set policy ...
> }
> 
> ...
> 
>      Error *local_err = NULL;
>      host_memory_backend_alloc(backend, &local_err);
>      if (local_err != NULL) {
>          error_propagate(errp, local_err);
>          return NULL;
>      }
> 
>      assert(memory_region_size(&backend->mr) != 0);
>      return &backend->mr;
> }
> 
[...]