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

2014-03-02 Thread Hu Tao 
On Tue, Feb 25, 2014 at 06:09:20PM +0800, Hu Tao wrote:
 On Wed, Feb 19, 2014 at 10:03:13AM +0100, Paolo Bonzini wrote:
 
 ...
 
   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...
 
 Tested with libnuma and works fine without the workaround. Will use
 libnuma in v19.

Sorry I might do something wrong about the test. With libnuma the
workaround is still needed. I checked numactl-2.0.9, it doesn't include
the workaround.

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

2014-02-26 Thread Igor Mammedov 
On Wed, 26 Feb 2014 13:00:03 +0800
Hu Tao hu...@cn.fujitsu.com wrote:

 On Tue, Feb 25, 2014 at 03:15:53PM +0100, Paolo Bonzini wrote:
  Il 25/02/2014 11:20, Hu Tao ha scritto:
  There is a problem that user_creatable_complete() is called before
  adding object to /objects (see object_create()), which triggers a
  assert failure when calling object_get_canonical_path() in
  ram_backend_memory_init(). Any ideas?
  
  You can call object_property_add_child before calling
  user_creatable_complete, and then call object_unparent (in addition
  to object_unref) if creation fails.
  
  Paolo
 
 Something like this?
 
 From 59c999c840e4305bb2b95389bbea32e07c1c14c0 Mon Sep 17 00:00:00 2001
 From: Hu Tao hu...@cn.fujitsu.com
 Date: Wed, 26 Feb 2014 12:54:34 +0800
 Subject: [PATCH] call user_creatable_complete() after adding object to
  /objects
 
 This makes it possible to get the path of object by calling
 object_get_canonical_path() in the complete callback if
 someone wants it.
 
 Signed-off-by: Hu Tao hu...@cn.fujitsu.com
 ---
  vl.c | 7 ---
  1 file changed, 4 insertions(+), 3 deletions(-)
 
 diff --git a/vl.c b/vl.c
 index 1d27b34..30b4297 100644
 --- a/vl.c
 +++ b/vl.c
 @@ -2770,14 +2770,15 @@ static int object_create(QemuOpts *opts, void *opaque)
  goto out;
  }
  
 +object_property_add_child(container_get(object_get_root(), /objects),
 +  id, obj, local_err);
 +
  user_creatable_complete(obj, local_err);
  if (local_err) {
 +object_unparent(obj);
  goto out;
  }
  
 -object_property_add_child(container_get(object_get_root(), /objects),
 -  id, obj, local_err);
 -
  out:
  object_unref(obj);
  if (local_err) {
Failure case is not handled properly,
I'm sorry that I've forgotten to mention prerequisite path
https://github.com/imammedo/qemu/commit/72a079b88e055fc690c8895a99ccbcce36f6fc1f
in stage tree https://github.com/imammedo/qemu/commits/memory-hotplug-v8

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

2014-02-26 Thread Paolo Bonzini 

Il 26/02/2014 06:57, Hu Tao ha scritto:

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)


I like (3).  I understand it's something like

void memory_backend_apply_mempolicy(HostMemoryBackend *be,
void *addr, size_t len, Error **err)

?

Paolo

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

2014-02-26 Thread Igor Mammedov 
On Wed, 26 Feb 2014 13:57:06 +0800
Hu Tao hu...@cn.fujitsu.com wrote:

 On Wed, Feb 19, 2014 at 10:36:57AM +0100, Igor Mammedov wrote:
  On Wed, 19 Feb 2014 10:03:13 +0100
  Paolo Bonzini pbonz...@redhat.com 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 hu...@cn.fujitsu.com
---
 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)
if we assume that NUMA policies apply to every hostmem derived backend,
then we could realize() approach used by DEVICE. i.e.
set NUMA policies in hostmem.c:hostmemory_backend_memory_init()

Add parent_complete field to ram-backend class and store there parent's
complete pointer. Then we can do:

ram_backend_memory_init(UserCreatable *uc, Error **errp) {
memory_region_init_ram();
...
MEMORY_BACKEND_RAM_CLASS(uc)-parent_complete(uc, errp);
...
}

 
 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?

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

