diff mbox series

[REPOST,v3,77/80] hostmem: introduce "prealloc-threads" property

Message ID 1579779525-20065-78-git-send-email-imammedo@redhat.com
State New
Headers show
Series refactor main RAM allocation to use hostmem backend | expand

Commit Message

Igor Mammedov Jan. 23, 2020, 11:38 a.m. UTC
the property will allow user to specify number of threads to use
in pre-allocation stage. It also will allow to reduce implicit
hostmem dependency on current_machine.
On object creation it will default to 1, but via machine
compat property it will be updated to MachineState::smp::cpus
to keep current behavior for hostmem and main RAM (which is
now also hostmem based).

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v3:
  - use object_register_sugar_prop() instead of directly hacking
    compat_props (Paolo Bonzini <pbonzini@redhat.com>)
  - fix TODO description

CC: pbonzini@redhat.com
CC: ehabkost@redhat.com
---
 include/sysemu/hostmem.h |  2 ++
 backends/hostmem.c       | 43 +++++++++++++++++++++++++++++++++++++++----
 vl.c                     | 14 ++++++++++----
 3 files changed, 51 insertions(+), 8 deletions(-)

Comments

Igor Mammedov Jan. 24, 2020, 9:50 a.m. UTC | #1
On Thu, 23 Jan 2020 12:38:42 +0100
Igor Mammedov <imammedo@redhat.com> wrote:

> the property will allow user to specify number of threads to use
> in pre-allocation stage. It also will allow to reduce implicit
> hostmem dependency on current_machine.
> On object creation it will default to 1, but via machine
> compat property it will be updated to MachineState::smp::cpus
> to keep current behavior for hostmem and main RAM (which is
> now also hostmem based).
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>

Jitendra,

As the one who introduced smp_cpus in backend

(1e356fc14be mem-prealloc: reduce large guest start-up and migration time.)

could you review this patch please?

