1. Patchew
  2. QEMU
  3. View series

[Qemu-devel] [PATCH] cpus: fix TCG timer leak

Marc-André Lureau posted 1 patch 5 years, 7 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20180925172305.5105-1-marcandre.lureau@redhat.com
Test docker-clang@ubuntu failed
Test checkpatch passed
cpus.c | 1 +
1 file changed, 1 insertion(+)
[Qemu-devel] [PATCH] cpus: fix TCG timer leak
Posted by Marc-André Lureau 5 years, 7 months ago 
Spotted by ASAN:

QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 tests/bios-tables-test  -p /x86_64/acpi/piix4/cpuhp
/x86_64/acpi/piix4/cpuhp: Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==21216==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!

=================================================================
==21216==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 48 byte(s) in 1 object(s) allocated from:
    #0 0x7f77c0668e50 in calloc (/lib64/libasan.so.5+0xeee50)
    #1 0x7f77beb7b41d in g_malloc0 (/lib64/libglib-2.0.so.0+0x5241d)
    #2 0x557f756df5bd in timer_new /home/elmarco/src/qemu/include/qemu/timer.h:561
    #3 0x557f756df5ee in timer_new_ns /home/elmarco/src/qemu/include/qemu/timer.h:584
    #4 0x557f756e4690 in start_tcg_kick_timer /home/elmarco/src/qemu/cpus.c:965
    #5 0x557f756e64d7 in qemu_tcg_rr_wait_io_event /home/elmarco/src/qemu/cpus.c:1210
    #6 0x557f756e90ad in qemu_tcg_rr_cpu_thread_fn /home/elmarco/src/qemu/cpus.c:1536
    #7 0x557f76e9f233 in qemu_thread_start /home/elmarco/src/qemu/util/qemu-thread-posix.c:504
    #8 0x7f77b4a11593 in start_thread (/lib64/libpthread.so.0+0x7593)

Direct leak of 48 byte(s) in 1 object(s) allocated from:
    #0 0x7f77c0668e50 in calloc (/lib64/libasan.so.5+0xeee50)
    #1 0x7f77beb7b41d in g_malloc0 (/lib64/libglib-2.0.so.0+0x5241d)
    #2 0x557f756df5bd in timer_new /home/elmarco/src/qemu/include/qemu/timer.h:561
    #3 0x557f756df5ee in timer_new_ns /home/elmarco/src/qemu/include/qemu/timer.h:584
    #4 0x557f756e4690 in start_tcg_kick_timer /home/elmarco/src/qemu/cpus.c:965
    #5 0x557f756e8616 in qemu_tcg_rr_cpu_thread_fn /home/elmarco/src/qemu/cpus.c:1466
    #6 0x557f76e9f233 in qemu_thread_start /home/elmarco/src/qemu/util/qemu-thread-posix.c:504
    #7 0x7f77b4a11593 in start_thread (/lib64/libpthread.so.0+0x7593)

SUMMARY: AddressSanitizer: 96 byte(s) leaked in 2 allocation(s).

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 cpus.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/cpus.c b/cpus.c
index 719788320f..0513c657f4 100644
--- a/cpus.c
+++ b/cpus.c
@@ -973,6 +973,7 @@ static void stop_tcg_kick_timer(void)
     assert(!mttcg_enabled);
     if (tcg_kick_vcpu_timer) {
         timer_del(tcg_kick_vcpu_timer);
+        timer_free(tcg_kick_vcpu_timer);
         tcg_kick_vcpu_timer = NULL;
     }
 }
