[PATCH 0/2] ppc: fix vcpu hotunplug leak in spapr_realize_vcpu

Daniel Henrique Barboza posted 2 patches 3 years, 10 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20220328125918.494787-1-danielhb413@gmail.com
Maintainers: "Cédric Le Goater" <clg@kaod.org>, Daniel Henrique Barboza <danielhb413@gmail.com>, David Gibson <david@gibson.dropbear.id.au>, Greg Kurz <groug@kaod.org>
hw/ppc/ppc.c            | 7 +++++++
hw/ppc/spapr_cpu_core.c | 3 +++
include/hw/ppc/ppc.h    | 1 +
3 files changed, 11 insertions(+)
[PATCH 0/2] ppc: fix vcpu hotunplug leak in spapr_realize_vcpu
Posted by Daniel Henrique Barboza 3 years, 10 months ago
Hi,

This is a memory leak found by Valgrind when testing vcpu
hotplug/unplug in pSeries guests.

Other vcpu hotplug/unplug leaks are still present in the common code
(one in the KVM thread loop and another in cpu_address_space via
cpu->cpu_ases) but these are already being handled by Mark Kanda and
Phillipe.


Daniel Henrique Barboza (2):
  hw/ppc/ppc.c: add cpu_ppc_tb_free()
  hw/ppc: free env->tb_env in spapr_unrealize_vcpu()

 hw/ppc/ppc.c            | 7 +++++++
 hw/ppc/spapr_cpu_core.c | 3 +++
 include/hw/ppc/ppc.h    | 1 +
 3 files changed, 11 insertions(+)

-- 
2.35.1
Re: [PATCH 0/2] ppc: fix vcpu hotunplug leak in spapr_realize_vcpu
Posted by David Gibson 3 years, 10 months ago
On Mon, Mar 28, 2022 at 09:59:16AM -0300, Daniel Henrique Barboza wrote:
> Hi,
> 
> This is a memory leak found by Valgrind when testing vcpu
> hotplug/unplug in pSeries guests.
> 
> Other vcpu hotplug/unplug leaks are still present in the common code
> (one in the KVM thread loop and another in cpu_address_space via
> cpu->cpu_ases) but these are already being handled by Mark Kanda and
> Phillipe.

Changes LGTM, but I don't see much reason to split this into two
patches.  They're both small, and are part of the same logical change.

> 
> 
> Daniel Henrique Barboza (2):
>   hw/ppc/ppc.c: add cpu_ppc_tb_free()
>   hw/ppc: free env->tb_env in spapr_unrealize_vcpu()
> 
>  hw/ppc/ppc.c            | 7 +++++++
>  hw/ppc/spapr_cpu_core.c | 3 +++
>  include/hw/ppc/ppc.h    | 1 +
>  3 files changed, 11 insertions(+)
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson
Re: [PATCH 0/2] ppc: fix vcpu hotunplug leak in spapr_realize_vcpu
Posted by Cédric Le Goater 3 years, 10 months ago
On 3/29/22 03:24, David Gibson wrote:
> On Mon, Mar 28, 2022 at 09:59:16AM -0300, Daniel Henrique Barboza wrote:
>> Hi,
>>
>> This is a memory leak found by Valgrind when testing vcpu
>> hotplug/unplug in pSeries guests.
>>
>> Other vcpu hotplug/unplug leaks are still present in the common code
>> (one in the KVM thread loop and another in cpu_address_space via
>> cpu->cpu_ases) but these are already being handled by Mark Kanda and
>> Phillipe.
> 
> Changes LGTM, but I don't see much reason to split this into two
> patches.  They're both small, and are part of the same logical change.

And it could be a 7.0 candidate. Are we ok with that ?

Thanks,

C.
Re: [PATCH 0/2] ppc: fix vcpu hotunplug leak in spapr_realize_vcpu
Posted by Daniel Henrique Barboza 3 years, 10 months ago

On 3/29/22 05:36, Cédric Le Goater wrote:
> On 3/29/22 03:24, David Gibson wrote:
>> On Mon, Mar 28, 2022 at 09:59:16AM -0300, Daniel Henrique Barboza wrote:
>>> Hi,
>>>
>>> This is a memory leak found by Valgrind when testing vcpu
>>> hotplug/unplug in pSeries guests.
>>>
>>> Other vcpu hotplug/unplug leaks are still present in the common code
>>> (one in the KVM thread loop and another in cpu_address_space via
>>> cpu->cpu_ases) but these are already being handled by Mark Kanda and
>>> Phillipe.
>>
>> Changes LGTM, but I don't see much reason to split this into two
>> patches.  They're both small, and are part of the same logical change.


I did it in separated patches because I tried to find other instances where
the timebase would need to be freed. Didn't find any.

I am ok with squashing them in a single patch. I'll send a v2.

> 
> And it could be a 7.0 candidate. Are we ok with that ?

Since it's a memory leak we are now aware of and have a fix for, yeah, I think
it qualifies for 7.0.


Daniel

> 
> Thanks,
> 
> C.