[Qemu-devel] [PATCH v2] hostmem: no need to check for host_memory_backend_mr_inited() in alloc()

Marc-André Lureau posted 1 patch 7 years, 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20180910134946.16773-1-marcandre.lureau@redhat.com
Test docker-clang@ubuntu failed
Test checkpatch passed
backends/hostmem-file.c  | 24 ++++++++++++------------
backends/hostmem-memfd.c |  4 ----
2 files changed, 12 insertions(+), 16 deletions(-)
[Qemu-devel] [PATCH v2] hostmem: no need to check for host_memory_backend_mr_inited() in alloc()
Posted by Marc-André Lureau 7 years, 1 month ago
memfd_backend_memory_alloc/file_backend_memory_alloc both needlessly
are are calling host_memory_backend_mr_inited() which creates an
illusion that alloc could be called multiple times but it isn't, it's
called once from UserCreatable complete().

Suggested-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 backends/hostmem-file.c  | 24 ++++++++++++------------
 backends/hostmem-memfd.c |  4 ----
 2 files changed, 12 insertions(+), 16 deletions(-)

v2:
 - cosmetic: remove extra indentation in backends/hostmem-file.c

diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
index 2476dcb435..09b21280ce 100644
--- a/backends/hostmem-file.c
+++ b/backends/hostmem-file.c
@@ -42,6 +42,9 @@ static void
 file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
 {
     HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(backend);
+#ifdef CONFIG_LINUX
+    gchar *path;
+#endif
 
     if (!backend->size) {
         error_setg(errp, "can't create backend with size 0");
@@ -54,18 +57,15 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
 #ifndef CONFIG_LINUX
     error_setg(errp, "-mem-path not supported on this host");
 #else
-    if (!host_memory_backend_mr_inited(backend)) {
-        gchar *path;
-        backend->force_prealloc = mem_prealloc;
-        path = object_get_canonical_path(OBJECT(backend));
-        memory_region_init_ram_from_file(&backend->mr, OBJECT(backend),
-                                 path,
-                                 backend->size, fb->align,
-                                 (backend->share ? RAM_SHARED : 0) |
-                                 (fb->is_pmem ? RAM_PMEM : 0),
-                                 fb->mem_path, errp);
-        g_free(path);
-    }
+    backend->force_prealloc = mem_prealloc;
+    path = object_get_canonical_path(OBJECT(backend));
+    memory_region_init_ram_from_file(&backend->mr, OBJECT(backend),
+                                     path,
+                                     backend->size, fb->align,
+                                     (backend->share ? RAM_SHARED : 0) |
+                                     (fb->is_pmem ? RAM_PMEM : 0),
+                                     fb->mem_path, errp);
+    g_free(path);
 #endif
 }
 
diff --git a/backends/hostmem-memfd.c b/backends/hostmem-memfd.c
index abd52364db..7184918112 100644
--- a/backends/hostmem-memfd.c
+++ b/backends/hostmem-memfd.c
@@ -44,10 +44,6 @@ memfd_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
         return;
     }
 
-    if (host_memory_backend_mr_inited(backend)) {
-        return;
-    }
-
     backend->force_prealloc = mem_prealloc;
     fd = qemu_memfd_create(TYPE_MEMORY_BACKEND_MEMFD, backend->size,
                            m->hugetlb, m->hugetlbsize, m->seal ?
-- 
2.19.0.rc1


Re: [Qemu-devel] [PATCH v2] hostmem: no need to check for host_memory_backend_mr_inited() in alloc()
Posted by Igor Mammedov 7 years, 1 month ago
On Mon, 10 Sep 2018 17:49:46 +0400
Marc-André Lureau <marcandre.lureau@redhat.com> wrote:

> memfd_backend_memory_alloc/file_backend_memory_alloc both needlessly
> are are calling host_memory_backend_mr_inited() which creates an
> illusion that alloc could be called multiple times but it isn't, it's
> called once from UserCreatable complete().
> 
> Suggested-by: Igor Mammedov <imammedo@redhat.com>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  backends/hostmem-file.c  | 24 ++++++++++++------------
>  backends/hostmem-memfd.c |  4 ----
>  2 files changed, 12 insertions(+), 16 deletions(-)
> 
> v2:
>  - cosmetic: remove extra indentation in backends/hostmem-file.c
> 
> diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
> index 2476dcb435..09b21280ce 100644
> --- a/backends/hostmem-file.c
> +++ b/backends/hostmem-file.c
> @@ -42,6 +42,9 @@ static void
>  file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
>  {
>      HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(backend);
> +#ifdef CONFIG_LINUX
> +    gchar *path;
> +#endif
>  
>      if (!backend->size) {
>          error_setg(errp, "can't create backend with size 0");
> @@ -54,18 +57,15 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
>  #ifndef CONFIG_LINUX
>      error_setg(errp, "-mem-path not supported on this host");
>  #else
> -    if (!host_memory_backend_mr_inited(backend)) {
> -        gchar *path;
> -        backend->force_prealloc = mem_prealloc;
> -        path = object_get_canonical_path(OBJECT(backend));
> -        memory_region_init_ram_from_file(&backend->mr, OBJECT(backend),
> -                                 path,
> -                                 backend->size, fb->align,
> -                                 (backend->share ? RAM_SHARED : 0) |
> -                                 (fb->is_pmem ? RAM_PMEM : 0),
> -                                 fb->mem_path, errp);
> -        g_free(path);
> -    }
> +    backend->force_prealloc = mem_prealloc;
> +    path = object_get_canonical_path(OBJECT(backend));
> +    memory_region_init_ram_from_file(&backend->mr, OBJECT(backend),
> +                                     path,
> +                                     backend->size, fb->align,
> +                                     (backend->share ? RAM_SHARED : 0) |
> +                                     (fb->is_pmem ? RAM_PMEM : 0),
> +                                     fb->mem_path, errp);
> +    g_free(path);
>  #endif
>  }
>  
> diff --git a/backends/hostmem-memfd.c b/backends/hostmem-memfd.c
> index abd52364db..7184918112 100644
> --- a/backends/hostmem-memfd.c
> +++ b/backends/hostmem-memfd.c
> @@ -44,10 +44,6 @@ memfd_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
>          return;
>      }
>  
> -    if (host_memory_backend_mr_inited(backend)) {
> -        return;
> -    }
> -
>      backend->force_prealloc = mem_prealloc;
>      fd = qemu_memfd_create(TYPE_MEMORY_BACKEND_MEMFD, backend->size,
>                             m->hugetlb, m->hugetlbsize, m->seal ?