-- 
2.19.0.271.gfe8321ec05
Re: [Qemu-devel] [PATCH] cpus: fix TCG timer leak
Posted by Philippe Mathieu-Daudé 5 years, 7 months ago 
On 9/25/18 7:23 PM, Marc-André Lureau wrote:
> Spotted by ASAN:
> 
> QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 tests/bios-tables-test  -p /x86_64/acpi/piix4/cpuhp
> /x86_64/acpi/piix4/cpuhp: Could not access KVM kernel module: No such file or directory
> qemu-system-x86_64: failed to initialize KVM: No such file or directory
> qemu-system-x86_64: Back to tcg accelerator
> ==21216==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
> 
> =================================================================
> ==21216==ERROR: LeakSanitizer: detected memory leaks
> 
> Direct leak of 48 byte(s) in 1 object(s) allocated from:
>     #0 0x7f77c0668e50 in calloc (/lib64/libasan.so.5+0xeee50)
>     #1 0x7f77beb7b41d in g_malloc0 (/lib64/libglib-2.0.so.0+0x5241d)
>     #2 0x557f756df5bd in timer_new /home/elmarco/src/qemu/include/qemu/timer.h:561
>     #3 0x557f756df5ee in timer_new_ns /home/elmarco/src/qemu/include/qemu/timer.h:584
>     #4 0x557f756e4690 in start_tcg_kick_timer /home/elmarco/src/qemu/cpus.c:965
>     #5 0x557f756e64d7 in qemu_tcg_rr_wait_io_event /home/elmarco/src/qemu/cpus.c:1210
>     #6 0x557f756e90ad in qemu_tcg_rr_cpu_thread_fn /home/elmarco/src/qemu/cpus.c:1536
>     #7 0x557f76e9f233 in qemu_thread_start /home/elmarco/src/qemu/util/qemu-thread-posix.c:504
>     #8 0x7f77b4a11593 in start_thread (/lib64/libpthread.so.0+0x7593)
> 
> Direct leak of 48 byte(s) in 1 object(s) allocated from:
>     #0 0x7f77c0668e50 in calloc (/lib64/libasan.so.5+0xeee50)
>     #1 0x7f77beb7b41d in g_malloc0 (/lib64/libglib-2.0.so.0+0x5241d)
>     #2 0x557f756df5bd in timer_new /home/elmarco/src/qemu/include/qemu/timer.h:561
>     #3 0x557f756df5ee in timer_new_ns /home/elmarco/src/qemu/include/qemu/timer.h:584
>     #4 0x557f756e4690 in start_tcg_kick_timer /home/elmarco/src/qemu/cpus.c:965
>     #5 0x557f756e8616 in qemu_tcg_rr_cpu_thread_fn /home/elmarco/src/qemu/cpus.c:1466
>     #6 0x557f76e9f233 in qemu_thread_start /home/elmarco/src/qemu/util/qemu-thread-posix.c:504
>     #7 0x7f77b4a11593 in start_thread (/lib64/libpthread.so.0+0x7593)
> 
> SUMMARY: AddressSanitizer: 96 byte(s) leaked in 2 allocation(s).
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>  cpus.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/cpus.c b/cpus.c
> index 719788320f..0513c657f4 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -973,6 +973,7 @@ static void stop_tcg_kick_timer(void)
>      assert(!mttcg_enabled);
>      if (tcg_kick_vcpu_timer) {
>          timer_del(tcg_kick_vcpu_timer);
> +        timer_free(tcg_kick_vcpu_timer);
>          tcg_kick_vcpu_timer = NULL;
>      }
>  }
>
Re: [Qemu-devel] [PATCH] cpus: fix TCG timer leak
Posted by Alex Bennée 5 years, 7 months ago 
Marc-André Lureau <marcandre.lureau@redhat.com> writes:

> Spotted by ASAN:
>
> QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 tests/bios-tables-test  -p /x86_64/acpi/piix4/cpuhp
> /x86_64/acpi/piix4/cpuhp: Could not access KVM kernel module: No such file or directory
> qemu-system-x86_64: failed to initialize KVM: No such file or directory
> qemu-system-x86_64: Back to tcg accelerator
> ==21216==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
>
> =================================================================
> ==21216==ERROR: LeakSanitizer: detected memory leaks
>
> Direct leak of 48 byte(s) in 1 object(s) allocated from:
>     #0 0x7f77c0668e50 in calloc (/lib64/libasan.so.5+0xeee50)
>     #1 0x7f77beb7b41d in g_malloc0 (/lib64/libglib-2.0.so.0+0x5241d)
>     #2 0x557f756df5bd in timer_new /home/elmarco/src/qemu/include/qemu/timer.h:561
>     #3 0x557f756df5ee in timer_new_ns /home/elmarco/src/qemu/include/qemu/timer.h:584
>     #4 0x557f756e4690 in start_tcg_kick_timer /home/elmarco/src/qemu/cpus.c:965
>     #5 0x557f756e64d7 in qemu_tcg_rr_wait_io_event /home/elmarco/src/qemu/cpus.c:1210
>     #6 0x557f756e90ad in qemu_tcg_rr_cpu_thread_fn /home/elmarco/src/qemu/cpus.c:1536
>     #7 0x557f76e9f233 in qemu_thread_start /home/elmarco/src/qemu/util/qemu-thread-posix.c:504
>     #8 0x7f77b4a11593 in start_thread (/lib64/libpthread.so.0+0x7593)
>
> Direct leak of 48 byte(s) in 1 object(s) allocated from:
>     #0 0x7f77c0668e50 in calloc (/lib64/libasan.so.5+0xeee50)
>     #1 0x7f77beb7b41d in g_malloc0 (/lib64/libglib-2.0.so.0+0x5241d)
>     #2 0x557f756df5bd in timer_new /home/elmarco/src/qemu/include/qemu/timer.h:561
>     #3 0x557f756df5ee in timer_new_ns /home/elmarco/src/qemu/include/qemu/timer.h:584
>     #4 0x557f756e4690 in start_tcg_kick_timer /home/elmarco/src/qemu/cpus.c:965
>     #5 0x557f756e8616 in qemu_tcg_rr_cpu_thread_fn /home/elmarco/src/qemu/cpus.c:1466
>     #6 0x557f76e9f233 in qemu_thread_start /home/elmarco/src/qemu/util/qemu-thread-posix.c:504
>     #7 0x7f77b4a11593 in start_thread (/lib64/libpthread.so.0+0x7593)
>
> SUMMARY: AddressSanitizer: 96 byte(s) leaked in 2 allocation(s).
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  cpus.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/cpus.c b/cpus.c
> index 719788320f..0513c657f4 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -973,6 +973,7 @@ static void stop_tcg_kick_timer(void)
>      assert(!mttcg_enabled);
>      if (tcg_kick_vcpu_timer) {
>          timer_del(tcg_kick_vcpu_timer);
> +        timer_free(tcg_kick_vcpu_timer);
>          tcg_kick_vcpu_timer = NULL;
>      }
>  }

Arguably we should be stopping and starting the timer properly rather
than re-creating it every time. However that would complicate the logic
that is currently just using the presence of a timer reference to infer
activeness. So:

Acked-by: Alex Bennée <alex.bennee@linaro.org>

