* [Qemu-devel] [PATCH] hostmem: no need to check for host_memory_backend_mr_inited() in alloc()
@ 2018-08-30 17:50 Marc-André Lureau
2018-08-30 19:09 ` Eduardo Habkost
0 siblings, 1 reply; 3+ messages in thread
From: Marc-André Lureau @ 2018-08-30 17:50 UTC (permalink / raw)
To: qemu-devel; +Cc: imammedo, Marc-André Lureau, Eduardo Habkost
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(-)
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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] hostmem: no need to check for host_memory_backend_mr_inited() in alloc()
2018-08-30 17:50 [Qemu-devel] [PATCH] hostmem: no need to check for host_memory_backend_mr_inited() in alloc() Marc-André Lureau
@ 2018-08-30 19:09 ` Eduardo Habkost
2018-08-31 6:53 ` Igor Mammedov
0 siblings, 1 reply; 3+ messages in thread
From: Eduardo Habkost @ 2018-08-30 19:09 UTC (permalink / raw)
To: Marc-André Lureau; +Cc: qemu-devel, imammedo
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
>
--
Eduardo
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] hostmem: no need to check for host_memory_backend_mr_inited() in alloc()
2018-08-30 19:09 ` Eduardo Habkost
@ 2018-08-31 6:53 ` Igor Mammedov
0 siblings, 0 replies; 3+ messages in thread
From: Igor Mammedov @ 2018-08-31 6:53 UTC (permalink / raw)
To: Eduardo Habkost; +Cc: Marc-André Lureau, qemu-devel
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
> >
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-08-31 6:53 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-30 17:50 [Qemu-devel] [PATCH] hostmem: no need to check for host_memory_backend_mr_inited() in alloc() Marc-André Lureau
2018-08-30 19:09 ` Eduardo Habkost
2018-08-31 6:53 ` Igor Mammedov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.