2014-02-26 Thread Hu Tao 
On Wed, Feb 26, 2014 at 09:47:08AM +0100, Igor Mammedov wrote:
 On Wed, 26 Feb 2014 13:00:03 +0800
 Hu Tao hu...@cn.fujitsu.com wrote:
 
  On Tue, Feb 25, 2014 at 03:15:53PM +0100, Paolo Bonzini wrote:
   Il 25/02/2014 11:20, Hu Tao ha scritto:
   There is a problem that user_creatable_complete() is called before
   adding object to /objects (see object_create()), which triggers a
   assert failure when calling object_get_canonical_path() in
   ram_backend_memory_init(). Any ideas?
   
   You can call object_property_add_child before calling
   user_creatable_complete, and then call object_unparent (in addition
   to object_unref) if creation fails.
   
   Paolo
  
  Something like this?
  
  From 59c999c840e4305bb2b95389bbea32e07c1c14c0 Mon Sep 17 00:00:00 2001
  From: Hu Tao hu...@cn.fujitsu.com
  Date: Wed, 26 Feb 2014 12:54:34 +0800
  Subject: [PATCH] call user_creatable_complete() after adding object to
   /objects
  
  This makes it possible to get the path of object by calling
  object_get_canonical_path() in the complete callback if
  someone wants it.
  
  Signed-off-by: Hu Tao hu...@cn.fujitsu.com
  ---
   vl.c | 7 ---
   1 file changed, 4 insertions(+), 3 deletions(-)
  
  diff --git a/vl.c b/vl.c
  index 1d27b34..30b4297 100644
  --- a/vl.c
  +++ b/vl.c
  @@ -2770,14 +2770,15 @@ static int object_create(QemuOpts *opts, void 
  *opaque)
   goto out;
   }
   
  +object_property_add_child(container_get(object_get_root(), /objects),
  +  id, obj, local_err);
  +
   user_creatable_complete(obj, local_err);
   if (local_err) {
  +object_unparent(obj);
   goto out;
   }
   
  -object_property_add_child(container_get(object_get_root(), /objects),
  -  id, obj, local_err);
  -
   out:
   object_unref(obj);
   if (local_err) {
 Failure case is not handled properly,
 I'm sorry that I've forgotten to mention prerequisite path
 https://github.com/imammedo/qemu/commit/72a079b88e055fc690c8895a99ccbcce36f6fc1f
 in stage tree https://github.com/imammedo/qemu/commits/memory-hotplug-v8

No problem. I should've noticed the patch when cherry-picking. Will you
post it seperately?

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

2014-02-26 Thread Paolo Bonzini 

Il 26/02/2014 10:10, Igor Mammedov ha scritto:

if we assume that NUMA policies apply to every hostmem derived backend,
then we could realize() approach used by DEVICE. i.e.
set NUMA policies in hostmem.c:hostmemory_backend_memory_init()

Add parent_complete field to ram-backend class and store there parent's
complete pointer. Then we can do:

ram_backend_memory_init(UserCreatable *uc, Error **errp) {
memory_region_init_ram();
...
MEMORY_BACKEND_RAM_CLASS(uc)-parent_complete(uc, errp);
...
}



The problem is that some backends might not be handled the same way. 
For example, not all backends might produce a single void*/size_t pair 
for the entire region.  Think of a composite backend that produces a 
large memory region from two smaller ones.


Paolo

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

2014-02-26 Thread Paolo Bonzini 

Il 26/02/2014 09:47, Igor Mammedov ha scritto:

I'm sorry that I've forgotten to mention prerequisite path
https://github.com/imammedo/qemu/commit/72a079b88e055fc690c8895a99ccbcce36f6fc1f
in stage tree https://github.com/imammedo/qemu/commits/memory-hotplug-v8


Thanks, I'll add this patch to the numa-queue branch.

Paolo

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

2014-02-26 Thread Igor Mammedov 
On Wed, 26 Feb 2014 16:59:44 +0800
Hu Tao hu...@cn.fujitsu.com wrote:

 On Wed, Feb 26, 2014 at 09:47:08AM +0100, Igor Mammedov wrote:
  On Wed, 26 Feb 2014 13:00:03 +0800
  Hu Tao hu...@cn.fujitsu.com wrote:
  
   On Tue, Feb 25, 2014 at 03:15:53PM +0100, Paolo Bonzini wrote:
Il 25/02/2014 11:20, Hu Tao ha scritto:
There is a problem that user_creatable_complete() is called before
adding object to /objects (see object_create()), which triggers a
assert failure when calling object_get_canonical_path() in
ram_backend_memory_init(). Any ideas?

You can call object_property_add_child before calling
user_creatable_complete, and then call object_unparent (in addition
to object_unref) if creation fails.

Paolo
   
   Something like this?
   
   From 59c999c840e4305bb2b95389bbea32e07c1c14c0 Mon Sep 17 00:00:00 2001
   From: Hu Tao hu...@cn.fujitsu.com
   Date: Wed, 26 Feb 2014 12:54:34 +0800
   Subject: [PATCH] call user_creatable_complete() after adding object to
/objects
   
   This makes it possible to get the path of object by calling
   object_get_canonical_path() in the complete callback if
   someone wants it.
   
   Signed-off-by: Hu Tao hu...@cn.fujitsu.com
   ---
vl.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
   
   diff --git a/vl.c b/vl.c
   index 1d27b34..30b4297 100644
   --- a/vl.c
   +++ b/vl.c
   @@ -2770,14 +2770,15 @@ static int object_create(QemuOpts *opts, void 
   *opaque)
goto out;
}

   +object_property_add_child(container_get(object_get_root(), 
   /objects),
   +  id, obj, local_err);
   +
