[PATCH] mem-prealloc: initialize cond and mutex

[PATCH] mem-prealloc: initialize cond and mutex

[Christian Borntraeger]

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);
-- 
2.25.0

Re: [PATCH] mem-prealloc: initialize cond and mutex

[Paolo Bonzini]

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

回复: [PATCH] mem-prealloc: initialize cond and mutex(Internet mail)

[bauerchen(陈蒙蒙)]

Very sorry for my fault, in fact,I test  in a old qemu version ,and patch generated based a new one ; 
All in all，its my fault, and I will check and test patch carefully In the future
If I  need to  submit the patch again ?

 

 
发件人： Christian Borntraeger
发送时间： 2020-03-06 16:50
收件人： qemu-devel
抄送： qemu-s390x; Christian Borntraeger; bauerchen(陈蒙蒙); Paolo Bonzini; Marc Hartmayer
主题： [PATCH] mem-prealloc: initialize cond and mutex(Internet mail)
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);
--
2.25.0
 
 

Re: 回复: [PATCH] mem-prealloc: initialize cond and mutex(Internet mail)

[Christian Borntraeger]

On 06.03.20 10:57, bauerchen(陈蒙蒙) wrote:
> Very sorry for my fault, in fact,I test  in a old qemu version ,and patch generated based a new one ; 
> All in all，its my fault, and I will check and test patch carefully In the future
> If I  need to  submit the patch again ?


Can you test and review my fixup? That will help Paolo to take my fix.



> 发件人： Christian Borntraeger
> 发送时间： 2020-03-06 16:50
> 收件人： qemu-devel
> 抄送： qemu-s390x; Christian Borntraeger; bauerchen(陈蒙蒙); Paolo Bonzini; Marc Hartmayer
> 主题： [PATCH] mem-prealloc: initialize cond and mutex(Internet mail)
> 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);
> --
> 2.25.0
>  
>  
> 

Re: [PATCH] mem-prealloc: initialize cond and mutex

[Igor Mammedov]

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);

Re: [PATCH] mem-prealloc: initialize cond and mutex

[Paolo Bonzini]

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

Re: [PATCH] mem-prealloc: initialize cond and mutex

[Christian Borntraeger]

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?

Re: Re: [PATCH] mem-prealloc: initialize cond and mutex(Internet mail)

[bauerchen(陈蒙蒙)]

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?
 
 

Re: [PATCH] mem-prealloc: initialize cond and mutex(Internet mail)

[Igor Mammedov]

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)

Re: Re: [PATCH] mem-prealloc: initialize cond and mutex(Internet mail)

[bauerchen(陈蒙蒙)]

[-- Attachment #1: Type: text/plain, Size: 1171 bytes --]

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 ?
________________________________
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)



[-- Attachment #2: Type: text/html, Size: 2515 bytes --]

Re: [PATCH] mem-prealloc: initialize cond and mutex(Internet mail)

[Paolo Bonzini]

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)
>      
>      
> 

Re: Re: [PATCH] mem-prealloc: initialize cond and mutex(Internet mail)

[bauerchen(陈蒙蒙)]

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)
>      
>      
>