[Qemu-devel] [PATCH v2 00/16] hw: Use object_initialize_child for correct reference counting

Philippe Mathieu-Daudé posted 16 patches 4 years, 12 months ago
Test asan passed
Test docker-mingw@fedora passed
Test docker-clang@ubuntu passed
Test checkpatch passed
Failed in applying to current master (apply log)
hw/arm/aspeed.c                      |  6 +--
hw/arm/aspeed_soc.c                  | 50 +++++++++--------------
hw/arm/bcm2835_peripherals.c         | 61 +++++++++++-----------------
hw/arm/digic.c                       | 17 +++-----
hw/arm/imx25_pdk.c                   |  5 +--
hw/arm/kzm.c                         |  5 +--
hw/arm/mps2-tz.c                     |  8 ++--
hw/arm/mps2.c                        |  8 ++--
hw/arm/raspi.c                       |  7 ++--
hw/arm/sabrelite.c                   |  5 +--
hw/arm/xlnx-zcu102.c                 |  5 +--
hw/arm/xlnx-zynqmp.c                 |  8 ++--
hw/intc/armv7m_nvic.c                |  6 +--
hw/microblaze/xlnx-zynqmp-pmu.c      | 45 ++++++++++----------
hw/mips/boston.c                     | 25 ++++++------
hw/mips/cps.c                        | 20 ++++-----
hw/mips/mips_malta.c                 | 17 ++++----
hw/misc/macio/macio.c                |  8 ++--
hw/ppc/pnv.c                         | 12 ++----
hw/virtio/virtio.c                   |  5 +--
include/hw/arm/bcm2835_peripherals.h |  3 +-
21 files changed, 140 insertions(+), 186 deletions(-)
[Qemu-devel] [PATCH v2 00/16] hw: Use object_initialize_child for correct reference counting
Posted by Philippe Mathieu-Daudé 4 years, 12 months ago
Hi,

This series looks at Eduardo suggestions from [1]
and Thomas commit aff39be0ed97 to replace various
object_initialize + qdev_set_parent_bus calls by
sysbus_init_child_obj().

Important comment from Eduardo:

  It's possible, but we need a volunteer to review each
  hunk because the existing code might be (correctly)
  calling object_unref() (either immediately or when
  parent is finalized).

I tried to split it enough to make the review process
easier.

Regards,

Phil.

[*] https://patchwork.ozlabs.org/patch/943333/#1953608
v1: https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg05931.html

Philippe Mathieu-Daudé (16):
  hw/ppc/pnv: Use object_initialize_child for correct reference counting
  hw/misc/macio: Use object_initialize_child for correct ref. counting
  hw/virtio: Use object_initialize_child for correct reference counting
  hw/arm/bcm2835: Use TYPE_PL011 instead of hardcoded string
  hw/arm/bcm2835: Use object_initialize() on PL011State
  hw/arm/bcm2835: Use object_initialize_child for correct ref. counting
  hw/arm/aspeed: Use object_initialize_child for correct ref. counting
  hw/arm: Use object_initialize_child for correct reference counting
  hw/mips: Use object_initialize() on MIPSCPSState
  hw/mips: Use object_initialize_child for correct reference counting
  hw/microblaze/zynqmp: Move the IPI state into the PMUSoC state
  hw/microblaze/zynqmp: Let the SoC manage the IPI devices
  hw/microblaze/zynqmp: Use object_initialize_child for correct ref.
    counting
  hw/microblaze/zynqmp: Use object_initialize_child for correct ref.
    counting
  hw/arm/mps2: Use object_initialize_child for correct reference
    counting
  hw/intc/nvic: Use object_initialize_child for correct reference
    counting

 hw/arm/aspeed.c                      |  6 +--
 hw/arm/aspeed_soc.c                  | 50 +++++++++--------------
 hw/arm/bcm2835_peripherals.c         | 61 +++++++++++-----------------
 hw/arm/digic.c                       | 17 +++-----
 hw/arm/imx25_pdk.c                   |  5 +--
 hw/arm/kzm.c                         |  5 +--
 hw/arm/mps2-tz.c                     |  8 ++--
 hw/arm/mps2.c                        |  8 ++--
 hw/arm/raspi.c                       |  7 ++--
 hw/arm/sabrelite.c                   |  5 +--
 hw/arm/xlnx-zcu102.c                 |  5 +--
 hw/arm/xlnx-zynqmp.c                 |  8 ++--
 hw/intc/armv7m_nvic.c                |  6 +--
 hw/microblaze/xlnx-zynqmp-pmu.c      | 45 ++++++++++----------
 hw/mips/boston.c                     | 25 ++++++------
 hw/mips/cps.c                        | 20 ++++-----
 hw/mips/mips_malta.c                 | 17 ++++----
 hw/misc/macio/macio.c                |  8 ++--
 hw/ppc/pnv.c                         | 12 ++----
 hw/virtio/virtio.c                   |  5 +--
 include/hw/arm/bcm2835_peripherals.h |  3 +-
 21 files changed, 140 insertions(+), 186 deletions(-)