user_creatable_complete(obj, local_err);
if (local_err) {
   +object_unparent(obj);
goto out;
}

   -object_property_add_child(container_get(object_get_root(), 
   /objects),
   -  id, obj, local_err);
   -
out:
object_unref(obj);
if (local_err) {
  Failure case is not handled properly,
  I'm sorry that I've forgotten to mention prerequisite path
  https://github.com/imammedo/qemu/commit/72a079b88e055fc690c8895a99ccbcce36f6fc1f
  in stage tree https://github.com/imammedo/qemu/commits/memory-hotplug-v8
 
 No problem. I should've noticed the patch when cherry-picking. Will you
 post it seperately?
 
 

sure

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

2014-02-26 Thread Igor Mammedov 
On Wed, 26 Feb 2014 11:33:30 +0100
Paolo Bonzini pbonz...@redhat.com wrote:

 Il 26/02/2014 10:10, Igor Mammedov ha scritto:
  if we assume that NUMA policies apply to every hostmem derived backend,
  then we could realize() approach used by DEVICE. i.e.
  set NUMA policies in hostmem.c:hostmemory_backend_memory_init()
 
  Add parent_complete field to ram-backend class and store there parent's
  complete pointer. Then we can do:
 
  ram_backend_memory_init(UserCreatable *uc, Error **errp) {
  memory_region_init_ram();
  ...
  MEMORY_BACKEND_RAM_CLASS(uc)-parent_complete(uc, errp);
  ...
  }
 
 
 The problem is that some backends might not be handled the same way. 
 For example, not all backends might produce a single void*/size_t pair 
 for the entire region.  Think of a composite backend that produces a 
 large memory region from two smaller ones.

I'd prefer to keep backends simple, with 1:1 mapping to memory regions.
Is there a need in composite one or something similar?

 
 Paolo

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

2014-02-26 Thread Paolo Bonzini 

Il 26/02/2014 13:31, Igor Mammedov ha scritto:

 The problem is that some backends might not be handled the same way.
 For example, not all backends might produce a single void*/size_t pair
 for the entire region.  Think of a composite backend that produces a
 large memory region from two smaller ones.

I'd prefer to keep backends simple, with 1:1 mapping to memory regions.


I agree.  However not all backends may have a mapping to a RAM memory 
region.  A composite backend could create a container memory region 
whose children are other HostMemoryBackend objects.



Is there a need in composite one or something similar?


I've heard of users that want a node backed partially by hugetlbfs and 
partially by regular RAM.  Not sure why.


Paolo

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

2014-02-26 Thread Marcelo Tosatti 
On Wed, Feb 26, 2014 at 01:45:38PM +0100, Paolo Bonzini wrote:
 Il 26/02/2014 13:31, Igor Mammedov ha scritto:
  The problem is that some backends might not be handled the same way.
  For example, not all backends might produce a single void*/size_t pair
  for the entire region.  Think of a composite backend that produces a
  large memory region from two smaller ones.
 I'd prefer to keep backends simple, with 1:1 mapping to memory regions.
 
 I agree.  However not all backends may have a mapping to a RAM
 memory region.  A composite backend could create a container memory
 region whose children are other HostMemoryBackend objects.
 
 Is there a need in composite one or something similar?
 
 I've heard of users that want a node backed partially by hugetlbfs
 and partially by regular RAM.  Not sure why.
 
 Paolo

To overcommit the non hugetlbfs backed guest RAM (think guest pagecache on that 
non
hugetlbfs backed memory, swappable and KSM-able).

The problem is, you have to in someway guarantee the guest allocates 
1GB pages out of the hugetlb backed GPA ranges. Some thoughts
(honestly, dislike all of them):

1) Boot guest with hugepages, allocate hugepages in guest,
later on hotplug 4K backed ranges. HV-unaware reboot might fail,
though.

