| Message ID | 20200306085014.120669-1-borntraeger@de.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | mem-prealloc: initialize cond and mutex | expand |
On 06/03/20 09:50, Christian Borntraeger wrote: > Guests with mem-prealloc do fail with > qemu-system-s390x: /home/cborntra/REPOS/qemu/util/qemu-thread-posix.c:76: qemu_mutex_lock_impl: Assertion `mutex->initialized' failed. > qemu-system-s390x: /home/cborntra/REPOS/qemu/util/qemu-thread-posix.c:161: qemu_cond_broadcast: Assertion `cond->initialized' failed. > > Let us initialize cond and mutex. > > Cc: bauerchen <bauerchen@tencent.com> > Cc: Paolo Bonzini <pbonzini@redhat.com> > Reported-by: Marc Hartmayer <mhartmay@linux.ibm.com> > Fixes: 037fb5eb3941 ("mem-prealloc: optimize large guest startup") > Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> > --- > util/oslib-posix.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/util/oslib-posix.c b/util/oslib-posix.c > index 897e8f3ba6..52650183d3 100644 > --- a/util/oslib-posix.c > +++ b/util/oslib-posix.c > @@ -470,6 +470,8 @@ static bool touch_all_pages(char *area, size_t hpagesize, size_t numpages, > char *addr = area; > int i = 0; > > + qemu_cond_init(&page_cond); > + qemu_mutex_init(&page_mutex); > memset_thread_failed = false; > threads_created_flag = false; > memset_num_threads = get_memset_num_threads(smp_cpus); > Thank you very much. It's my fault, but is it too much to ask that submitters test their patches??? Paolo
On Fri, 6 Mar 2020 03:50:14 -0500 Christian Borntraeger <borntraeger@de.ibm.com> wrote: > Guests with mem-prealloc do fail with > qemu-system-s390x: /home/cborntra/REPOS/qemu/util/qemu-thread-posix.c:76: qemu_mutex_lock_impl: Assertion `mutex->initialized' failed. > qemu-system-s390x: /home/cborntra/REPOS/qemu/util/qemu-thread-posix.c:161: qemu_cond_broadcast: Assertion `cond->initialized' failed. > > Let us initialize cond and mutex. > > Cc: bauerchen <bauerchen@tencent.com> > Cc: Paolo Bonzini <pbonzini@redhat.com> > Reported-by: Marc Hartmayer <mhartmay@linux.ibm.com> > Fixes: 037fb5eb3941 ("mem-prealloc: optimize large guest startup") > Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> > --- > util/oslib-posix.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/util/oslib-posix.c b/util/oslib-posix.c > index 897e8f3ba6..52650183d3 100644 > --- a/util/oslib-posix.c > +++ b/util/oslib-posix.c > @@ -470,6 +470,8 @@ static bool touch_all_pages(char *area, size_t hpagesize, size_t numpages, > char *addr = area; > int i = 0; > > + qemu_cond_init(&page_cond); > + qemu_mutex_init(&page_mutex); Is it possible for touch_all_pages to be called several times? If it's then it probably needs a guard against that to make sure it won't explode, something like: static bool page_mutex_inited; if(page_mutex_inited) page_mutex_inited = true qemu_mutex_init(&page_mutex) ... > memset_thread_failed = false; > threads_created_flag = false; > memset_num_threads = get_memset_num_threads(smp_cpus);
On 09/03/20 11:03, Igor Mammedov wrote: >> + qemu_cond_init(&page_cond); >> + qemu_mutex_init(&page_mutex); > Is it possible for touch_all_pages to be called several times? > If it's then it probably needs a guard against that to make > sure it won't explode, something like: > > static bool page_mutex_inited; > > if(page_mutex_inited) > page_mutex_inited = true > qemu_mutex_init(&page_mutex) > ... > Hmm, good idea, it should also use GOnce. Paolo
On 09.03.20 11:05, Paolo Bonzini wrote: > On 09/03/20 11:03, Igor Mammedov wrote: >>> + qemu_cond_init(&page_cond); >>> + qemu_mutex_init(&page_mutex); >> Is it possible for touch_all_pages to be called several times? >> If it's then it probably needs a guard against that to make >> sure it won't explode, something like: >> >> static bool page_mutex_inited; >> >> if(page_mutex_inited) >> page_mutex_inited = true >> qemu_mutex_init(&page_mutex) >> ... >> > > Hmm, good idea, it should also use GOnce. Maybe start with a revert and let the original submitter send a fixed up patch?
Thanks, in fact,do_touch_pages is called just when vm starts up, but using init flag and Gonce maybe more elegant ! if needed,I can submit a new patch ! thanks very much! bauerchen From: Christian Borntraeger Date: 2020-03-09 19:01 To: Paolo Bonzini; Igor Mammedov CC: qemu-devel; qemu-s390x; Marc Hartmayer; bauerchen(陈蒙蒙) Subject: Re: [PATCH] mem-prealloc: initialize cond and mutex(Internet mail) On 09.03.20 11:05, Paolo Bonzini wrote: > On 09/03/20 11:03, Igor Mammedov wrote: >>> + qemu_cond_init(&page_cond); >>> + qemu_mutex_init(&page_mutex); >> Is it possible for touch_all_pages to be called several times? >> If it's then it probably needs a guard against that to make >> sure it won't explode, something like: >> >> static bool page_mutex_inited; >> >> if(page_mutex_inited) >> page_mutex_inited = true >> qemu_mutex_init(&page_mutex) >> ... >> > > Hmm, good idea, it should also use GOnce. Maybe start with a revert and let the original submitter send a fixed up patch?
On Mon, 9 Mar 2020 11:16:10 +0000 bauerchen(陈蒙蒙) <bauerchen@tencent.com> wrote: > Thanks, in fact,do_touch_pages is called just when vm starts up, but using init flag and Gonce maybe more elegant ! > if needed,I can submit a new patch ! > thanks very much! it's called from os_mem_prealloc() -> touch_all_pages() which is called at least once per an instance of hotsmem backend. So if several backends are used then it should be called several times. The same applies when a hostmem backend is added during runtime (hotplug)
oh ,yes.Thanks I want to know if I submit a new fixed patch or just a patch fixed current problem?? if a new fixed patch, maybe need a revert ?
On 10/03/20 08:06, bauerchen(陈蒙蒙) wrote: > oh ,yes.Thanks > I want to know if I submit a new fixed patch or just a patch fixed > current problem?? > if a new fixed patch, maybe need a revert ? Sorry I missed this message. I have already sent a fixed patch, thanks! Paolo > ------------------------------------------------------------------------ > bauerchen(陈蒙蒙) > > > *From:* Igor Mammedov <mailto:imammedo@redhat.com> > *Date:* 2020-03-09 21:19 > *To:* bauerchen(陈蒙蒙) <mailto:bauerchen@tencent.com> > *CC:* borntraeger <mailto:borntraeger@de.ibm.com>; pbonzini > <mailto:pbonzini@redhat.com>; qemu-devel > <mailto:qemu-devel@nongnu.org>; qemu-s390x > <mailto:qemu-s390x@nongnu.org>; mhartmay <mailto:mhartmay@linux.ibm.com> > *Subject:* Re: [PATCH] mem-prealloc: initialize cond and > mutex(Internet mail) > On Mon, 9 Mar 2020 11:16:10 +0000 > bauerchen(陈蒙蒙) <bauerchen@tencent.com> wrote: > > > Thanks, in fact,do_touch_pages is called just when vm starts up, > but using init flag and Gonce maybe more elegant ! > > if needed,I can submit a new patch ! > > thanks very much! > > it's called from os_mem_prealloc() -> touch_all_pages() which is called > at least once per an instance of hotsmem backend. So if several backends > are used then it should be called several times. > The same applies when a hostmem backend is added during runtime > (hotplug) > > >
ok ,thanks Paolo bauer From: Paolo Bonzini Date: 2020-03-12 02:34 To: bauerchen(陈蒙蒙); Igor Mammedov CC: borntraeger; qemu-devel; qemu-s390x; mhartmay Subject: Re: [PATCH] mem-prealloc: initialize cond and mutex(Internet mail) On 10/03/20 08:06, bauerchen(陈蒙蒙) wrote: > oh ,yes.Thanks > I want to know if I submit a new fixed patch or just a patch fixed > current problem?? > if a new fixed patch, maybe need a revert ? Sorry I missed this message. I have already sent a fixed patch, thanks! Paolo > ------------------------------------------------------------------------ > bauerchen(陈蒙蒙) > > > *From:* Igor Mammedov <mailto:imammedo@redhat.com> > *Date:* 2020-03-09 21:19 > *To:* bauerchen(陈蒙蒙) <mailto:bauerchen@tencent.com> > *CC:* borntraeger <mailto:borntraeger@de.ibm.com>; pbonzini > <mailto:pbonzini@redhat.com>; qemu-devel > <mailto:qemu-devel@nongnu.org>; qemu-s390x > <mailto:qemu-s390x@nongnu.org>; mhartmay <mailto:mhartmay@linux.ibm.com> > *Subject:* Re: [PATCH] mem-prealloc: initialize cond and > mutex(Internet mail) > On Mon, 9 Mar 2020 11:16:10 +0000 > bauerchen(陈蒙蒙) <bauerchen@tencent.com> wrote: > > > Thanks, in fact,do_touch_pages is called just when vm starts up, > but using init flag and Gonce maybe more elegant ! > > if needed,I can submit a new patch ! > > thanks very much! > > it's called from os_mem_prealloc() -> touch_all_pages() which is called > at least once per an instance of hotsmem backend. So if several backends > are used then it should be called several times. > The same applies when a hostmem backend is added during runtime > (hotplug) > > >
diff --git a/util/oslib-posix.c b/util/oslib-posix.c index 897e8f3ba6..52650183d3 100644 --- a/util/oslib-posix.c +++ b/util/oslib-posix.c @@ -470,6 +470,8 @@ static bool touch_all_pages(char *area, size_t hpagesize, size_t numpages, char *addr = area; int i = 0; + qemu_cond_init(&page_cond); + qemu_mutex_init(&page_mutex); memset_thread_failed = false; threads_created_flag = false; memset_num_threads = get_memset_num_threads(smp_cpus);
Guests with mem-prealloc do fail with qemu-system-s390x: /home/cborntra/REPOS/qemu/util/qemu-thread-posix.c:76: qemu_mutex_lock_impl: Assertion `mutex->initialized' failed. qemu-system-s390x: /home/cborntra/REPOS/qemu/util/qemu-thread-posix.c:161: qemu_cond_broadcast: Assertion `cond->initialized' failed. Let us initialize cond and mutex. Cc: bauerchen <bauerchen@tencent.com> Cc: Paolo Bonzini <pbonzini@redhat.com> Reported-by: Marc Hartmayer <mhartmay@linux.ibm.com> Fixes: 037fb5eb3941 ("mem-prealloc: optimize large guest startup") Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> --- util/oslib-posix.c | 2 ++ 1 file changed, 2 insertions(+)