-- 
2.20.1


Re: [Qemu-devel] [PATCH v2 00/16] hw: Use object_initialize_child for correct reference counting
Posted by Philippe Mathieu-Daudé 4 years, 11 months ago
Hi Eduardo,

On 5/7/19 6:34 PM, Philippe Mathieu-Daudé wrote:
> Hi,
> 
> This series looks at Eduardo suggestions from [1]
> and Thomas commit aff39be0ed97 to replace various
> object_initialize + qdev_set_parent_bus calls by
> sysbus_init_child_obj().

Do you think you can take this series?
Else, via which tree it should go?

Thanks!

Phil.

> 
> Important comment from Eduardo:
> 
>   It's possible, but we need a volunteer to review each
>   hunk because the existing code might be (correctly)
>   calling object_unref() (either immediately or when
>   parent is finalized).
> 
> I tried to split it enough to make the review process
> easier.
> 
> Regards,
> 
> Phil.
> 
> [*] https://patchwork.ozlabs.org/patch/943333/#1953608
> v1: https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg05931.html
> 
> Philippe Mathieu-Daudé (16):
>   hw/ppc/pnv: Use object_initialize_child for correct reference counting
>   hw/misc/macio: Use object_initialize_child for correct ref. counting
>   hw/virtio: Use object_initialize_child for correct reference counting
>   hw/arm/bcm2835: Use TYPE_PL011 instead of hardcoded string
>   hw/arm/bcm2835: Use object_initialize() on PL011State
>   hw/arm/bcm2835: Use object_initialize_child for correct ref. counting
>   hw/arm/aspeed: Use object_initialize_child for correct ref. counting
>   hw/arm: Use object_initialize_child for correct reference counting
>   hw/mips: Use object_initialize() on MIPSCPSState
>   hw/mips: Use object_initialize_child for correct reference counting
>   hw/microblaze/zynqmp: Move the IPI state into the PMUSoC state
>   hw/microblaze/zynqmp: Let the SoC manage the IPI devices
>   hw/microblaze/zynqmp: Use object_initialize_child for correct ref.
>     counting
>   hw/microblaze/zynqmp: Use object_initialize_child for correct ref.
>     counting
>   hw/arm/mps2: Use object_initialize_child for correct reference
>     counting
>   hw/intc/nvic: Use object_initialize_child for correct reference
>     counting

Re: [Qemu-devel] [PATCH v2 00/16] hw: Use object_initialize_child for correct reference counting
Posted by Eduardo Habkost 4 years, 11 months ago
On Fri, May 17, 2019 at 12:32:18PM +0200, Philippe Mathieu-Daudé wrote:
> Hi Eduardo,
> 
> On 5/7/19 6:34 PM, Philippe Mathieu-Daudé wrote:
> > Hi,
> > 
> > This series looks at Eduardo suggestions from [1]
> > and Thomas commit aff39be0ed97 to replace various
> > object_initialize + qdev_set_parent_bus calls by
> > sysbus_init_child_obj().
> 
> Do you think you can take this series?
> Else, via which tree it should go?

