hw/ppc/spapr_caps.c | 2 ++ hw/ppc/spapr_drc.c | 5 ++++- hw/ppc/spapr_hcall.c | 2 ++ target/ppc/kvm.c | 3 ++- 4 files changed, 10 insertions(+), 2 deletions(-)
valgrind showed some memory leaks while running qemu-system-ppc64.
Fixing them in this patch.
Signed-off-by: Shivaprasad G Bhat <sbhat@linux.ibm.com>
---
hw/ppc/spapr_caps.c | 2 ++
hw/ppc/spapr_drc.c | 5 ++++-
hw/ppc/spapr_hcall.c | 2 ++
target/ppc/kvm.c | 3 ++-
4 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
index bbb001f84a..8e3350f777 100644
--- a/hw/ppc/spapr_caps.c
+++ b/hw/ppc/spapr_caps.c
@@ -786,11 +786,13 @@ void spapr_caps_add_properties(SpaprMachineClass *smc, Error **errp)
NULL, cap, &local_err);
if (local_err) {
error_propagate(errp, local_err);
+ g_free((char *)name);
return;
}
desc = g_strdup_printf("%s", cap->description);
object_class_property_set_description(klass, name, desc, &local_err);
+ g_free((char *)name);
g_free(desc);
if (local_err) {
error_propagate(errp, local_err);
diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index bacadfcac5..37fbfe6900 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -827,6 +827,7 @@ int spapr_dt_drc(void *fdt, int offset, Object *owner, uint32_t drc_type_mask)
Object *obj;
SpaprDrc *drc;
SpaprDrcClass *drck;
+ const char *drc_name = NULL;
uint32_t drc_index, drc_power_domain;
if (!strstart(prop->type, "link<", NULL)) {
@@ -856,8 +857,10 @@ int spapr_dt_drc(void *fdt, int offset, Object *owner, uint32_t drc_type_mask)
g_array_append_val(drc_power_domains, drc_power_domain);
/* ibm,drc-names */
- drc_names = g_string_append(drc_names, spapr_drc_name(drc));
+ drc_name = spapr_drc_name(drc);
+ drc_names = g_string_append(drc_names, drc_name);
drc_names = g_string_insert_len(drc_names, -1, "\0", 1);
+ g_free((char *)drc_name);
/* ibm,drc-types */
drc_types = g_string_append(drc_types, drck->typename);
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 6808d4cda8..0fc58156a0 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -1612,6 +1612,7 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
ov5_updates = spapr_ovec_new();
spapr->cas_reboot = spapr_ovec_diff(ov5_updates,
ov5_cas_old, spapr->ov5_cas);
+ spapr_ovec_cleanup(ov5_cas_old);
/* Now that processing is finished, set the radix/hash bit for the
* guest if it requested a valid mode; otherwise terminate the boot. */
if (guest_radix) {
@@ -1640,6 +1641,7 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
(spapr_h_cas_compose_response(spapr, args[1], args[2],
ov5_updates) != 0);
}
+ spapr_ovec_cleanup(ov1_guest);
/*
* Ensure the guest asks for an interrupt mode we support; otherwise
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index 8a06d3171e..498ca6d53b 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -491,7 +491,8 @@ int kvm_arch_init_vcpu(CPUState *cs)
return ret;
}
- idle_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, kvm_kick_cpu, cpu);
+ if (!idle_timer)
+ idle_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, kvm_kick_cpu, cpu);
switch (cenv->mmu_model) {
case POWERPC_MMU_BOOKE206:
On Tue, 16 Jul 2019 03:24:57 -0500 Shivaprasad G Bhat <sbhat@linux.ibm.com> wrote: > valgrind showed some memory leaks while running qemu-system-ppc64. > Fixing them in this patch. > Since this covers many unrelated code paths, I think it would be better to make this a series of separate patches, rather than one patch. > Signed-off-by: Shivaprasad G Bhat <sbhat@linux.ibm.com> > --- > hw/ppc/spapr_caps.c | 2 ++ > hw/ppc/spapr_drc.c | 5 ++++- > hw/ppc/spapr_hcall.c | 2 ++ > target/ppc/kvm.c | 3 ++- > 4 files changed, 10 insertions(+), 2 deletions(-) > > diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c > index bbb001f84a..8e3350f777 100644 > --- a/hw/ppc/spapr_caps.c > +++ b/hw/ppc/spapr_caps.c > @@ -786,11 +786,13 @@ void spapr_caps_add_properties(SpaprMachineClass *smc, Error **errp) > NULL, cap, &local_err); > if (local_err) { > error_propagate(errp, local_err); > + g_free((char *)name); Like Richard indicated in another mail, this is wrong : 'const char *' means that the string is constant forever. If you have to free it, then it shouldn't be const in the first place. > return; > } > > desc = g_strdup_printf("%s", cap->description); > object_class_property_set_description(klass, name, desc, &local_err); > + g_free((char *)name); > g_free(desc); > if (local_err) { > error_propagate(errp, local_err); > diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c > index bacadfcac5..37fbfe6900 100644 > --- a/hw/ppc/spapr_drc.c > +++ b/hw/ppc/spapr_drc.c > @@ -827,6 +827,7 @@ int spapr_dt_drc(void *fdt, int offset, Object *owner, uint32_t drc_type_mask) > Object *obj; > SpaprDrc *drc; > SpaprDrcClass *drck; > + const char *drc_name = NULL; Same here. > uint32_t drc_index, drc_power_domain; > > if (!strstart(prop->type, "link<", NULL)) { > @@ -856,8 +857,10 @@ int spapr_dt_drc(void *fdt, int offset, Object *owner, uint32_t drc_type_mask) > g_array_append_val(drc_power_domains, drc_power_domain); > > /* ibm,drc-names */ > - drc_names = g_string_append(drc_names, spapr_drc_name(drc)); > + drc_name = spapr_drc_name(drc); > + drc_names = g_string_append(drc_names, drc_name); > drc_names = g_string_insert_len(drc_names, -1, "\0", 1); > + g_free((char *)drc_name); > > /* ibm,drc-types */ > drc_types = g_string_append(drc_types, drck->typename); > diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c > index 6808d4cda8..0fc58156a0 100644 > --- a/hw/ppc/spapr_hcall.c > +++ b/hw/ppc/spapr_hcall.c > @@ -1612,6 +1612,7 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu, > ov5_updates = spapr_ovec_new(); > spapr->cas_reboot = spapr_ovec_diff(ov5_updates, > ov5_cas_old, spapr->ov5_cas); > + spapr_ovec_cleanup(ov5_cas_old); > /* Now that processing is finished, set the radix/hash bit for the > * guest if it requested a valid mode; otherwise terminate the boot. */ > if (guest_radix) { > @@ -1640,6 +1641,7 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu, > (spapr_h_cas_compose_response(spapr, args[1], args[2], > ov5_updates) != 0); > } > + spapr_ovec_cleanup(ov1_guest); > Maybe a few lines ealier, just after its last user: spapr->cas_legacy_guest_workaround = !spapr_ovec_test(ov1_guest, OV1_PPC_3_00); > /* > * Ensure the guest asks for an interrupt mode we support; otherwise > diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c > index 8a06d3171e..498ca6d53b 100644 > --- a/target/ppc/kvm.c > +++ b/target/ppc/kvm.c > @@ -491,7 +491,8 @@ int kvm_arch_init_vcpu(CPUState *cs) > return ret; > } > > - idle_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, kvm_kick_cpu, cpu); > + if (!idle_timer) > + idle_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, kvm_kick_cpu, cpu); > Yikes... idle_timer is a static but it is used by all vcpus... it looks like it's a bug. > switch (cenv->mmu_model) { > case POWERPC_MMU_BOOKE206: > >
On Tue, Jul 16, 2019 at 03:24:57AM -0500, Shivaprasad G Bhat wrote: > valgrind showed some memory leaks while running qemu-system-ppc64. > Fixing them in this patch. > > Signed-off-by: Shivaprasad G Bhat <sbhat@linux.ibm.com> Applied, thanks. > --- > hw/ppc/spapr_caps.c | 2 ++ > hw/ppc/spapr_drc.c | 5 ++++- > hw/ppc/spapr_hcall.c | 2 ++ > target/ppc/kvm.c | 3 ++- > 4 files changed, 10 insertions(+), 2 deletions(-) > > diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c > index bbb001f84a..8e3350f777 100644 > --- a/hw/ppc/spapr_caps.c > +++ b/hw/ppc/spapr_caps.c > @@ -786,11 +786,13 @@ void spapr_caps_add_properties(SpaprMachineClass *smc, Error **errp) > NULL, cap, &local_err); > if (local_err) { > error_propagate(errp, local_err); > + g_free((char *)name); > return; > } > > desc = g_strdup_printf("%s", cap->description); > object_class_property_set_description(klass, name, desc, &local_err); > + g_free((char *)name); > g_free(desc); > if (local_err) { > error_propagate(errp, local_err); > diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c > index bacadfcac5..37fbfe6900 100644 > --- a/hw/ppc/spapr_drc.c > +++ b/hw/ppc/spapr_drc.c > @@ -827,6 +827,7 @@ int spapr_dt_drc(void *fdt, int offset, Object *owner, uint32_t drc_type_mask) > Object *obj; > SpaprDrc *drc; > SpaprDrcClass *drck; > + const char *drc_name = NULL; > uint32_t drc_index, drc_power_domain; > > if (!strstart(prop->type, "link<", NULL)) { > @@ -856,8 +857,10 @@ int spapr_dt_drc(void *fdt, int offset, Object *owner, uint32_t drc_type_mask) > g_array_append_val(drc_power_domains, drc_power_domain); > > /* ibm,drc-names */ > - drc_names = g_string_append(drc_names, spapr_drc_name(drc)); > + drc_name = spapr_drc_name(drc); > + drc_names = g_string_append(drc_names, drc_name); > drc_names = g_string_insert_len(drc_names, -1, "\0", 1); > + g_free((char *)drc_name); > > /* ibm,drc-types */ > drc_types = g_string_append(drc_types, drck->typename); > diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c > index 6808d4cda8..0fc58156a0 100644 > --- a/hw/ppc/spapr_hcall.c > +++ b/hw/ppc/spapr_hcall.c > @@ -1612,6 +1612,7 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu, > ov5_updates = spapr_ovec_new(); > spapr->cas_reboot = spapr_ovec_diff(ov5_updates, > ov5_cas_old, spapr->ov5_cas); > + spapr_ovec_cleanup(ov5_cas_old); > /* Now that processing is finished, set the radix/hash bit for the > * guest if it requested a valid mode; otherwise terminate the boot. */ > if (guest_radix) { > @@ -1640,6 +1641,7 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu, > (spapr_h_cas_compose_response(spapr, args[1], args[2], > ov5_updates) != 0); > } > + spapr_ovec_cleanup(ov1_guest); > > /* > * Ensure the guest asks for an interrupt mode we support; otherwise > diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c > index 8a06d3171e..498ca6d53b 100644 > --- a/target/ppc/kvm.c > +++ b/target/ppc/kvm.c > @@ -491,7 +491,8 @@ int kvm_arch_init_vcpu(CPUState *cs) > return ret; > } > > - idle_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, kvm_kick_cpu, cpu); > + if (!idle_timer) > + idle_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, kvm_kick_cpu, cpu); > > switch (cenv->mmu_model) { > case POWERPC_MMU_BOOKE206: > -- 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
On 7/16/19 8:24 AM, Shivaprasad G Bhat wrote: > @@ -827,6 +827,7 @@ int spapr_dt_drc(void *fdt, int offset, Object *owner, uint32_t drc_type_mask) > Object *obj; > SpaprDrc *drc; > SpaprDrcClass *drck; > + const char *drc_name = NULL; > uint32_t drc_index, drc_power_domain; > > if (!strstart(prop->type, "link<", NULL)) { > @@ -856,8 +857,10 @@ int spapr_dt_drc(void *fdt, int offset, Object *owner, uint32_t drc_type_mask) > g_array_append_val(drc_power_domains, drc_power_domain); > > /* ibm,drc-names */ > - drc_names = g_string_append(drc_names, spapr_drc_name(drc)); > + drc_name = spapr_drc_name(drc); > + drc_names = g_string_append(drc_names, drc_name); > drc_names = g_string_insert_len(drc_names, -1, "\0", 1); > + g_free((char *)drc_name); This sort of casting means that you got the original type wrong. r~
On Tue, Jul 16, 2019 at 07:44:24AM -0700, Richard Henderson wrote: > On 7/16/19 8:24 AM, Shivaprasad G Bhat wrote: > > @@ -827,6 +827,7 @@ int spapr_dt_drc(void *fdt, int offset, Object *owner, uint32_t drc_type_mask) > > Object *obj; > > SpaprDrc *drc; > > SpaprDrcClass *drck; > > + const char *drc_name = NULL; > > uint32_t drc_index, drc_power_domain; > > > > if (!strstart(prop->type, "link<", NULL)) { > > @@ -856,8 +857,10 @@ int spapr_dt_drc(void *fdt, int offset, Object *owner, uint32_t drc_type_mask) > > g_array_append_val(drc_power_domains, drc_power_domain); > > > > /* ibm,drc-names */ > > - drc_names = g_string_append(drc_names, spapr_drc_name(drc)); > > + drc_name = spapr_drc_name(drc); > > + drc_names = g_string_append(drc_names, drc_name); > > drc_names = g_string_insert_len(drc_names, -1, "\0", 1); > > + g_free((char *)drc_name); > > This sort of casting means that you got the original type wrong. Ah, good point. And spapr_drc_name() has the wrong type also. I've pulled this patch out of my ppc-for-4.2 tree until this is fixed up. -- 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
© 2016 - 2024 Red Hat, Inc.