diff mbox

[11/15] qom: use mmap for bigger Objects

Message ID 1467104499-27517-12-git-send-email-pl@kamp.de (mailing list archive)
State New, archived
Headers show

Commit Message

Peter Lieven June 28, 2016, 9:01 a.m. UTC
Signed-off-by: Peter Lieven <pl@kamp.de>
---
 include/qom/object.h |  1 +
 qom/object.c         | 20 +++++++++++++++++---
 2 files changed, 18 insertions(+), 3 deletions(-)

Comments

Daniel P. Berrangé June 28, 2016, 10:08 a.m. UTC | #1
On Tue, Jun 28, 2016 at 11:01:35AM +0200, Peter Lieven wrote:
> Signed-off-by: Peter Lieven <pl@kamp.de>
> ---
>  include/qom/object.h |  1 +
>  qom/object.c         | 20 +++++++++++++++++---
>  2 files changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/include/qom/object.h b/include/qom/object.h
> index 2f8ac47..c612f3a 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -400,6 +400,7 @@ struct Object
>      GHashTable *properties;
>      uint32_t ref;
>      Object *parent;
> +    size_t instance_size;

This is not required....

>  };
>  
>  /**
> diff --git a/qom/object.c b/qom/object.c
> index 9743ea4..203162b 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -15,6 +15,7 @@
>  #include "qom/object.h"
>  #include "qom/object_interfaces.h"
>  #include "qemu/cutils.h"
> +#include "qemu/mmap-alloc.h"
>  #include "qapi/visitor.h"
>  #include "qapi-visit.h"
>  #include "qapi/string-input-visitor.h"
> @@ -453,6 +454,12 @@ static void object_deinit(Object *obj, TypeImpl *type)
>      }
>  }
>  
> +static void object_munmap(void *opaque)
> +{
> +    Object *obj = opaque;
> +    qemu_anon_ram_munmap(obj, obj->instance_size);

...since you have an Object pointer, you can get the corresponding
Type,  obj->class->type, and thus directly access type->instance_size

> +}
> +
>  static void object_finalize(void *data)
>  {
>      Object *obj = data;
> @@ -467,16 +474,23 @@ static void object_finalize(void *data)
>      }
>  }
>  
> +#define OBJECT_MMAP_THRESH 4096
> +
>  Object *object_new_with_type(Type type)
>  {
>      Object *obj;
>  
>      g_assert(type != NULL);
>      type_initialize(type);
> -
> -    obj = g_malloc(type->instance_size);
> +    if (type->instance_size < OBJECT_MMAP_THRESH) {
> +        obj = g_malloc(type->instance_size);
> +        obj->free = g_free;
> +    } else {
> +        obj = qemu_anon_ram_mmap(type->instance_size);
> +        obj->free = object_munmap;
> +    }
> +    obj->instance_size = type->instance_size;
>      object_initialize_with_type(obj, type->instance_size, type);
> -    obj->free = g_free;
>  
>      return obj;


Regards,
Daniel
Peter Maydell June 28, 2016, 10:10 a.m. UTC | #2
On 28 June 2016 at 10:01, Peter Lieven <pl@kamp.de> wrote:
> Signed-off-by: Peter Lieven <pl@kamp.de>
> ---
>  include/qom/object.h |  1 +
>  qom/object.c         | 20 +++++++++++++++++---
>  2 files changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/include/qom/object.h b/include/qom/object.h
> index 2f8ac47..c612f3a 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -400,6 +400,7 @@ struct Object
>      GHashTable *properties;
>      uint32_t ref;
>      Object *parent;
> +    size_t instance_size;
>  };
>
>  /**
> diff --git a/qom/object.c b/qom/object.c
> index 9743ea4..203162b 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -15,6 +15,7 @@
>  #include "qom/object.h"
>  #include "qom/object_interfaces.h"
>  #include "qemu/cutils.h"
> +#include "qemu/mmap-alloc.h"
>  #include "qapi/visitor.h"
>  #include "qapi-visit.h"
>  #include "qapi/string-input-visitor.h"
> @@ -453,6 +454,12 @@ static void object_deinit(Object *obj, TypeImpl *type)
>      }
>  }
>
> +static void object_munmap(void *opaque)
> +{
> +    Object *obj = opaque;
> +    qemu_anon_ram_munmap(obj, obj->instance_size);
> +}
> +
>  static void object_finalize(void *data)
>  {
>      Object *obj = data;
> @@ -467,16 +474,23 @@ static void object_finalize(void *data)
>      }
>  }
>
> +#define OBJECT_MMAP_THRESH 4096
> +
>  Object *object_new_with_type(Type type)
>  {
>      Object *obj;
>
>      g_assert(type != NULL);
>      type_initialize(type);
> -
> -    obj = g_malloc(type->instance_size);
> +    if (type->instance_size < OBJECT_MMAP_THRESH) {
> +        obj = g_malloc(type->instance_size);
> +        obj->free = g_free;
> +    } else {
> +        obj = qemu_anon_ram_mmap(type->instance_size);
> +        obj->free = object_munmap;
> +    }
> +    obj->instance_size = type->instance_size;
>      object_initialize_with_type(obj, type->instance_size, type);
> -    obj->free = g_free;

This one I disagree with. We should trust the platform malloc
implementation (or g_malloc() in this case), or get it fixed
if it is broken somehow.

thanks
-- PMM
Peter Lieven June 28, 2016, 10:19 a.m. UTC | #3
Am 28.06.2016 um 12:10 schrieb Peter Maydell:
> On 28 June 2016 at 10:01, Peter Lieven <pl@kamp.de> wrote:
>> Signed-off-by: Peter Lieven <pl@kamp.de>
>> ---
>>   include/qom/object.h |  1 +
>>   qom/object.c         | 20 +++++++++++++++++---
>>   2 files changed, 18 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/qom/object.h b/include/qom/object.h
>> index 2f8ac47..c612f3a 100644
>> --- a/include/qom/object.h
>> +++ b/include/qom/object.h
>> @@ -400,6 +400,7 @@ struct Object
>>       GHashTable *properties;
>>       uint32_t ref;
>>       Object *parent;
>> +    size_t instance_size;
>>   };
>>
>>   /**
>> diff --git a/qom/object.c b/qom/object.c
>> index 9743ea4..203162b 100644
>> --- a/qom/object.c
>> +++ b/qom/object.c
>> @@ -15,6 +15,7 @@
>>   #include "qom/object.h"
>>   #include "qom/object_interfaces.h"
>>   #include "qemu/cutils.h"
>> +#include "qemu/mmap-alloc.h"
>>   #include "qapi/visitor.h"
>>   #include "qapi-visit.h"
>>   #include "qapi/string-input-visitor.h"
>> @@ -453,6 +454,12 @@ static void object_deinit(Object *obj, TypeImpl *type)
>>       }
>>   }
>>
>> +static void object_munmap(void *opaque)
>> +{
>> +    Object *obj = opaque;
>> +    qemu_anon_ram_munmap(obj, obj->instance_size);
>> +}
>> +
>>   static void object_finalize(void *data)
>>   {
>>       Object *obj = data;
>> @@ -467,16 +474,23 @@ static void object_finalize(void *data)
>>       }
>>   }
>>
>> +#define OBJECT_MMAP_THRESH 4096
>> +
>>   Object *object_new_with_type(Type type)
>>   {
>>       Object *obj;
>>
>>       g_assert(type != NULL);
>>       type_initialize(type);
>> -
>> -    obj = g_malloc(type->instance_size);
>> +    if (type->instance_size < OBJECT_MMAP_THRESH) {
>> +        obj = g_malloc(type->instance_size);
>> +        obj->free = g_free;
>> +    } else {
>> +        obj = qemu_anon_ram_mmap(type->instance_size);
>> +        obj->free = object_munmap;
>> +    }
>> +    obj->instance_size = type->instance_size;
>>       object_initialize_with_type(obj, type->instance_size, type);
>> -    obj->free = g_free;
> This one I disagree with. We should trust the platform malloc
> implementation (or g_malloc() in this case), or get it fixed
> if it is broken somehow.

The ptmalloc implementation can only return memory back to the
kernel if there is unallocated memory at the end of the heap. Every
hole in between cannot be returned.

But I agree I would appreciate if malloc would handle this.

Some Objects we allocate are very large. E.g. some are 70kB as far as
I remember.

Peter
Paolo Bonzini June 28, 2016, 10:42 a.m. UTC | #4
On 28/06/2016 11:01, Peter Lieven wrote:
> Signed-off-by: Peter Lieven <pl@kamp.de>
> ---
>  include/qom/object.h |  1 +
>  qom/object.c         | 20 +++++++++++++++++---
>  2 files changed, 18 insertions(+), 3 deletions(-)

No, please---glibc should be fixed instead.

Paolo

> diff --git a/include/qom/object.h b/include/qom/object.h
> index 2f8ac47..c612f3a 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -400,6 +400,7 @@ struct Object
>      GHashTable *properties;
>      uint32_t ref;
>      Object *parent;
> +    size_t instance_size;
>  };
>  
>  /**
> diff --git a/qom/object.c b/qom/object.c
> index 9743ea4..203162b 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -15,6 +15,7 @@
>  #include "qom/object.h"
>  #include "qom/object_interfaces.h"
>  #include "qemu/cutils.h"
> +#include "qemu/mmap-alloc.h"
>  #include "qapi/visitor.h"
>  #include "qapi-visit.h"
>  #include "qapi/string-input-visitor.h"
> @@ -453,6 +454,12 @@ static void object_deinit(Object *obj, TypeImpl *type)
>      }
>  }
>  
> +static void object_munmap(void *opaque)
> +{
> +    Object *obj = opaque;
> +    qemu_anon_ram_munmap(obj, obj->instance_size);
> +}
> +
>  static void object_finalize(void *data)
>  {
>      Object *obj = data;
> @@ -467,16 +474,23 @@ static void object_finalize(void *data)
>      }
>  }
>  
> +#define OBJECT_MMAP_THRESH 4096
> +
>  Object *object_new_with_type(Type type)
>  {
>      Object *obj;
>  
>      g_assert(type != NULL);
>      type_initialize(type);
> -
> -    obj = g_malloc(type->instance_size);
> +    if (type->instance_size < OBJECT_MMAP_THRESH) {
> +        obj = g_malloc(type->instance_size);
> +        obj->free = g_free;
> +    } else {
> +        obj = qemu_anon_ram_mmap(type->instance_size);
> +        obj->free = object_munmap;
> +    }
> +    obj->instance_size = type->instance_size;
>      object_initialize_with_type(obj, type->instance_size, type);
> -    obj->free = g_free;
>  
>      return obj;
>  }
>
Peter Lieven June 28, 2016, 10:49 a.m. UTC | #5
Am 28.06.2016 um 12:42 schrieb Paolo Bonzini:
>
> On 28/06/2016 11:01, Peter Lieven wrote:
>> Signed-off-by: Peter Lieven <pl@kamp.de>
>> ---
>>   include/qom/object.h |  1 +
>>   qom/object.c         | 20 +++++++++++++++++---
>>   2 files changed, 18 insertions(+), 3 deletions(-)
> No, please---glibc should be fixed instead.

The objects we allocate are sometimes as big as 70kB...

Peter
Markus Armbruster June 30, 2016, 2:15 p.m. UTC | #6
Peter Lieven <pl@kamp.de> writes:

> Am 28.06.2016 um 12:42 schrieb Paolo Bonzini:
>>
>> On 28/06/2016 11:01, Peter Lieven wrote:
>>> Signed-off-by: Peter Lieven <pl@kamp.de>
>>> ---
>>>   include/qom/object.h |  1 +
>>>   qom/object.c         | 20 +++++++++++++++++---
>>>   2 files changed, 18 insertions(+), 3 deletions(-)
>> No, please---glibc should be fixed instead.
>
> The objects we allocate are sometimes as big as 70kB...

That's a rather pedestrian size, isn't it?  If malloc() sucks at
managing such sizes, it needs fixing very badly.
diff mbox

Patch

diff --git a/include/qom/object.h b/include/qom/object.h
index 2f8ac47..c612f3a 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -400,6 +400,7 @@  struct Object
     GHashTable *properties;
     uint32_t ref;
     Object *parent;
+    size_t instance_size;
 };
 
 /**
diff --git a/qom/object.c b/qom/object.c
index 9743ea4..203162b 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -15,6 +15,7 @@ 
 #include "qom/object.h"
 #include "qom/object_interfaces.h"
 #include "qemu/cutils.h"
+#include "qemu/mmap-alloc.h"
 #include "qapi/visitor.h"
 #include "qapi-visit.h"
 #include "qapi/string-input-visitor.h"
@@ -453,6 +454,12 @@  static void object_deinit(Object *obj, TypeImpl *type)
     }
 }
 
+static void object_munmap(void *opaque)
+{
+    Object *obj = opaque;
+    qemu_anon_ram_munmap(obj, obj->instance_size);
+}
+
 static void object_finalize(void *data)
 {
     Object *obj = data;
@@ -467,16 +474,23 @@  static void object_finalize(void *data)
     }
 }
 
+#define OBJECT_MMAP_THRESH 4096
+
 Object *object_new_with_type(Type type)
 {
     Object *obj;
 
     g_assert(type != NULL);
     type_initialize(type);
-
-    obj = g_malloc(type->instance_size);
+    if (type->instance_size < OBJECT_MMAP_THRESH) {
+        obj = g_malloc(type->instance_size);
+        obj->free = g_free;
+    } else {
+        obj = qemu_anon_ram_mmap(type->instance_size);
+        obj->free = object_munmap;
+    }
+    obj->instance_size = type->instance_size;
     object_initialize_with_type(obj, type->instance_size, type);
-    obj->free = g_free;
 
     return obj;
 }