[PULL 16/49] clock: treat outputs and inputs the same in NamedClockList

Paolo Bonzini posted 49 patches 4 months ago
[PULL 16/49] clock: treat outputs and inputs the same in NamedClockList
Posted by Paolo Bonzini 4 months ago
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/core/qdev-clock.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/hw/core/qdev-clock.c b/hw/core/qdev-clock.c
index ca65685c04e..2f9d6cb7579 100644
--- a/hw/core/qdev-clock.c
+++ b/hw/core/qdev-clock.c
@@ -48,14 +48,6 @@ static NamedClockList *qdev_init_clocklist(DeviceState *dev, const char *name,
     if (clk == NULL) {
         clk = CLOCK(object_new(TYPE_CLOCK));
         object_property_add_child(OBJECT(dev), name, OBJECT(clk));
-        if (output) {
-            /*
-             * Remove object_new()'s initial reference.
-             * Note that for inputs, the reference created by object_new()
-             * will be deleted in qdev_finalize_clocklist().
-             */
-            object_unref(OBJECT(clk));
-        }
     } else {
         object_property_add_link(OBJECT(dev), name,
                                  object_get_typename(OBJECT(clk)),
@@ -84,7 +76,7 @@ void qdev_finalize_clocklist(DeviceState *dev)
 
     QLIST_FOREACH_SAFE(ncl, &dev->clocks, node, ncl_next) {
         QLIST_REMOVE(ncl, node);
-        if (!ncl->output && !ncl->alias) {
+        if (!ncl->alias) {
             /*
              * We kept a reference on the input clock to ensure it lives up to
              * this point; it is used by the monitor to show the frequency.
-- 
2.47.1
Re: [PULL 16/49] clock: treat outputs and inputs the same in NamedClockList
Posted by Philippe Mathieu-Daudé 3 months, 2 weeks ago
Hi Paolo,

The commit description is missing... See:
https://lore.kernel.org/qemu-devel/20241129180326.722436-3-pbonzini@redhat.com/

 >> Leave around a reference not just for inputs but also for outputs.
 >> This is a better choice because in principle the monitor could walk
 >> the NamedClockList after the clocks have been unparented (which would
 >> free them) and before qdev_finalize_clocklist().

Any clue what happened?

On 11/12/24 17:26, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   hw/core/qdev-clock.c | 10 +---------
>   1 file changed, 1 insertion(+), 9 deletions(-)
> 
> diff --git a/hw/core/qdev-clock.c b/hw/core/qdev-clock.c
> index ca65685c04e..2f9d6cb7579 100644
> --- a/hw/core/qdev-clock.c
> +++ b/hw/core/qdev-clock.c
> @@ -48,14 +48,6 @@ static NamedClockList *qdev_init_clocklist(DeviceState *dev, const char *name,
>       if (clk == NULL) {
>           clk = CLOCK(object_new(TYPE_CLOCK));
>           object_property_add_child(OBJECT(dev), name, OBJECT(clk));
> -        if (output) {
> -            /*
> -             * Remove object_new()'s initial reference.
> -             * Note that for inputs, the reference created by object_new()
> -             * will be deleted in qdev_finalize_clocklist().
> -             */
> -            object_unref(OBJECT(clk));
> -        }
>       } else {
>           object_property_add_link(OBJECT(dev), name,
>                                    object_get_typename(OBJECT(clk)),
> @@ -84,7 +76,7 @@ void qdev_finalize_clocklist(DeviceState *dev)
>   
>       QLIST_FOREACH_SAFE(ncl, &dev->clocks, node, ncl_next) {
>           QLIST_REMOVE(ncl, node);
> -        if (!ncl->output && !ncl->alias) {
> +        if (!ncl->alias) {
>               /*
>                * We kept a reference on the input clock to ensure it lives up to
>                * this point; it is used by the monitor to show the frequency.