2) Communicate hugepage GPAs to guest.

3) Create holes in non hugepage backed GPA range.

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

2014-02-26 Thread Paolo Bonzini 

Il 26/02/2014 13:58, Marcelo Tosatti ha scritto:

I'd prefer to keep backends simple, with 1:1 mapping to memory regions.


I agree.  However not all backends may have a mapping to a RAM
memory region.  A composite backend could create a container memory
region whose children are other HostMemoryBackend objects.


Is there a need in composite one or something similar?


I've heard of users that want a node backed partially by hugetlbfs
and partially by regular RAM.  Not sure why.


To overcommit the non hugetlbfs backed guest RAM (think guest pagecache on that 
non
hugetlbfs backed memory, swappable and KSM-able).

The problem is, you have to in someway guarantee the guest allocates
1GB pages out of the hugetlb backed GPA ranges. Some thoughts
(honestly, dislike all of them):

1) Boot guest with hugepages, allocate hugepages in guest,
later on hotplug 4K backed ranges. HV-unaware reboot might fail,
though.

2) Communicate hugepage GPAs to guest.

3) Create holes in non hugepage backed GPA range.


I guess (2) is the only one I like, and I like it just because it 
officially becomes Not Our Problem.


Paolo

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

2014-02-26 Thread Igor Mammedov 
On Wed, 26 Feb 2014 13:45:38 +0100
Paolo Bonzini pbonz...@redhat.com wrote:

 Il 26/02/2014 13:31, Igor Mammedov ha scritto:
   The problem is that some backends might not be handled the same way.
   For example, not all backends might produce a single void*/size_t pair
   for the entire region.  Think of a composite backend that produces a
   large memory region from two smaller ones.
  I'd prefer to keep backends simple, with 1:1 mapping to memory regions.
 
 I agree.  However not all backends may have a mapping to a RAM memory 
 region.  A composite backend could create a container memory region 
 whose children are other HostMemoryBackend objects.
 
  Is there a need in composite one or something similar?
 
 I've heard of users that want a node backed partially by hugetlbfs and 
 partially by regular RAM.  Not sure why.
Isn't issue here in how backend is mapped into GPA? Well that is not
backend's job.

Once one starts to put layout (alignment, noncontinuously mapped
memory regions inside of container, ...), mapping HPA-GPA gets complicated.

It would be better to use simple building blocks and model as:
2 separate backends (ram + hugetlbfs) and 2 corresponding DIMM devices.


 
 Paolo

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

2014-02-26 Thread Paolo Bonzini 

Il 26/02/2014 14:43, Igor Mammedov ha scritto:

On Wed, 26 Feb 2014 13:45:38 +0100
Paolo Bonzini pbonz...@redhat.com wrote:


