Message ID | 20180830175019.16939-1-marcandre.lureau@redhat.com |
---|---|
State | New |
Headers | show |
Series | hostmem: no need to check for host_memory_backend_mr_inited() in alloc() | expand |
On Thu, Aug 30, 2018 at 07:50:19PM +0200, Marc-André Lureau 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> If calling the function multiple times is a mistake, should we replace the checks with: assert(!host_memory_backend_mr_inited(backend)); ? > --- > backends/hostmem-file.c | 2 +- > backends/hostmem-memfd.c | 4 ---- > 2 files changed, 1 insertion(+), 5 deletions(-) > > diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c > index 2476dcb435..5cd5fa75a7 100644 > --- a/backends/hostmem-file.c > +++ b/backends/hostmem-file.c > @@ -54,7 +54,7 @@ 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)); > 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.rc0.48.gb9dfa238d5 >
On Thu, 30 Aug 2018 16:09:58 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote: > On Thu, Aug 30, 2018 at 07:50:19PM +0200, Marc-André Lureau 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(). could you extend this patch to cover file_backend_memory_alloc() as well? Otherwise this logic would be copy-pasted again in the future. > > Suggested-by: Igor Mammedov <imammedo@redhat.com> > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > > If calling the function multiple times is a mistake, should we > replace the checks with: > assert(!host_memory_backend_mr_inited(backend)); > ? it is confusing calling it where it's not necessary, hence this cleanup. Considering that HostMemoryBackendClass::alloc is internal bussines of hostmem backends, assert() would just add clutter. > > --- > > backends/hostmem-file.c | 2 +- > > backends/hostmem-memfd.c | 4 ---- > > 2 files changed, 1 insertion(+), 5 deletions(-) > > > > diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c > > index 2476dcb435..5cd5fa75a7 100644 > > --- a/backends/hostmem-file.c > > +++ b/backends/hostmem-file.c > > @@ -54,7 +54,7 @@ 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)); > > 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.rc0.48.gb9dfa238d5 > > >
diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c index 2476dcb435..5cd5fa75a7 100644 --- a/backends/hostmem-file.c +++ b/backends/hostmem-file.c @@ -54,7 +54,7 @@ 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)); 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 ?
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 | 2 +- backends/hostmem-memfd.c | 4 ---- 2 files changed, 1 insertion(+), 5 deletions(-)