* [Qemu-devel] [PATCH 0/2] s390x: cpu cleanups
@ 2017-10-17 13:41 Igor Mammedov
2017-10-17 13:41 ` [Qemu-devel] [PATCH 1/2] s390x: fix cpu object referrence leak in s390x_new_cpu() Igor Mammedov
2017-10-17 13:41 ` [Qemu-devel] [PATCH 2/2] s390x: move s390x_new_cpu() into board code Igor Mammedov
0 siblings, 2 replies; 8+ messages in thread
From: Igor Mammedov @ 2017-10-17 13:41 UTC (permalink / raw)
To: qemu-devel
Cc: Richard Henderson, Alexander Graf, Cornelia Huck, Christian Borntraeger
series includes remaining in my queue non cpu_model refactoring
patches that fix and cleanup cpu_init code
CC: Richard Henderson <rth@twiddle.net>
CC: Alexander Graf <agraf@suse.de>
CC: Cornelia Huck <cohuck@redhat.com>
CC: Christian Borntraeger <borntraeger@de.ibm.com>
Igor Mammedov (2):
s390x: fix cpu object referrence leak in s390x_new_cpu()
s390x: move s390x_new_cpu() into board code
target/s390x/cpu.h | 1 -
hw/s390x/s390-virtio-ccw.c | 21 +++++++++++++++++++++
target/s390x/helper.c | 20 --------------------
3 files changed, 21 insertions(+), 21 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH 1/2] s390x: fix cpu object referrence leak in s390x_new_cpu()
2017-10-17 13:41 [Qemu-devel] [PATCH 0/2] s390x: cpu cleanups Igor Mammedov
@ 2017-10-17 13:41 ` Igor Mammedov
2017-10-17 14:28 ` Cornelia Huck
2017-10-17 15:49 ` Cornelia Huck
2017-10-17 13:41 ` [Qemu-devel] [PATCH 2/2] s390x: move s390x_new_cpu() into board code Igor Mammedov
1 sibling, 2 replies; 8+ messages in thread
From: Igor Mammedov @ 2017-10-17 13:41 UTC (permalink / raw)
To: qemu-devel
Cc: Richard Henderson, Alexander Graf, Cornelia Huck, Christian Borntraeger
object_new() returns cpu with refcnt == 1 and after realize
refcnt == 2*. s390x_new_cpu() as an owner of the first refcnt
should have released it on exit in both cases (on error and
success) to avoid it leaking. Do so for both cases.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
target/s390x/helper.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/s390x/helper.c b/target/s390x/helper.c
index 97adbcc..64d4c48 100644
--- a/target/s390x/helper.c
+++ b/target/s390x/helper.c
@@ -80,9 +80,9 @@ S390CPU *s390x_new_cpu(const char *typename, uint32_t core_id, Error **errp)
object_property_set_bool(OBJECT(cpu), true, "realized", &err);
out:
+ object_unref(OBJECT(cpu));
if (err) {
error_propagate(errp, err);
- object_unref(OBJECT(cpu));
cpu = NULL;
}
return cpu;
--
2.7.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH 2/2] s390x: move s390x_new_cpu() into board code
2017-10-17 13:41 [Qemu-devel] [PATCH 0/2] s390x: cpu cleanups Igor Mammedov
2017-10-17 13:41 ` [Qemu-devel] [PATCH 1/2] s390x: fix cpu object referrence leak in s390x_new_cpu() Igor Mammedov
@ 2017-10-17 13:41 ` Igor Mammedov
2017-10-17 14:29 ` Cornelia Huck
1 sibling, 1 reply; 8+ messages in thread
From: Igor Mammedov @ 2017-10-17 13:41 UTC (permalink / raw)
To: qemu-devel
Cc: Richard Henderson, Alexander Graf, Cornelia Huck, Christian Borntraeger
s390-virtio-ccw.c is the sole user of s390x_new_cpu(),
so move this helper there.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
target/s390x/cpu.h | 1 -
hw/s390x/s390-virtio-ccw.c | 21 +++++++++++++++++++++
target/s390x/helper.c | 20 --------------------
3 files changed, 21 insertions(+), 21 deletions(-)
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 7e864c8..b177be9 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -695,7 +695,6 @@ void s390_cpu_list(FILE *f, fprintf_function cpu_fprintf);
/* helper.c */
#define cpu_init(cpu_model) cpu_generic_init(TYPE_S390_CPU, cpu_model)
-S390CPU *s390x_new_cpu(const char *typename, uint32_t core_id, Error **errp);
#define S390_CPU_TYPE_SUFFIX "-" TYPE_S390_CPU
#define S390_CPU_TYPE_NAME(name) (name S390_CPU_TYPE_SUFFIX)
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 32d3f11..f64db51 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -52,6 +52,27 @@ S390CPU *s390_cpu_addr2state(uint16_t cpu_addr)
return S390_CPU(ms->possible_cpus->cpus[cpu_addr].cpu);
}
+static
+S390CPU *s390x_new_cpu(const char *typename, uint32_t core_id, Error **errp)
+{
+ S390CPU *cpu = S390_CPU(object_new(typename));
+ Error *err = NULL;
+
+ object_property_set_int(OBJECT(cpu), core_id, "core-id", &err);
+ if (err != NULL) {
+ goto out;
+ }
+ object_property_set_bool(OBJECT(cpu), true, "realized", &err);
+
+out:
+ object_unref(OBJECT(cpu));
+ if (err) {
+ error_propagate(errp, err);
+ cpu = NULL;
+ }
+ return cpu;
+}
+
static void s390_init_cpus(MachineState *machine)
{
MachineClass *mc = MACHINE_GET_CLASS(machine);
diff --git a/target/s390x/helper.c b/target/s390x/helper.c
index 64d4c48..52aa64b 100644
--- a/target/s390x/helper.c
+++ b/target/s390x/helper.c
@@ -68,26 +68,6 @@ void s390x_cpu_timer(void *opaque)
}
#endif
-S390CPU *s390x_new_cpu(const char *typename, uint32_t core_id, Error **errp)
-{
- S390CPU *cpu = S390_CPU(object_new(typename));
- Error *err = NULL;
-
- object_property_set_int(OBJECT(cpu), core_id, "core-id", &err);
- if (err != NULL) {
- goto out;
- }
- object_property_set_bool(OBJECT(cpu), true, "realized", &err);
-
-out:
- object_unref(OBJECT(cpu));
- if (err) {
- error_propagate(errp, err);
- cpu = NULL;
- }
- return cpu;
-}
-
#ifndef CONFIG_USER_ONLY
hwaddr s390_cpu_get_phys_page_debug(CPUState *cs, vaddr vaddr)
--
2.7.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] s390x: fix cpu object referrence leak in s390x_new_cpu()
2017-10-17 13:41 ` [Qemu-devel] [PATCH 1/2] s390x: fix cpu object referrence leak in s390x_new_cpu() Igor Mammedov
@ 2017-10-17 14:28 ` Cornelia Huck
2017-10-17 15:49 ` Cornelia Huck
1 sibling, 0 replies; 8+ messages in thread
From: Cornelia Huck @ 2017-10-17 14:28 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, Richard Henderson, Alexander Graf,
Christian Borntraeger, qemu-s390x
On Tue, 17 Oct 2017 15:41:19 +0200
Igor Mammedov <imammedo@redhat.com> wrote:
> object_new() returns cpu with refcnt == 1 and after realize
> refcnt == 2*. s390x_new_cpu() as an owner of the first refcnt
> should have released it on exit in both cases (on error and
> success) to avoid it leaking. Do so for both cases.
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> target/s390x/helper.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/s390x/helper.c b/target/s390x/helper.c
> index 97adbcc..64d4c48 100644
> --- a/target/s390x/helper.c
> +++ b/target/s390x/helper.c
> @@ -80,9 +80,9 @@ S390CPU *s390x_new_cpu(const char *typename, uint32_t core_id, Error **errp)
> object_property_set_bool(OBJECT(cpu), true, "realized", &err);
>
> out:
> + object_unref(OBJECT(cpu));
> if (err) {
> error_propagate(errp, err);
> - object_unref(OBJECT(cpu));
> cpu = NULL;
> }
> return cpu;
Looks reasonable.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] s390x: move s390x_new_cpu() into board code
2017-10-17 13:41 ` [Qemu-devel] [PATCH 2/2] s390x: move s390x_new_cpu() into board code Igor Mammedov
@ 2017-10-17 14:29 ` Cornelia Huck
2017-10-17 15:13 ` [Qemu-devel] [PATCH v2 " Igor Mammedov
0 siblings, 1 reply; 8+ messages in thread
From: Cornelia Huck @ 2017-10-17 14:29 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, Richard Henderson, Alexander Graf,
Christian Borntraeger, qemu-s390x
On Tue, 17 Oct 2017 15:41:20 +0200
Igor Mammedov <imammedo@redhat.com> wrote:
> s390-virtio-ccw.c is the sole user of s390x_new_cpu(),
> so move this helper there.
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> target/s390x/cpu.h | 1 -
> hw/s390x/s390-virtio-ccw.c | 21 +++++++++++++++++++++
> target/s390x/helper.c | 20 --------------------
> 3 files changed, 21 insertions(+), 21 deletions(-)
>
> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
> index 7e864c8..b177be9 100644
> --- a/target/s390x/cpu.h
> +++ b/target/s390x/cpu.h
> @@ -695,7 +695,6 @@ void s390_cpu_list(FILE *f, fprintf_function cpu_fprintf);
>
> /* helper.c */
> #define cpu_init(cpu_model) cpu_generic_init(TYPE_S390_CPU, cpu_model)
> -S390CPU *s390x_new_cpu(const char *typename, uint32_t core_id, Error **errp);
>
> #define S390_CPU_TYPE_SUFFIX "-" TYPE_S390_CPU
> #define S390_CPU_TYPE_NAME(name) (name S390_CPU_TYPE_SUFFIX)
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index 32d3f11..f64db51 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -52,6 +52,27 @@ S390CPU *s390_cpu_addr2state(uint16_t cpu_addr)
> return S390_CPU(ms->possible_cpus->cpus[cpu_addr].cpu);
> }
>
> +static
> +S390CPU *s390x_new_cpu(const char *typename, uint32_t core_id, Error **errp)
I don't like this wrap; better wrap the argument list.
> +{
> + S390CPU *cpu = S390_CPU(object_new(typename));
> + Error *err = NULL;
> +
> + object_property_set_int(OBJECT(cpu), core_id, "core-id", &err);
> + if (err != NULL) {
> + goto out;
> + }
> + object_property_set_bool(OBJECT(cpu), true, "realized", &err);
> +
> +out:
> + object_unref(OBJECT(cpu));
> + if (err) {
> + error_propagate(errp, err);
> + cpu = NULL;
> + }
> + return cpu;
> +}
> +
> static void s390_init_cpus(MachineState *machine)
> {
> MachineClass *mc = MACHINE_GET_CLASS(machine);
> diff --git a/target/s390x/helper.c b/target/s390x/helper.c
> index 64d4c48..52aa64b 100644
> --- a/target/s390x/helper.c
> +++ b/target/s390x/helper.c
> @@ -68,26 +68,6 @@ void s390x_cpu_timer(void *opaque)
> }
> #endif
>
> -S390CPU *s390x_new_cpu(const char *typename, uint32_t core_id, Error **errp)
> -{
> - S390CPU *cpu = S390_CPU(object_new(typename));
> - Error *err = NULL;
> -
> - object_property_set_int(OBJECT(cpu), core_id, "core-id", &err);
> - if (err != NULL) {
> - goto out;
> - }
> - object_property_set_bool(OBJECT(cpu), true, "realized", &err);
> -
> -out:
> - object_unref(OBJECT(cpu));
> - if (err) {
> - error_propagate(errp, err);
> - cpu = NULL;
> - }
> - return cpu;
> -}
> -
> #ifndef CONFIG_USER_ONLY
>
> hwaddr s390_cpu_get_phys_page_debug(CPUState *cs, vaddr vaddr)
Otherwise, looks sane.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH v2 2/2] s390x: move s390x_new_cpu() into board code
2017-10-17 14:29 ` Cornelia Huck
@ 2017-10-17 15:13 ` Igor Mammedov
2017-10-17 15:50 ` Cornelia Huck
0 siblings, 1 reply; 8+ messages in thread
From: Igor Mammedov @ 2017-10-17 15:13 UTC (permalink / raw)
To: qemu-devel; +Cc: Cornelia Huck
s390-virtio-ccw.c is the sole user of s390x_new_cpu(),
so move this helper there.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
CC: Cornelia Huck <cohuck@redhat.com>
v2:
- style fix: wrap the argument list
---
target/s390x/cpu.h | 1 -
hw/s390x/s390-virtio-ccw.c | 21 +++++++++++++++++++++
target/s390x/helper.c | 20 --------------------
3 files changed, 21 insertions(+), 21 deletions(-)
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 7e864c8..b177be9 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -695,7 +695,6 @@ void s390_cpu_list(FILE *f, fprintf_function cpu_fprintf);
/* helper.c */
#define cpu_init(cpu_model) cpu_generic_init(TYPE_S390_CPU, cpu_model)
-S390CPU *s390x_new_cpu(const char *typename, uint32_t core_id, Error **errp);
#define S390_CPU_TYPE_SUFFIX "-" TYPE_S390_CPU
#define S390_CPU_TYPE_NAME(name) (name S390_CPU_TYPE_SUFFIX)
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 32d3f11..e593c71 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -52,6 +52,27 @@ S390CPU *s390_cpu_addr2state(uint16_t cpu_addr)
return S390_CPU(ms->possible_cpus->cpus[cpu_addr].cpu);
}
+static S390CPU *s390x_new_cpu(const char *typename, uint32_t core_id,
+ Error **errp)
+{
+ S390CPU *cpu = S390_CPU(object_new(typename));
+ Error *err = NULL;
+
+ object_property_set_int(OBJECT(cpu), core_id, "core-id", &err);
+ if (err != NULL) {
+ goto out;
+ }
+ object_property_set_bool(OBJECT(cpu), true, "realized", &err);
+
+out:
+ object_unref(OBJECT(cpu));
+ if (err) {
+ error_propagate(errp, err);
+ cpu = NULL;
+ }
+ return cpu;
+}
+
static void s390_init_cpus(MachineState *machine)
{
MachineClass *mc = MACHINE_GET_CLASS(machine);
diff --git a/target/s390x/helper.c b/target/s390x/helper.c
index 64d4c48..52aa64b 100644
--- a/target/s390x/helper.c
+++ b/target/s390x/helper.c
@@ -68,26 +68,6 @@ void s390x_cpu_timer(void *opaque)
}
#endif
-S390CPU *s390x_new_cpu(const char *typename, uint32_t core_id, Error **errp)
-{
- S390CPU *cpu = S390_CPU(object_new(typename));
- Error *err = NULL;
-
- object_property_set_int(OBJECT(cpu), core_id, "core-id", &err);
- if (err != NULL) {
- goto out;
- }
- object_property_set_bool(OBJECT(cpu), true, "realized", &err);
-
-out:
- object_unref(OBJECT(cpu));
- if (err) {
- error_propagate(errp, err);
- cpu = NULL;
- }
- return cpu;
-}
-
#ifndef CONFIG_USER_ONLY
hwaddr s390_cpu_get_phys_page_debug(CPUState *cs, vaddr vaddr)
--
2.7.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] s390x: fix cpu object referrence leak in s390x_new_cpu()
2017-10-17 13:41 ` [Qemu-devel] [PATCH 1/2] s390x: fix cpu object referrence leak in s390x_new_cpu() Igor Mammedov
2017-10-17 14:28 ` Cornelia Huck
@ 2017-10-17 15:49 ` Cornelia Huck
1 sibling, 0 replies; 8+ messages in thread
From: Cornelia Huck @ 2017-10-17 15:49 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, Richard Henderson, Alexander Graf,
Christian Borntraeger, qemu-s390x
On Tue, 17 Oct 2017 15:41:19 +0200
Igor Mammedov <imammedo@redhat.com> wrote:
> object_new() returns cpu with refcnt == 1 and after realize
> refcnt == 2*. s390x_new_cpu() as an owner of the first refcnt
> should have released it on exit in both cases (on error and
> success) to avoid it leaking. Do so for both cases.
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> target/s390x/helper.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/s390x/helper.c b/target/s390x/helper.c
> index 97adbcc..64d4c48 100644
> --- a/target/s390x/helper.c
> +++ b/target/s390x/helper.c
> @@ -80,9 +80,9 @@ S390CPU *s390x_new_cpu(const char *typename, uint32_t core_id, Error **errp)
> object_property_set_bool(OBJECT(cpu), true, "realized", &err);
>
> out:
> + object_unref(OBJECT(cpu));
> if (err) {
> error_propagate(errp, err);
> - object_unref(OBJECT(cpu));
> cpu = NULL;
> }
> return cpu;
Thanks, applied.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/2] s390x: move s390x_new_cpu() into board code
2017-10-17 15:13 ` [Qemu-devel] [PATCH v2 " Igor Mammedov
@ 2017-10-17 15:50 ` Cornelia Huck
0 siblings, 0 replies; 8+ messages in thread
From: Cornelia Huck @ 2017-10-17 15:50 UTC (permalink / raw)
To: Igor Mammedov; +Cc: qemu-devel, qemu-s390x
On Tue, 17 Oct 2017 17:13:23 +0200
Igor Mammedov <imammedo@redhat.com> wrote:
> s390-virtio-ccw.c is the sole user of s390x_new_cpu(),
> so move this helper there.
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> CC: Cornelia Huck <cohuck@redhat.com>
>
> v2:
> - style fix: wrap the argument list
>
> ---
> target/s390x/cpu.h | 1 -
> hw/s390x/s390-virtio-ccw.c | 21 +++++++++++++++++++++
> target/s390x/helper.c | 20 --------------------
> 3 files changed, 21 insertions(+), 21 deletions(-)
Thanks, applied.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-10-17 15:50 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-17 13:41 [Qemu-devel] [PATCH 0/2] s390x: cpu cleanups Igor Mammedov
2017-10-17 13:41 ` [Qemu-devel] [PATCH 1/2] s390x: fix cpu object referrence leak in s390x_new_cpu() Igor Mammedov
2017-10-17 14:28 ` Cornelia Huck
2017-10-17 15:49 ` Cornelia Huck
2017-10-17 13:41 ` [Qemu-devel] [PATCH 2/2] s390x: move s390x_new_cpu() into board code Igor Mammedov
2017-10-17 14:29 ` Cornelia Huck
2017-10-17 15:13 ` [Qemu-devel] [PATCH v2 " Igor Mammedov
2017-10-17 15:50 ` Cornelia Huck
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.