--
Alex Bennée
Re: [Qemu-devel] [PATCH] cpus: fix TCG timer leak
Posted by Paolo Bonzini 5 years, 7 months ago 
On 25/09/2018 22:17, Alex Bennée wrote:
> 
> Marc-André Lureau <marcandre.lureau@redhat.com> writes:
> 
>> Spotted by ASAN:
>>
>> QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 tests/bios-tables-test  -p /x86_64/acpi/piix4/cpuhp
>> /x86_64/acpi/piix4/cpuhp: Could not access KVM kernel module: No such file or directory
>> qemu-system-x86_64: failed to initialize KVM: No such file or directory
>> qemu-system-x86_64: Back to tcg accelerator
>> ==21216==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
>>
>> =================================================================
>> ==21216==ERROR: LeakSanitizer: detected memory leaks
>>
>> Direct leak of 48 byte(s) in 1 object(s) allocated from:
>>     #0 0x7f77c0668e50 in calloc (/lib64/libasan.so.5+0xeee50)
>>     #1 0x7f77beb7b41d in g_malloc0 (/lib64/libglib-2.0.so.0+0x5241d)
>>     #2 0x557f756df5bd in timer_new /home/elmarco/src/qemu/include/qemu/timer.h:561
>>     #3 0x557f756df5ee in timer_new_ns /home/elmarco/src/qemu/include/qemu/timer.h:584
>>     #4 0x557f756e4690 in start_tcg_kick_timer /home/elmarco/src/qemu/cpus.c:965
>>     #5 0x557f756e64d7 in qemu_tcg_rr_wait_io_event /home/elmarco/src/qemu/cpus.c:1210
>>     #6 0x557f756e90ad in qemu_tcg_rr_cpu_thread_fn /home/elmarco/src/qemu/cpus.c:1536
>>     #7 0x557f76e9f233 in qemu_thread_start /home/elmarco/src/qemu/util/qemu-thread-posix.c:504
>>     #8 0x7f77b4a11593 in start_thread (/lib64/libpthread.so.0+0x7593)
>>
>> Direct leak of 48 byte(s) in 1 object(s) allocated from:
>>     #0 0x7f77c0668e50 in calloc (/lib64/libasan.so.5+0xeee50)
>>     #1 0x7f77beb7b41d in g_malloc0 (/lib64/libglib-2.0.so.0+0x5241d)
>>     #2 0x557f756df5bd in timer_new /home/elmarco/src/qemu/include/qemu/timer.h:561
>>     #3 0x557f756df5ee in timer_new_ns /home/elmarco/src/qemu/include/qemu/timer.h:584
>>     #4 0x557f756e4690 in start_tcg_kick_timer /home/elmarco/src/qemu/cpus.c:965
>>     #5 0x557f756e8616 in qemu_tcg_rr_cpu_thread_fn /home/elmarco/src/qemu/cpus.c:1466
>>     #6 0x557f76e9f233 in qemu_thread_start /home/elmarco/src/qemu/util/qemu-thread-posix.c:504
>>     #7 0x7f77b4a11593 in start_thread (/lib64/libpthread.so.0+0x7593)
>>
>> SUMMARY: AddressSanitizer: 96 byte(s) leaked in 2 allocation(s).
>>
>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> ---
>>  cpus.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/cpus.c b/cpus.c
>> index 719788320f..0513c657f4 100644
>> --- a/cpus.c
>> +++ b/cpus.c
>> @@ -973,6 +973,7 @@ static void stop_tcg_kick_timer(void)
>>      assert(!mttcg_enabled);
>>      if (tcg_kick_vcpu_timer) {
>>          timer_del(tcg_kick_vcpu_timer);
>> +        timer_free(tcg_kick_vcpu_timer);
>>          tcg_kick_vcpu_timer = NULL;
>>      }
>>  }
> 
> Arguably we should be stopping and starting the timer properly rather
> than re-creating it every time. However that would complicate the logic
> that is currently just using the presence of a timer reference to infer
> activeness.

You can use timer_pending for that (and then timer_init instead of
timer_new).

Paolo
Re: [Qemu-devel] [PATCH] cpus: fix TCG timer leak
Posted by Alex Bennée 5 years, 6 months ago 
Marc-André Lureau <marcandre.lureau@redhat.com> writes:

> Spotted by ASAN:
>
> QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 tests/bios-tables-test  -p /x86_64/acpi/piix4/cpuhp
> /x86_64/acpi/piix4/cpuhp: Could not access KVM kernel module: No such file or directory
> qemu-system-x86_64: failed to initialize KVM: No such file or directory
> qemu-system-x86_64: Back to tcg accelerator
> ==21216==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
>
> =================================================================
> ==21216==ERROR: LeakSanitizer: detected memory leaks
>
> Direct leak of 48 byte(s) in 1 object(s) allocated from:
>     #0 0x7f77c0668e50 in calloc (/lib64/libasan.so.5+0xeee50)
>     #1 0x7f77beb7b41d in g_malloc0 (/lib64/libglib-2.0.so.0+0x5241d)
>     #2 0x557f756df5bd in timer_new /home/elmarco/src/qemu/include/qemu/timer.h:561
>     #3 0x557f756df5ee in timer_new_ns /home/elmarco/src/qemu/include/qemu/timer.h:584
>     #4 0x557f756e4690 in start_tcg_kick_timer /home/elmarco/src/qemu/cpus.c:965
>     #5 0x557f756e64d7 in qemu_tcg_rr_wait_io_event /home/elmarco/src/qemu/cpus.c:1210
>     #6 0x557f756e90ad in qemu_tcg_rr_cpu_thread_fn /home/elmarco/src/qemu/cpus.c:1536
>     #7 0x557f76e9f233 in qemu_thread_start /home/elmarco/src/qemu/util/qemu-thread-posix.c:504
>     #8 0x7f77b4a11593 in start_thread (/lib64/libpthread.so.0+0x7593)

I'm trying to reproduce to try the alternate approach but I can't get
the full object backtraces:

