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

[Hu Tao]

On Wed, Feb 19, 2014 at 10:36:57AM +0100, Igor Mammedov wrote:
> 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.

If lines about memory polices are moved up to hostmem.c, the only thing
left in ram_backend_memory_init() is calling memory_region_init_ram() to
allocate memory. Then it comes a problem that when to apply memory
polices? Choices:

1. apply memory polices in hostmem.c since this is the place user sets
   memory polices. But user_creatable_complete() seems not to support
   this.( but fix me)

2. cast to HostMemoryBackend in ram_backend_memory_init() (or in other
   memory backends) and add lines to apply memory polices.

3. provide an interface in HostMemoryBackendClass to do the thing and
   call it in subclasses. (this is basically the same as 2 except that
   we can reuse code)

Opinions?