From 2d7a1eb6e65d5dd32cb9943b6fb3c81d206cd61f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Mon, 20 Nov 2023 12:59:15 +0100 Subject: backends: Use g_autofree in HostMemoryBackendClass::alloc() handlers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In preparation of having HostMemoryBackendClass::alloc() handlers return a boolean, have them use g_autofree. Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Manos Pitsidianakis Reviewed-by: Gavin Shan Message-Id: <20231120213301.24349-15-philmd@linaro.org> --- backends/hostmem-epc.c | 3 +-- backends/hostmem-file.c | 3 +-- backends/hostmem-memfd.c | 3 +-- backends/hostmem-ram.c | 3 +-- 4 files changed, 4 insertions(+), 8 deletions(-) (limited to 'backends') diff --git a/backends/hostmem-epc.c b/backends/hostmem-epc.c index 4e162d6789..3ceb079f9e 100644 --- a/backends/hostmem-epc.c +++ b/backends/hostmem-epc.c @@ -20,8 +20,8 @@ static void sgx_epc_backend_memory_alloc(HostMemoryBackend *backend, Error **errp) { + g_autofree char *name = NULL; uint32_t ram_flags; - char *name; int fd; if (!backend->size) { @@ -41,7 +41,6 @@ sgx_epc_backend_memory_alloc(HostMemoryBackend *backend, Error **errp) memory_region_init_ram_from_fd(&backend->mr, OBJECT(backend), name, backend->size, ram_flags, fd, 0, errp); - g_free(name); } static void sgx_epc_backend_instance_init(Object *obj) diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c index 361d4a8103..fe8c481f8f 100644 --- a/backends/hostmem-file.c +++ b/backends/hostmem-file.c @@ -44,8 +44,8 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp) object_get_typename(OBJECT(backend))); #else HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(backend); + g_autofree gchar *name = NULL; uint32_t ram_flags; - gchar *name; if (!backend->size) { error_setg(errp, "can't create backend with size 0"); @@ -89,7 +89,6 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp) memory_region_init_ram_from_file(&backend->mr, OBJECT(backend), name, backend->size, fb->align, ram_flags, fb->mem_path, fb->offset, errp); - g_free(name); #endif } diff --git a/backends/hostmem-memfd.c b/backends/hostmem-memfd.c index 3fc85c3db8..db28ab5a56 100644 --- a/backends/hostmem-memfd.c +++ b/backends/hostmem-memfd.c @@ -35,8 +35,8 @@ static void memfd_backend_memory_alloc(HostMemoryBackend *backend, Error **errp) { HostMemoryBackendMemfd *m = MEMORY_BACKEND_MEMFD(backend); + g_autofree char *name = NULL; uint32_t ram_flags; - char *name; int fd; if (!backend->size) { @@ -57,7 +57,6 @@ memfd_backend_memory_alloc(HostMemoryBackend *backend, Error **errp) ram_flags |= backend->reserve ? 0 : RAM_NORESERVE; memory_region_init_ram_from_fd(&backend->mr, OBJECT(backend), name, backend->size, ram_flags, fd, 0, errp); - g_free(name); } static bool diff --git a/backends/hostmem-ram.c b/backends/hostmem-ram.c index b8e55cdbd0..0a670fc22a 100644 --- a/backends/hostmem-ram.c +++ b/backends/hostmem-ram.c @@ -19,8 +19,8 @@ static void ram_backend_memory_alloc(HostMemoryBackend *backend, Error **errp) { + g_autofree char *name = NULL; uint32_t ram_flags; - char *name; if (!backend->size) { error_setg(errp, "can't create backend with size 0"); @@ -32,7 +32,6 @@ ram_backend_memory_alloc(HostMemoryBackend *backend, Error **errp) ram_flags |= backend->reserve ? 0 : RAM_NORESERVE; memory_region_init_ram_flags_nomigrate(&backend->mr, OBJECT(backend), name, backend->size, ram_flags, errp); - g_free(name); } static void -- cgit v1.2.3 From e199f7ad4df8b2c2efad0acdea91b936c130261c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Mon, 20 Nov 2023 13:49:30 +0100 Subject: backends: Simplify host_memory_backend_memory_complete() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Return early if bc->alloc is NULL. De-indent the if() ladder. Note, this avoids a pointless call to error_propagate() with errp=NULL at the 'out:' label. Change trivial when reviewed with 'git-diff --ignore-all-space'. Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Manos Pitsidianakis Reviewed-by: Gavin Shan Message-Id: <20231120213301.24349-16-philmd@linaro.org> --- backends/hostmem.c | 135 +++++++++++++++++++++++++++-------------------------- 1 file changed, 68 insertions(+), 67 deletions(-) (limited to 'backends') diff --git a/backends/hostmem.c b/backends/hostmem.c index 747e7838c0..1723c19165 100644 --- a/backends/hostmem.c +++ b/backends/hostmem.c @@ -328,83 +328,84 @@ host_memory_backend_memory_complete(UserCreatable *uc, Error **errp) void *ptr; uint64_t sz; - if (bc->alloc) { - bc->alloc(backend, &local_err); - if (local_err) { - goto out; - } + if (!bc->alloc) { + return; + } + bc->alloc(backend, &local_err); + if (local_err) { + goto out; + } - ptr = memory_region_get_ram_ptr(&backend->mr); - sz = memory_region_size(&backend->mr); + ptr = memory_region_get_ram_ptr(&backend->mr); + sz = memory_region_size(&backend->mr); - if (backend->merge) { - qemu_madvise(ptr, sz, QEMU_MADV_MERGEABLE); - } - if (!backend->dump) { - qemu_madvise(ptr, sz, QEMU_MADV_DONTDUMP); - } + if (backend->merge) { + qemu_madvise(ptr, sz, QEMU_MADV_MERGEABLE); + } + if (!backend->dump) { + qemu_madvise(ptr, sz, QEMU_MADV_DONTDUMP); + } #ifdef CONFIG_NUMA - unsigned long lastbit = find_last_bit(backend->host_nodes, MAX_NODES); - /* lastbit == MAX_NODES means maxnode = 0 */ - unsigned long maxnode = (lastbit + 1) % (MAX_NODES + 1); - /* ensure policy won't be ignored in case memory is preallocated - * before mbind(). note: MPOL_MF_STRICT is ignored on hugepages so - * this doesn't catch hugepage case. */ - unsigned flags = MPOL_MF_STRICT | MPOL_MF_MOVE; - int mode = backend->policy; - - /* check for invalid host-nodes and policies and give more verbose - * error messages than mbind(). */ - if (maxnode && backend->policy == MPOL_DEFAULT) { - error_setg(errp, "host-nodes must be empty for policy default," - " or you should explicitly specify a policy other" - " than default"); - return; - } else if (maxnode == 0 && backend->policy != MPOL_DEFAULT) { - error_setg(errp, "host-nodes must be set for policy %s", - HostMemPolicy_str(backend->policy)); - return; - } + unsigned long lastbit = find_last_bit(backend->host_nodes, MAX_NODES); + /* lastbit == MAX_NODES means maxnode = 0 */ + unsigned long maxnode = (lastbit + 1) % (MAX_NODES + 1); + /* ensure policy won't be ignored in case memory is preallocated + * before mbind(). note: MPOL_MF_STRICT is ignored on hugepages so + * this doesn't catch hugepage case. */ + unsigned flags = MPOL_MF_STRICT | MPOL_MF_MOVE; + int mode = backend->policy; + + /* check for invalid host-nodes and policies and give more verbose + * error messages than mbind(). */ + if (maxnode && backend->policy == MPOL_DEFAULT) { + error_setg(errp, "host-nodes must be empty for policy default," + " or you should explicitly specify a policy other" + " than default"); + return; + } else if (maxnode == 0 && backend->policy != MPOL_DEFAULT) { + error_setg(errp, "host-nodes must be set for policy %s", + HostMemPolicy_str(backend->policy)); + return; + } - /* We can have up to MAX_NODES nodes, but we need to pass maxnode+1 - * as argument to mbind() due to an old Linux bug (feature?) which - * cuts off the last specified node. This means backend->host_nodes - * must have MAX_NODES+1 bits available. - */ - assert(sizeof(backend->host_nodes) >= - BITS_TO_LONGS(MAX_NODES + 1) * sizeof(unsigned long)); - assert(maxnode <= MAX_NODES); + /* We can have up to MAX_NODES nodes, but we need to pass maxnode+1 + * as argument to mbind() due to an old Linux bug (feature?) which + * cuts off the last specified node. This means backend->host_nodes + * must have MAX_NODES+1 bits available. + */ + assert(sizeof(backend->host_nodes) >= + BITS_TO_LONGS(MAX_NODES + 1) * sizeof(unsigned long)); + assert(maxnode <= MAX_NODES); #ifdef HAVE_NUMA_HAS_PREFERRED_MANY - if (mode == MPOL_PREFERRED && numa_has_preferred_many() > 0) { - /* - * Replace with MPOL_PREFERRED_MANY otherwise the mbind() below - * silently picks the first node. - */ - mode = MPOL_PREFERRED_MANY; - } + if (mode == MPOL_PREFERRED && numa_has_preferred_many() > 0) { + /* + * Replace with MPOL_PREFERRED_MANY otherwise the mbind() below + * silently picks the first node. + */ + mode = MPOL_PREFERRED_MANY; + } #endif - if (maxnode && - mbind(ptr, sz, mode, backend->host_nodes, maxnode + 1, flags)) { - if (backend->policy != MPOL_DEFAULT || errno != ENOSYS) { - error_setg_errno(errp, errno, - "cannot bind memory to host NUMA nodes"); - return; - } + if (maxnode && + mbind(ptr, sz, mode, backend->host_nodes, maxnode + 1, flags)) { + if (backend->policy != MPOL_DEFAULT || errno != ENOSYS) { + error_setg_errno(errp, errno, + "cannot bind memory to host NUMA nodes"); + return; } + } #endif - /* Preallocate memory after the NUMA policy has been instantiated. - * This is necessary to guarantee memory is allocated with - * specified NUMA policy in place. - */ - if (backend->prealloc) { - qemu_prealloc_mem(memory_region_get_fd(&backend->mr), ptr, sz, - backend->prealloc_threads, - backend->prealloc_context, &local_err); - if (local_err) { - goto out; - } + /* Preallocate memory after the NUMA policy has been instantiated. + * This is necessary to guarantee memory is allocated with + * specified NUMA policy in place. + */ + if (backend->prealloc) { + qemu_prealloc_mem(memory_region_get_fd(&backend->mr), ptr, sz, + backend->prealloc_threads, + backend->prealloc_context, &local_err); + if (local_err) { + goto out; } } out: -- cgit v1.2.3 From fdb63cf3b57a230f4cc530ea2515229aeada0998 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Mon, 20 Nov 2023 13:50:52 +0100 Subject: backends: Have HostMemoryBackendClass::alloc() handler return a boolean MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Following the example documented since commit e3fe3988d7 ("error: Document Error API usage rules"), have HostMemoryBackendClass::alloc return a boolean indicating whether an error is set or not. Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Manos Pitsidianakis Reviewed-by: Gavin Shan Message-Id: <20231120213301.24349-17-philmd@linaro.org> --- backends/hostmem-epc.c | 11 +++++------ backends/hostmem-file.c | 19 ++++++++++--------- backends/hostmem-memfd.c | 10 +++++----- backends/hostmem-ram.c | 9 +++++---- backends/hostmem.c | 5 ++--- 5 files changed, 27 insertions(+), 27 deletions(-) (limited to 'backends') diff --git a/backends/hostmem-epc.c b/backends/hostmem-epc.c index 3ceb079f9e..735e2e1cf8 100644 --- a/backends/hostmem-epc.c +++ b/backends/hostmem-epc.c @@ -17,7 +17,7 @@ #include "sysemu/hostmem.h" #include "hw/i386/hostmem-epc.h" -static void +static bool sgx_epc_backend_memory_alloc(HostMemoryBackend *backend, Error **errp) { g_autofree char *name = NULL; @@ -26,21 +26,20 @@ sgx_epc_backend_memory_alloc(HostMemoryBackend *backend, Error **errp) if (!backend->size) { error_setg(errp, "can't create backend with size 0"); - return; + return false; } fd = qemu_open_old("/dev/sgx_vepc", O_RDWR); if (fd < 0) { error_setg_errno(errp, errno, "failed to open /dev/sgx_vepc to alloc SGX EPC"); - return; + return false; } name = object_get_canonical_path(OBJECT(backend)); ram_flags = (backend->share ? RAM_SHARED : 0) | RAM_PROTECTED; - memory_region_init_ram_from_fd(&backend->mr, OBJECT(backend), - name, backend->size, ram_flags, - fd, 0, errp); + return memory_region_init_ram_from_fd(&backend->mr, OBJECT(backend), name, + backend->size, ram_flags, fd, 0, errp); } static void sgx_epc_backend_instance_init(Object *obj) diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c index fe8c481f8f..ac3e433cbd 100644 --- a/backends/hostmem-file.c +++ b/backends/hostmem-file.c @@ -36,12 +36,13 @@ struct HostMemoryBackendFile { OnOffAuto rom; }; -static void +static bool file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp) { #ifndef CONFIG_POSIX error_setg(errp, "backend '%s' not supported on this host", object_get_typename(OBJECT(backend))); + return false; #else HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(backend); g_autofree gchar *name = NULL; @@ -49,11 +50,11 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp) if (!backend->size) { error_setg(errp, "can't create backend with size 0"); - return; + return false; } if (!fb->mem_path) { error_setg(errp, "mem-path property not set"); - return; + return false; } switch (fb->rom) { @@ -65,18 +66,18 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp) if (!fb->readonly) { error_setg(errp, "property 'rom' = 'on' is not supported with" " 'readonly' = 'off'"); - return; + return false; } break; case ON_OFF_AUTO_OFF: if (fb->readonly && backend->share) { error_setg(errp, "property 'rom' = 'off' is incompatible with" " 'readonly' = 'on' and 'share' = 'on'"); - return; + return false; } break; default: - assert(false); + g_assert_not_reached(); } name = host_memory_backend_get_name(backend); @@ -86,9 +87,9 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp) ram_flags |= backend->reserve ? 0 : RAM_NORESERVE; ram_flags |= fb->is_pmem ? RAM_PMEM : 0; ram_flags |= RAM_NAMED_FILE; - memory_region_init_ram_from_file(&backend->mr, OBJECT(backend), name, - backend->size, fb->align, ram_flags, - fb->mem_path, fb->offset, errp); + return memory_region_init_ram_from_file(&backend->mr, OBJECT(backend), name, + backend->size, fb->align, ram_flags, + fb->mem_path, fb->offset, errp); #endif } diff --git a/backends/hostmem-memfd.c b/backends/hostmem-memfd.c index db28ab5a56..3923ea9364 100644 --- a/backends/hostmem-memfd.c +++ b/backends/hostmem-memfd.c @@ -31,7 +31,7 @@ struct HostMemoryBackendMemfd { bool seal; }; -static void +static bool memfd_backend_memory_alloc(HostMemoryBackend *backend, Error **errp) { HostMemoryBackendMemfd *m = MEMORY_BACKEND_MEMFD(backend); @@ -41,7 +41,7 @@ memfd_backend_memory_alloc(HostMemoryBackend *backend, Error **errp) if (!backend->size) { error_setg(errp, "can't create backend with size 0"); - return; + return false; } fd = qemu_memfd_create(TYPE_MEMORY_BACKEND_MEMFD, backend->size, @@ -49,14 +49,14 @@ memfd_backend_memory_alloc(HostMemoryBackend *backend, Error **errp) F_SEAL_GROW | F_SEAL_SHRINK | F_SEAL_SEAL : 0, errp); if (fd == -1) { - return; + return false; } name = host_memory_backend_get_name(backend); ram_flags = backend->share ? RAM_SHARED : 0; ram_flags |= backend->reserve ? 0 : RAM_NORESERVE; - memory_region_init_ram_from_fd(&backend->mr, OBJECT(backend), name, - backend->size, ram_flags, fd, 0, errp); + return memory_region_init_ram_from_fd(&backend->mr, OBJECT(backend), name, + backend->size, ram_flags, fd, 0, errp); } static bool diff --git a/backends/hostmem-ram.c b/backends/hostmem-ram.c index 0a670fc22a..d121249f0f 100644 --- a/backends/hostmem-ram.c +++ b/backends/hostmem-ram.c @@ -16,7 +16,7 @@ #include "qemu/module.h" #include "qom/object_interfaces.h" -static void +static bool ram_backend_memory_alloc(HostMemoryBackend *backend, Error **errp) { g_autofree char *name = NULL; @@ -24,14 +24,15 @@ ram_backend_memory_alloc(HostMemoryBackend *backend, Error **errp) if (!backend->size) { error_setg(errp, "can't create backend with size 0"); - return; + return false; } name = host_memory_backend_get_name(backend); ram_flags = backend->share ? RAM_SHARED : 0; ram_flags |= backend->reserve ? 0 : RAM_NORESERVE; - memory_region_init_ram_flags_nomigrate(&backend->mr, OBJECT(backend), name, - backend->size, ram_flags, errp); + return memory_region_init_ram_flags_nomigrate(&backend->mr, OBJECT(backend), + name, backend->size, + ram_flags, errp); } static void diff --git a/backends/hostmem.c b/backends/hostmem.c index 1723c19165..3f8eb936d7 100644 --- a/backends/hostmem.c +++ b/backends/hostmem.c @@ -331,9 +331,8 @@ host_memory_backend_memory_complete(UserCreatable *uc, Error **errp) if (!bc->alloc) { return; } - bc->alloc(backend, &local_err); - if (local_err) { - goto out; + if (!bc->alloc(backend, errp)) { + return; } ptr = memory_region_get_ram_ptr(&backend->mr); -- cgit v1.2.3 From 3961613a760766921cdd6423ced0f801c2a6f1fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Mon, 20 Nov 2023 15:16:38 +0100 Subject: backends: Reduce variable scope in host_memory_backend_memory_complete MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reduce the &local_err variable use and remove the 'out:' label. Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Manos Pitsidianakis Reviewed-by: Gavin Shan Message-Id: <20231120213301.24349-18-philmd@linaro.org> --- backends/hostmem.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'backends') diff --git a/backends/hostmem.c b/backends/hostmem.c index 3f8eb936d7..1b0043a0d9 100644 --- a/backends/hostmem.c +++ b/backends/hostmem.c @@ -324,7 +324,6 @@ host_memory_backend_memory_complete(UserCreatable *uc, Error **errp) { HostMemoryBackend *backend = MEMORY_BACKEND(uc); HostMemoryBackendClass *bc = MEMORY_BACKEND_GET_CLASS(uc); - Error *local_err = NULL; void *ptr; uint64_t sz; @@ -400,15 +399,16 @@ host_memory_backend_memory_complete(UserCreatable *uc, Error **errp) * specified NUMA policy in place. */ if (backend->prealloc) { + Error *local_err = NULL; + qemu_prealloc_mem(memory_region_get_fd(&backend->mr), ptr, sz, backend->prealloc_threads, backend->prealloc_context, &local_err); if (local_err) { - goto out; + error_propagate(errp, local_err); + return; } } -out: - error_propagate(errp, local_err); } static bool -- cgit v1.2.3 From 9c878ad6fbfc7934c9c4f6ba607c0842202afdbb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Mon, 20 Nov 2023 15:32:57 +0100 Subject: misc: Simplify qemu_prealloc_mem() calls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since qemu_prealloc_mem() returns whether or not an error occured, we don't need to check the @errp pointer. Remove local_err uses when we can return directly. Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Manos Pitsidianakis Reviewed-by: Gavin Shan Message-Id: <20231120213301.24349-20-philmd@linaro.org> --- backends/hostmem.c | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-) (limited to 'backends') diff --git a/backends/hostmem.c b/backends/hostmem.c index 1b0043a0d9..30f69b2cb5 100644 --- a/backends/hostmem.c +++ b/backends/hostmem.c @@ -219,7 +219,6 @@ static bool host_memory_backend_get_prealloc(Object *obj, Error **errp) static void host_memory_backend_set_prealloc(Object *obj, bool value, Error **errp) { - Error *local_err = NULL; HostMemoryBackend *backend = MEMORY_BACKEND(obj); if (!backend->reserve && value) { @@ -237,10 +236,8 @@ 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); - qemu_prealloc_mem(fd, ptr, sz, backend->prealloc_threads, - backend->prealloc_context, &local_err); - if (local_err) { - error_propagate(errp, local_err); + if (!qemu_prealloc_mem(fd, ptr, sz, backend->prealloc_threads, + backend->prealloc_context, errp)) { return; } backend->prealloc = true; @@ -398,16 +395,11 @@ host_memory_backend_memory_complete(UserCreatable *uc, Error **errp) * This is necessary to guarantee memory is allocated with * specified NUMA policy in place. */ - if (backend->prealloc) { - Error *local_err = NULL; - - qemu_prealloc_mem(memory_region_get_fd(&backend->mr), ptr, sz, - backend->prealloc_threads, - backend->prealloc_context, &local_err); - if (local_err) { - error_propagate(errp, local_err); - return; - } + if (backend->prealloc && !qemu_prealloc_mem(memory_region_get_fd(&backend->mr), + ptr, sz, + backend->prealloc_threads, + backend->prealloc_context, errp)) { + return; } } -- cgit v1.2.3