> ---
> v3:
>   - use object_register_sugar_prop() instead of directly hacking
>     compat_props (Paolo Bonzini <pbonzini@redhat.com>)
>   - fix TODO description
> 
> CC: pbonzini@redhat.com
> CC: ehabkost@redhat.com
> ---
>  include/sysemu/hostmem.h |  2 ++
>  backends/hostmem.c       | 43 +++++++++++++++++++++++++++++++++++++++----
>  vl.c                     | 14 ++++++++++----
>  3 files changed, 51 insertions(+), 8 deletions(-)
> 
> diff --git a/include/sysemu/hostmem.h b/include/sysemu/hostmem.h
> index 5db0d66..bdf8666 100644
> --- a/include/sysemu/hostmem.h
> +++ b/include/sysemu/hostmem.h
> @@ -61,6 +61,7 @@ struct HostMemoryBackendClass {
>   * @parent: opaque parent object container
>   * @size: amount of memory backend provides
>   * @mr: MemoryRegion representing host memory belonging to backend
> + * @prealloc_threads: number of threads to be used for preallocatining RAM
>   */
>  struct HostMemoryBackend {
>      /* private */
> @@ -70,6 +71,7 @@ struct HostMemoryBackend {
>      uint64_t size;
>      bool merge, dump, use_canonical_path;
>      bool prealloc, force_prealloc, is_mapped, share;
> +    uint32_t prealloc_threads;
>      DECLARE_BITMAP(host_nodes, MAX_NODES + 1);
>      HostMemPolicy policy;
>  
> diff --git a/backends/hostmem.c b/backends/hostmem.c
> index e773bdf..0988986 100644
> --- a/backends/hostmem.c
> +++ b/backends/hostmem.c
> @@ -223,7 +223,6 @@ static void host_memory_backend_set_prealloc(Object *obj, bool value,
>  {
>      Error *local_err = NULL;
>      HostMemoryBackend *backend = MEMORY_BACKEND(obj);
> -    MachineState *ms = MACHINE(qdev_get_machine());
>  
>      if (backend->force_prealloc) {
>          if (value) {
> @@ -243,7 +242,7 @@ static void host_memory_backend_set_prealloc(Object *obj, bool value,
>          void *ptr = memory_region_get_ram_ptr(&backend->mr);
>          uint64_t sz = memory_region_size(&backend->mr);
>  
> -        os_mem_prealloc(fd, ptr, sz, ms->smp.cpus, &local_err);
> +        os_mem_prealloc(fd, ptr, sz, backend->prealloc_threads, &local_err);
>          if (local_err) {
>              error_propagate(errp, local_err);
>              return;
> @@ -252,14 +251,45 @@ static void host_memory_backend_set_prealloc(Object *obj, bool value,
>      }
>  }
>  
> +static void host_memory_backend_get_prealloc_threads(Object *obj, Visitor *v,
> +    const char *name, void *opaque, Error **errp)
> +{
> +    HostMemoryBackend *backend = MEMORY_BACKEND(obj);
> +    visit_type_uint32(v, name, &backend->prealloc_threads, errp);
> +}
> +
> +static void host_memory_backend_set_prealloc_threads(Object *obj, Visitor *v,
> +    const char *name, void *opaque, Error **errp)
> +{
> +    HostMemoryBackend *backend = MEMORY_BACKEND(obj);
> +    Error *local_err = NULL;
> +    uint32_t value;
> +
> +    visit_type_uint32(v, name, &value, &local_err);
> +    if (local_err) {
> +        goto out;
> +    }
> +    if (value <= 0) {
> +        error_setg(&local_err,
> +                   "property '%s' of %s doesn't take value '%d'",
> +                   name, object_get_typename(obj), value);
> +        goto out;
> +    }
> +    backend->prealloc_threads = value;
> +out:
> +    error_propagate(errp, local_err);
> +}
> +
>  static void host_memory_backend_init(Object *obj)
>  {
>      HostMemoryBackend *backend = MEMORY_BACKEND(obj);
>      MachineState *machine = MACHINE(qdev_get_machine());
>  
> +    /* TODO: convert access to globals to compat properties */
>      backend->merge = machine_mem_merge(machine);
>      backend->dump = machine_dump_guest_core(machine);
>      backend->prealloc = mem_prealloc;
> +    backend->prealloc_threads = 1;
>  }
>  
>  static void host_memory_backend_post_init(Object *obj)
> @@ -313,7 +343,6 @@ host_memory_backend_memory_complete(UserCreatable *uc, Error **errp)
>  {
>      HostMemoryBackend *backend = MEMORY_BACKEND(uc);
>      HostMemoryBackendClass *bc = MEMORY_BACKEND_GET_CLASS(uc);
> -    MachineState *ms = MACHINE(qdev_get_machine());
>      Error *local_err = NULL;
>      void *ptr;
>      uint64_t sz;
> @@ -378,7 +407,7 @@ host_memory_backend_memory_complete(UserCreatable *uc, Error **errp)
>           */
>          if (backend->prealloc) {
>              os_mem_prealloc(memory_region_get_fd(&backend->mr), ptr, sz,
> -                            ms->smp.cpus, &local_err);
> +                            backend->prealloc_threads, &local_err);
>              if (local_err) {
>                  goto out;
>              }
> @@ -456,6 +485,12 @@ host_memory_backend_class_init(ObjectClass *oc, void *data)
>          host_memory_backend_set_prealloc, &error_abort);
>      object_class_property_set_description(oc, "prealloc",
>          "Preallocate memory", &error_abort);
> +    object_class_property_add(oc, "prealloc-threads", "int",
> +        host_memory_backend_get_prealloc_threads,
> +        host_memory_backend_set_prealloc_threads,
> +        NULL, NULL, &error_abort);
> +    object_class_property_set_description(oc, "prealloc-threads",
> +        "Number of CPU threads to use for prealloc", &error_abort);
>      object_class_property_add(oc, "size", "int",
>          host_memory_backend_get_size,
>          host_memory_backend_set_size,
> diff --git a/vl.c b/vl.c
> index d825a34..21e3262 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2822,8 +2822,7 @@ static void configure_accelerators(const char *progname)
>      }
>  }
>  
> -static void create_default_memdev(MachineState *ms, const char *path,
> -                                  bool prealloc)
> +static void create_default_memdev(MachineState *ms, const char *path)
>  {
>      Object *obj;
>      MachineClass *mc = MACHINE_GET_CLASS(ms);
> @@ -2832,7 +2831,6 @@ static void create_default_memdev(MachineState *ms, const char *path,
>      if (path) {
>          object_property_set_str(obj, path, "mem-path", &error_fatal);
>      }
> -    object_property_set_bool(obj, prealloc, "prealloc", &error_fatal);
>      object_property_set_int(obj, ms->ram_size, "size", &error_fatal);
>      object_property_add_child(object_get_objects_root(), mc->default_ram_id,
>                                obj, &error_fatal);
> @@ -3975,6 +3973,14 @@ int main(int argc, char **argv, char **envp)
>          exit(1);
>      }
>  
> +    if (mem_prealloc) {
> +        char *val;
> +
> +        val = g_strdup_printf("%d", current_machine->smp.cpus);
> +        object_register_sugar_prop("memory-backend", "prealloc-threads", val);
> +        g_free(val);
> +    }
> +
>      /*
>       * Get the default machine options from the machine if it is not already
>       * specified either by the configuration file or by the command line.
> @@ -4309,7 +4315,7 @@ int main(int argc, char **argv, char **envp)
>          machine_class->default_ram_size &&
>          machine_class->default_ram_id &&
>          !current_machine->ram_memdev) {
> -        create_default_memdev(current_machine, mem_path, mem_prealloc);
> +        create_default_memdev(current_machine, mem_path);
>      }
>      /* do monitor/qmp handling at preconfig state if requested */
>      main_loop();
diff mbox series

Patch

diff --git a/include/sysemu/hostmem.h b/include/sysemu/hostmem.h
index 5db0d66..bdf8666 100644
--- a/include/sysemu/hostmem.h
+++ b/include/sysemu/hostmem.h
@@ -61,6 +61,7 @@  struct HostMemoryBackendClass {
  * @parent: opaque parent object container
  * @size: amount of memory backend provides
  * @mr: MemoryRegion representing host memory belonging to backend
+ * @prealloc_threads: number of threads to be used for preallocatining RAM
  */
 struct HostMemoryBackend {
     /* private */
@@ -70,6 +71,7 @@  struct HostMemoryBackend {
     uint64_t size;
     bool merge, dump, use_canonical_path;
     bool prealloc, force_prealloc, is_mapped, share;
+    uint32_t prealloc_threads;
     DECLARE_BITMAP(host_nodes, MAX_NODES + 1);
     HostMemPolicy policy;
 
diff --git a/backends/hostmem.c b/backends/hostmem.c
index e773bdf..0988986 100644
--- a/backends/hostmem.c
+++ b/backends/hostmem.c
@@ -223,7 +223,6 @@  static void host_memory_backend_set_prealloc(Object *obj, bool value,
 {
     Error *local_err = NULL;
     HostMemoryBackend *backend = MEMORY_BACKEND(obj);
-    MachineState *ms = MACHINE(qdev_get_machine());
 
     if (backend->force_prealloc) {
         if (value) {
@@ -243,7 +242,7 @@  static void host_memory_backend_set_prealloc(Object *obj, bool value,
         void *ptr = memory_region_get_ram_ptr(&backend->mr);
         uint64_t sz = memory_region_size(&backend->mr);
 
-        os_mem_prealloc(fd, ptr, sz, ms->smp.cpus, &local_err);
+        os_mem_prealloc(fd, ptr, sz, backend->prealloc_threads, &local_err);
         if (local_err) {
             error_propagate(errp, local_err);
             return;
@@ -252,14 +251,45 @@  static void host_memory_backend_set_prealloc(Object *obj, bool value,
     }
 }
 
+static void host_memory_backend_get_prealloc_threads(Object *obj, Visitor *v,
+    const char *name, void *opaque, Error **errp)
+{
+    HostMemoryBackend *backend = MEMORY_BACKEND(obj);
+    visit_type_uint32(v, name, &backend->prealloc_threads, errp);
+}
+
+static void host_memory_backend_set_prealloc_threads(Object *obj, Visitor *v,
+    const char *name, void *opaque, Error **errp)
+{
+    HostMemoryBackend *backend = MEMORY_BACKEND(obj);
+    Error *local_err = NULL;
+    uint32_t value;
+
+    visit_type_uint32(v, name, &value, &local_err);
+    if (local_err) {
+        goto out;
+    }
+    if (value <= 0) {
+        error_setg(&local_err,
+                   "property '%s' of %s doesn't take value '%d'",
+                   name, object_get_typename(obj), value);
+        goto out;
+    }
+    backend->prealloc_threads = value;
+out:
+    error_propagate(errp, local_err);
+}
+
 static void host_memory_backend_init(Object *obj)
 {
     HostMemoryBackend *backend = MEMORY_BACKEND(obj);
     MachineState *machine = MACHINE(qdev_get_machine());
 
+    /* TODO: convert access to globals to compat properties */
     backend->merge = machine_mem_merge(machine);
     backend->dump = machine_dump_guest_core(machine);
     backend->prealloc = mem_prealloc;
+    backend->prealloc_threads = 1;
 }
 
 static void host_memory_backend_post_init(Object *obj)
@@ -313,7 +343,6 @@  host_memory_backend_memory_complete(UserCreatable *uc, Error **errp)
 {
     HostMemoryBackend *backend = MEMORY_BACKEND(uc);
     HostMemoryBackendClass *bc = MEMORY_BACKEND_GET_CLASS(uc);
-    MachineState *ms = MACHINE(qdev_get_machine());
     Error *local_err = NULL;
     void *ptr;
     uint64_t sz;
@@ -378,7 +407,7 @@  host_memory_backend_memory_complete(UserCreatable *uc, Error **errp)
          */
         if (backend->prealloc) {
             os_mem_prealloc(memory_region_get_fd(&backend->mr), ptr, sz,
-                            ms->smp.cpus, &local_err);
+                            backend->prealloc_threads, &local_err);
             if (local_err) {
                 goto out;
             }
@@ -456,6 +485,12 @@  host_memory_backend_class_init(ObjectClass *oc, void *data)
         host_memory_backend_set_prealloc, &error_abort);
     object_class_property_set_description(oc, "prealloc",
         "Preallocate memory", &error_abort);
+    object_class_property_add(oc, "prealloc-threads", "int",
+        host_memory_backend_get_prealloc_threads,
+        host_memory_backend_set_prealloc_threads,
+        NULL, NULL, &error_abort);
+    object_class_property_set_description(oc, "prealloc-threads",
+        "Number of CPU threads to use for prealloc", &error_abort);
     object_class_property_add(oc, "size", "int",
         host_memory_backend_get_size,
         host_memory_backend_set_size,
diff --git a/vl.c b/vl.c
index d825a34..21e3262 100644
--- a/vl.c
+++ b/vl.c
@@ -2822,8 +2822,7 @@  static void configure_accelerators(const char *progname)
     }
 }
 
-static void create_default_memdev(MachineState *ms, const char *path,
-                                  bool prealloc)
+static void create_default_memdev(MachineState *ms, const char *path)
 {
     Object *obj;
     MachineClass *mc = MACHINE_GET_CLASS(ms);
@@ -2832,7 +2831,6 @@  static void create_default_memdev(MachineState *ms, const char *path,
     if (path) {
         object_property_set_str(obj, path, "mem-path", &error_fatal);
     }
-    object_property_set_bool(obj, prealloc, "prealloc", &error_fatal);
     object_property_set_int(obj, ms->ram_size, "size", &error_fatal);
     object_property_add_child(object_get_objects_root(), mc->default_ram_id,
                               obj, &error_fatal);
@@ -3975,6 +3973,14 @@  int main(int argc, char **argv, char **envp)
         exit(1);
     }
 
+    if (mem_prealloc) {
+        char *val;
+
+        val = g_strdup_printf("%d", current_machine->smp.cpus);
+        object_register_sugar_prop("memory-backend", "prealloc-threads", val);
+        g_free(val);
+    }
+
     /*
      * Get the default machine options from the machine if it is not already
      * specified either by the configuration file or by the command line.
@@ -4309,7 +4315,7 @@  int main(int argc, char **argv, char **envp)
         machine_class->default_ram_size &&
         machine_class->default_ram_id &&
         !current_machine->ram_memdev) {
-        create_default_memdev(current_machine, mem_path, mem_prealloc);
+        create_default_memdev(current_machine, mem_path);
     }
     /* do monitor/qmp handling at preconfig state if requested */
     main_loop();