I was expecting the maintainers of each architecture to apply the
patches for their areas.  But I'd be glad to merge it through my
tree if it makes it easier for everybody.

Are the arm, microblaze, mips, and ppc maintainers OK with that?

> 
> Thanks!
> 
> Phil.
> 
> > 
> > Important comment from Eduardo:
> > 
> >   It's possible, but we need a volunteer to review each
> >   hunk because the existing code might be (correctly)
> >   calling object_unref() (either immediately or when
> >   parent is finalized).
> > 
> > I tried to split it enough to make the review process
> > easier.
> > 
> > Regards,
> > 
> > Phil.
> > 
> > [*] https://patchwork.ozlabs.org/patch/943333/#1953608
> > v1: https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg05931.html
> > 
> > Philippe Mathieu-Daudé (16):
> >   hw/ppc/pnv: Use object_initialize_child for correct reference counting
> >   hw/misc/macio: Use object_initialize_child for correct ref. counting
> >   hw/virtio: Use object_initialize_child for correct reference counting
> >   hw/arm/bcm2835: Use TYPE_PL011 instead of hardcoded string
> >   hw/arm/bcm2835: Use object_initialize() on PL011State
> >   hw/arm/bcm2835: Use object_initialize_child for correct ref. counting
> >   hw/arm/aspeed: Use object_initialize_child for correct ref. counting
> >   hw/arm: Use object_initialize_child for correct reference counting
> >   hw/mips: Use object_initialize() on MIPSCPSState
> >   hw/mips: Use object_initialize_child for correct reference counting
> >   hw/microblaze/zynqmp: Move the IPI state into the PMUSoC state
> >   hw/microblaze/zynqmp: Let the SoC manage the IPI devices
> >   hw/microblaze/zynqmp: Use object_initialize_child for correct ref.
> >     counting
> >   hw/microblaze/zynqmp: Use object_initialize_child for correct ref.
> >     counting
> >   hw/arm/mps2: Use object_initialize_child for correct reference
> >     counting
> >   hw/intc/nvic: Use object_initialize_child for correct reference
> >     counting

-- 
Eduardo

Re: [Qemu-devel] [PATCH v2 00/16] hw: Use object_initialize_child for correct reference counting
Posted by Peter Maydell 4 years, 11 months ago
On Fri, 17 May 2019 at 18:56, Eduardo Habkost <ehabkost@redhat.com> wrote:
>
> On Fri, May 17, 2019 at 12:32:18PM +0200, Philippe Mathieu-Daudé wrote:
> > Hi Eduardo,
> >
> > On 5/7/19 6:34 PM, Philippe Mathieu-Daudé wrote:
> > > Hi,
> > >
> > > This series looks at Eduardo suggestions from [1]
> > > and Thomas commit aff39be0ed97 to replace various
> > > object_initialize + qdev_set_parent_bus calls by
> > > sysbus_init_child_obj().
> >
> > Do you think you can take this series?
> > Else, via which tree it should go?
>
> I was expecting the maintainers of each architecture to apply the
> patches for their areas.

This in my experience rarely happens, because splitting up
a patchset is effort and there's a coordination problem
working out who's going to take which patches -- it's why it
works better to have several series each of which covers one
submaintainer's area, rather than one big series which then
doesn't have an obvious path into the tree.

(Personally I also tend to treat omnibus patch series as
"somebody else's problem" whereas patch series that are
mostly or entirely arm changes go on my list as needing
to be dealt with...)

> Are the arm, microblaze, mips, and ppc maintainers OK with that?

No objections for arm if the patches have been reviewed.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 00/16] hw: Use object_initialize_child for correct reference counting
Posted by Aleksandar Markovic 4 years, 11 months ago
> > On 5/7/19 6:34 PM, Philippe Mathieu-Daudé wrote:
> > > Hi,
> > >
> > > This series looks at Eduardo suggestions from [1]
> > > and Thomas commit aff39be0ed97 to replace various
> > > object_initialize + qdev_set_parent_bus calls by
> > > sysbus_init_child_obj().
> >
> > Do you think you can take this series?
> > Else, via which tree it should go?
>
> I was expecting the maintainers of each architecture to apply the
> patches for their areas.  But I'd be glad to merge it through my
> tree if it makes it easier for everybody.
>
> Are the arm, microblaze, mips, and ppc maintainers OK with that?