Il 26/02/2014 13:31, Igor Mammedov ha scritto:

The problem is that some backends might not be handled the same way.
For example, not all backends might produce a single void*/size_t pair
for the entire region.  Think of a composite backend that produces a
large memory region from two smaller ones.

I'd prefer to keep backends simple, with 1:1 mapping to memory regions.


I agree.  However not all backends may have a mapping to a RAM memory
region.  A composite backend could create a container memory region
whose children are other HostMemoryBackend objects.


Is there a need in composite one or something similar?


I've heard of users that want a node backed partially by hugetlbfs and
partially by regular RAM.  Not sure why.

Isn't issue here in how backend is mapped into GPA? Well that is not
backend's job.

Once one starts to put layout (alignment, noncontinuously mapped
memory regions inside of container, ...), mapping HPA-GPA gets complicated.

It would be better to use simple building blocks and model as:
2 separate backends (ram + hugetlbfs) and 2 corresponding DIMM devices.


Right, I had forgotten that you can have cold-plugged DIMM devices. 
That's a nice solution, also because it simplifies passing the GPA 
configuration down to the guest.


How would that translate to sharing HostMemoryBackend code for memory 
policies?  Which of Hu Tao's proposals do you like best?


Paolo

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

2014-02-26 Thread Igor Mammedov 
On Wed, 26 Feb 2014 14:47:28 +0100
Paolo Bonzini pbonz...@redhat.com wrote:

 Il 26/02/2014 14:43, Igor Mammedov ha scritto:
  On Wed, 26 Feb 2014 13:45:38 +0100
  Paolo Bonzini pbonz...@redhat.com wrote:
 
  Il 26/02/2014 13:31, Igor Mammedov ha scritto:
  The problem is that some backends might not be handled the same way.
  For example, not all backends might produce a single void*/size_t pair
  for the entire region.  Think of a composite backend that produces a
  large memory region from two smaller ones.
  I'd prefer to keep backends simple, with 1:1 mapping to memory regions.
 
  I agree.  However not all backends may have a mapping to a RAM memory
  region.  A composite backend could create a container memory region
  whose children are other HostMemoryBackend objects.
 
  Is there a need in composite one or something similar?
 
  I've heard of users that want a node backed partially by hugetlbfs and
  partially by regular RAM.  Not sure why.
  Isn't issue here in how backend is mapped into GPA? Well that is not
  backend's job.
 
  Once one starts to put layout (alignment, noncontinuously mapped
  memory regions inside of container, ...), mapping HPA-GPA gets complicated.
 
  It would be better to use simple building blocks and model as:
  2 separate backends (ram + hugetlbfs) and 2 corresponding DIMM devices.
 
 Right, I had forgotten that you can have cold-plugged DIMM devices. 
 That's a nice solution, also because it simplifies passing the GPA 
 configuration down to the guest.
 
 How would that translate to sharing HostMemoryBackend code for memory 
 policies?  Which of Hu Tao's proposals do you like best?
possible choices could be:

 1: 'realize' approach I suggested
  drawback is: assumption that all backends derived from HostMemoryBackend
   will inherit NUMA controls even if backend shouldn't have
   one (for example: fictional remote memory backend)
  plus: derived types from HostMemoryBackend, don't need to know anything
about NUMA.
 2: #3 from Hu Tao's suggestion
  drawback is: every new backend have to explicitly call NUMA callbacks
  somewhat plus is that not NUMA aware backends could ignore NUMA callbacks,
  but they would still have NUMA properties available, which is confusing.

 3: might be over-engineered #1 from above: build proper class hierarchy:
  HostMemoryBackend  - NumaMemoryBackend - RamBackend
| - HugepageBackend
|- whatever else

 
 Paolo

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

2014-02-26 Thread Paolo Bonzini 

Il 26/02/2014 15:25, Igor Mammedov ha scritto:

 1: 'realize' approach I suggested
  drawback is: assumption that all backends derived from HostMemoryBackend
   will inherit NUMA controls even if backend shouldn't have
   one (for example: fictional remote memory backend)
  plus: derived types from HostMemoryBackend, don't need to know anything
