Re: QEMU device refcounting when device creates a container MR

[Igor Mammedov]

On Wed, 9 Mar 2022 16:56:21 +0000
Peter Maydell <peter.maydell@linaro.org> wrote:

> On Wed, 9 Mar 2022 at 16:53, Peter Maydell <peter.maydell@linaro.org> wrote:
> >
> > On Wed, 9 Mar 2022 at 16:21, Paolo Bonzini <pbonzini@redhat.com> wrote:  
> > >
> > > On 3/9/22 11:33, Peter Maydell wrote:  
> > > > Hi; does anybody know how device reference counting is supposed
> > > > to work when the device creates a "container" MemoryRegion which
> > > > it then puts some of its own subregions in to?
> > > >
> > > > As far as I can see when you do memory_region_add_subregion it
> > > > increases the refcount on the owner of the subregion. So if a
> > > > device creates a container MR in its own init or realize method
> > > > and adds sub-MRs that it owns to that container, this increases
> > > > the refcount on the device permanently, and so the device won't
> > > > ever be deinited.  
> > >
> > > The unparent method is supposed to break reference cycles.
> > >
> > > In the case of QOM, unparent calls unrealize, so unrealize should remove
> > > the subregions it created.  

well, making it asymmetric doesn't sound nice.
I think unrealize() should deal only with stuff initialized in
realize(), and instance_finalize() undo whatever  instance_init() did.
And if we got beyond Device type, i.e. directly inherited from "Object"
(/me thinking about backends) attempt to hijack unrealize() won't help.

> > It seems asymmetric for unrealize to undo something that was
> > done in instance_init, though. I would expect unrealize to
> > undo the effects of realize, and instance_finalize to undo
> > the effects of instance_init.  
> 
> ...also, in the device-introspect-test where I see this problem,
> unrealize is never going to be called anyway, because the device
> is only put through "instance_init" and then dereffed (which
> does not result in instance_finalize being called, because the
> refcount is still non-zero).

question is why introspected device is deferred instead of being
destroyed if it's no longer needed?

> 
> -- PMM
>