14:56:08 [alex@zen:~/l/q/qemu.git] misc/tcg-timer-fix(1) 2 + env QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 tests/bios-tables-test  -p /x86_64/acpi/piix4/cpuhp
/x86_64/acpi/piix4/cpuhp: ==30510==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
/home/alex/lsrc/qemu/qemu.git/include/qemu/rcu.h:78:21: runtime error: member access within null pointer of type 'struct rcu_reader_data'
/home/alex/lsrc/qemu/qemu.git/include/qemu/rcu.h:93:5: runtime error: member access within null pointer of type 'struct rcu_reader_data'
/home/alex/lsrc/qemu/qemu.git/include/qemu/rcu.h:94:8: runtime error: member access within null pointer of type 'struct rcu_reader_data'
/home/alex/lsrc/qemu/qemu.git/include/qemu/rcu.h:93:5: runtime error: member access within null pointer of type 'struct rcu_reader_data'
OK

What is your configure setup?

--
Alex Bennée
Re: [Qemu-devel] [PATCH] cpus: fix TCG timer leak
Posted by Marc-André Lureau 5 years, 6 months ago 
Hi

On Thu, Sep 27, 2018 at 5:57 PM Alex Bennée <alex.bennee@linaro.org> wrote:
>
>
> Marc-André Lureau <marcandre.lureau@redhat.com> writes:
>
> > Spotted by ASAN:
> >
> > QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 tests/bios-tables-test  -p /x86_64/acpi/piix4/cpuhp
> > /x86_64/acpi/piix4/cpuhp: Could not access KVM kernel module: No such file or directory
> > qemu-system-x86_64: failed to initialize KVM: No such file or directory
> > qemu-system-x86_64: Back to tcg accelerator
> > ==21216==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
> >
> > =================================================================
> > ==21216==ERROR: LeakSanitizer: detected memory leaks
> >
> > Direct leak of 48 byte(s) in 1 object(s) allocated from:
> >     #0 0x7f77c0668e50 in calloc (/lib64/libasan.so.5+0xeee50)
> >     #1 0x7f77beb7b41d in g_malloc0 (/lib64/libglib-2.0.so.0+0x5241d)
> >     #2 0x557f756df5bd in timer_new /home/elmarco/src/qemu/include/qemu/timer.h:561
> >     #3 0x557f756df5ee in timer_new_ns /home/elmarco/src/qemu/include/qemu/timer.h:584
> >     #4 0x557f756e4690 in start_tcg_kick_timer /home/elmarco/src/qemu/cpus.c:965
> >     #5 0x557f756e64d7 in qemu_tcg_rr_wait_io_event /home/elmarco/src/qemu/cpus.c:1210
> >     #6 0x557f756e90ad in qemu_tcg_rr_cpu_thread_fn /home/elmarco/src/qemu/cpus.c:1536
> >     #7 0x557f76e9f233 in qemu_thread_start /home/elmarco/src/qemu/util/qemu-thread-posix.c:504
> >     #8 0x7f77b4a11593 in start_thread (/lib64/libpthread.so.0+0x7593)
>
> I'm trying to reproduce to try the alternate approach but I can't get
> the full object backtraces:
>
> 14:56:08 [alex@zen:~/l/q/qemu.git] misc/tcg-timer-fix(1) 2 + env QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 tests/bios-tables-test  -p /x86_64/acpi/piix4/cpuhp
> /x86_64/acpi/piix4/cpuhp: ==30510==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
> /home/alex/lsrc/qemu/qemu.git/include/qemu/rcu.h:78:21: runtime error: member access within null pointer of type 'struct rcu_reader_data'
> /home/alex/lsrc/qemu/qemu.git/include/qemu/rcu.h:93:5: runtime error: member access within null pointer of type 'struct rcu_reader_data'
> /home/alex/lsrc/qemu/qemu.git/include/qemu/rcu.h:94:8: runtime error: member access within null pointer of type 'struct rcu_reader_data'
> /home/alex/lsrc/qemu/qemu.git/include/qemu/rcu.h:93:5: runtime error: member access within null pointer of type 'struct rcu_reader_data'
> OK
>
> What is your configure setup?

It's run in the docker build, where /dev/kvm isn't available (not sure why btw)

"/x86_64/acpi/piix4/cpuhp: Could not access KVM kernel module: No such
file or directory"

>
> --
> Alex Bennée

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.