about NUMA.


Let's go with this for now.  It keeps things simple.

Paolo

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

2014-02-25 Thread Hu Tao 
On Wed, Feb 19, 2014 at 10:03:13AM +0100, Paolo Bonzini wrote:

...

  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...

Tested with libnuma and works fine without the workaround. Will use
libnuma in v19.

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

2014-02-25 Thread 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 pbonz...@redhat.com 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 hu...@cn.fujitsu.com
   ---
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.

There is a problem that user_creatable_complete() is called before
adding object to /objects (see object_create()), which triggers a
assert failure when calling object_get_canonical_path() in
ram_backend_memory_init(). Any ideas?

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

2014-02-25 Thread Paolo Bonzini 

Il 25/02/2014 11:20, Hu Tao ha scritto:

There is a problem that user_creatable_complete() is called before
adding object to /objects (see object_create()), which triggers a
assert failure when calling object_get_canonical_path() in
ram_backend_memory_init(). Any ideas?


You can call object_property_add_child before calling 
user_creatable_complete, and then call object_unparent (in addition to 
object_unref) if creation fails.


Paolo

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

2014-02-25 Thread Hu Tao 
On Tue, Feb 25, 2014 at 03:15:53PM +0100, Paolo Bonzini wrote:
 Il 25/02/2014 11:20, Hu Tao ha scritto:
 There is a problem that user_creatable_complete() is called before
 adding object to /objects (see object_create()), which triggers a
 assert failure when calling object_get_canonical_path() in
 ram_backend_memory_init(). Any ideas?
 
 You can call object_property_add_child before calling
 user_creatable_complete, and then call object_unparent (in addition
 to object_unref) if creation fails.
 
 Paolo

Something like this?

From 59c999c840e4305bb2b95389bbea32e07c1c14c0 Mon Sep 17 00:00:00 2001
From: Hu Tao hu...@cn.fujitsu.com
Date: Wed, 26 Feb 2014 12:54:34 +0800
Subject: [PATCH] call user_creatable_complete() after adding object to
 /objects

This makes it possible to get the path of object by calling
object_get_canonical_path() in the complete callback if
someone wants it.

Signed-off-by: Hu Tao hu...@cn.fujitsu.com
---
 vl.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/vl.c b/vl.c
index 1d27b34..30b4297 100644
--- a/vl.c
+++ b/vl.c
@@ -2770,14 +2770,15 @@ static int object_create(QemuOpts *opts, void *opaque)
 goto out;
 }
 
+object_property_add_child(container_get(object_get_root(), /objects),
+  id, obj, local_err);
+
 user_creatable_complete(obj, local_err);
 if (local_err) {
+object_unparent(obj);
 goto out;
 }
 