Hello, Eduardo.

I am OK with two patches applicable to MIPS. Moreover, I am going
to apply them to my pull request scheduled to be sent today. Sorry
if that makes your part more difficult (you will have to skip two
patches from this series). The reason for my urgency is that we in
Wave start regression testing for QEMU 4.1 in MIPS environments, and
we wanted these two patches integrated sooner rather than later.

Thanks to everyone involved!

Aleksandar

________________________________________
From: Eduardo Habkost <ehabkost@redhat.com>
Sent: Friday, May 17, 2019 7:56:21 PM
To: Philippe Mathieu-Daudé
Cc: Markus Armbruster; Thomas Huth; qemu-devel@nongnu.org; Peter Maydell; Philippe Mathieu-Daudé; qemu-arm@nongnu.org; Aleksandar Markovic; Andrew Jeffery; Peter Chubb; Alistair Francis; Cédric Le Goater; Aurelien Jarno; David Gibson; Paul Burton; Antony Pavlov; Andrew Baumann; Joel Stanley; Michael S. Tsirkin; Mark Cave-Ayland; qemu-ppc@nongnu.org; Edgar E. Iglesias; Aleksandar Rikalo; Jean-Christophe Dubois
Subject: Re: [PATCH v2 00/16] hw: Use object_initialize_child for correct reference counting

On Fri, May 17, 2019 at 12:32:18PM +0200, Philippe Mathieu-Daudé wrote:
> Hi Eduardo,
>
> On 5/7/19 6:34 PM, Philippe Mathieu-Daudé wrote:
> > Hi,
> >
> > This series looks at Eduardo suggestions from [1]
> > and Thomas commit aff39be0ed97 to replace various
> > object_initialize + qdev_set_parent_bus calls by
> > sysbus_init_child_obj().
>
> Do you think you can take this series?
> Else, via which tree it should go?

I was expecting the maintainers of each architecture to apply the
patches for their areas.  But I'd be glad to merge it through my
tree if it makes it easier for everybody.

Are the arm, microblaze, mips, and ppc maintainers OK with that?

>
> Thanks!
>
> Phil.
>
> >
> > Important comment from Eduardo:
> >
> >   It's possible, but we need a volunteer to review each
> >   hunk because the existing code might be (correctly)
> >   calling object_unref() (either immediately or when
> >   parent is finalized).
> >
> > I tried to split it enough to make the review process
> > easier.
> >
> > Regards,
> >
> > Phil.
> >
> > [*] https://patchwork.ozlabs.org/patch/943333/#1953608
> > v1: https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg05931.html
> >
> > Philippe Mathieu-Daudé (16):
> >   hw/ppc/pnv: Use object_initialize_child for correct reference counting
> >   hw/misc/macio: Use object_initialize_child for correct ref. counting
> >   hw/virtio: Use object_initialize_child for correct reference counting
> >   hw/arm/bcm2835: Use TYPE_PL011 instead of hardcoded string
> >   hw/arm/bcm2835: Use object_initialize() on PL011State
> >   hw/arm/bcm2835: Use object_initialize_child for correct ref. counting
> >   hw/arm/aspeed: Use object_initialize_child for correct ref. counting
> >   hw/arm: Use object_initialize_child for correct reference counting
> >   hw/mips: Use object_initialize() on MIPSCPSState
> >   hw/mips: Use object_initialize_child for correct reference counting
> >   hw/microblaze/zynqmp: Move the IPI state into the PMUSoC state
> >   hw/microblaze/zynqmp: Let the SoC manage the IPI devices
> >   hw/microblaze/zynqmp: Use object_initialize_child for correct ref.
> >     counting
> >   hw/microblaze/zynqmp: Use object_initialize_child for correct ref.
> >     counting
> >   hw/arm/mps2: Use object_initialize_child for correct reference
> >     counting
> >   hw/intc/nvic: Use object_initialize_child for correct reference
> >     counting

--
Eduardo