-object_property_add_child(container_get(object_get_root(), /objects),
-  id, obj, local_err);
-
 out:
 object_unref(obj);
 if (local_err) {
-- 
1.8.5.2.229.g4448466

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

2014-02-25 Thread 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 pbonz...@redhat.com 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 hu...@cn.fujitsu.com
   ---
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?

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

2014-02-19 Thread Paolo Bonzini 

 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 hu...@cn.fujitsu.com
---
 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
index a496dbd..2da9341 100644
--- a/backends/hostmem-ram.c
+++ b/backends/hostmem-ram.c
@@ -10,23 +10,179 @@
  * See the COPYING file in the top-level directory.
  */
 #include sysemu/hostmem.h
+#include sysemu/sysemu.h
+#include qemu/bitmap.h
+#include qapi-visit.h
+#include qemu/config-file.h
+#include qapi/opts-visitor.h

 #define TYPE_MEMORY_BACKEND_RAM memory-ram
+#define MEMORY_BACKEND_RAM(obj) \
+OBJECT_CHECK(HostMemoryBackendRam, (obj), TYPE_MEMORY_BACKEND_RAM)

+typedef struct HostMemoryBackendRam HostMemoryBackendRam;
+
+/**
+ * @HostMemoryBackendRam
+ *
+ * @parent: opaque parent object container
+ * @host_nodes: host nodes bitmap used for memory policy
+ * @policy: host memory policy
+ * @relative: if the host nodes bitmap is relative
+ */
+struct HostMemoryBackendRam {
+/* private */
+HostMemoryBackend parent;
+
+DECLARE_BITMAP(host_nodes, MAX_NODES);
+HostMemPolicy policy;
+bool relative;
+};
+
+static void
+get_host_nodes(Object *obj, Visitor *v, void *opaque, const char *name,
+   Error **errp)
+{
+HostMemoryBackendRam *ram_backend = MEMORY_BACKEND_RAM(obj);
+uint16List *host_nodes = NULL;
+uint16List **node = host_nodes;
+unsigned long value;
+
+value = find_first_bit(ram_backend-host_nodes, MAX_NODES);
+if (value == MAX_NODES) {
+return;
+}
+
+*node = g_malloc0(sizeof(**node));
+(*node)-value = value;
+node = (*node)-next;
+
+do {
+value = find_next_bit(ram_backend-host_nodes, MAX_NODES, value + 1);
+if (value == MAX_NODES) {
+break;
+}
+
+*node = g_malloc0(sizeof(**node));
+(*node)-value = value;
+node = (*node)-next;
+} while (true);
+
+visit_type_uint16List(v, host_nodes, name, errp);
+}
+
+static void
+set_host_nodes(Object *obj, Visitor *v, void *opaque, const char *name,
+   Error **errp)
+{
+HostMemoryBackendRam *ram_backend = MEMORY_BACKEND_RAM(obj);
+uint16List *l = NULL;
+
+visit_type_uint16List(v, l, name, errp);
+
+while (l) {
+bitmap_set(ram_backend-host_nodes, l-value, 1);
+l = l-next;
+}
+}
+
+static const char *policies[HOST_MEM_POLICY_MAX + 1] = {
+[HOST_MEM_POLICY_DEFAULT] = default,
+[HOST_MEM_POLICY_PREFERRED] = preferred,
+[HOST_MEM_POLICY_MEMBIND] = membind,
+[HOST_MEM_POLICY_INTERLEAVE] = interleave,
+[HOST_MEM_POLICY_MAX] = NULL,
+};


This is already available in qapi-types.c as HostMemPolicy_lookup.


+static void
+get_policy(Object *obj, Visitor *v, void *opaque, const char *name,
+   Error **errp)
+{
+HostMemoryBackendRam *ram_backend = MEMORY_BACKEND_RAM(obj);
+int policy = ram_backend-policy;
+
+visit_type_enum(v, policy, policies, NULL, name, errp);
+}
+
+static void
+set_policy(Object *obj, Visitor *v, void *opaque, const char *name,
+   Error **errp)
+{
+HostMemoryBackendRam *ram_backend = MEMORY_BACKEND_RAM(obj);
+int policy;
+
+visit_type_enum(v, policy, policies, NULL, name, errp);
+ram_backend-policy = policy;


I think you need to set an error if backend-mr != NULL.


+}
+
+
+static bool get_relative(Object *obj, Error **errp)
+{
+HostMemoryBackendRam *ram_backend = MEMORY_BACKEND_RAM(obj);
+
+return ram_backend-relative;
+}
+
+static void set_relative(Object *obj, bool value, Error **errp)
+{
+HostMemoryBackendRam *ram_backend = MEMORY_BACKEND_RAM(obj);
+
+ram_backend-relative = value;
+}


Do we need relative vs. static?  Also, the default right now is static, 
while in Linux kernel this is a tri-state: relative, static, default.


I think that for now we should just omit this and only allow the default 
setting.  We can introduce an enum later without make the API 
backwards-incompatible.



+#include sys/syscall.h
+#ifndef MPOL_F_RELATIVE_NODES
+#define MPOL_F_RELATIVE_NODES (1  14)
+#define MPOL_F_STATIC_NODES   (1  15)
+#endif

 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;
+/*

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

2014-02-19 Thread Igor Mammedov 
On Wed, 19 Feb 2014 10:03:13 +0100
Paolo Bonzini pbonz...@redhat.com 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 hu...@cn.fujitsu.com
  ---
   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;
 }
 
[...]