On 2025/09/19 5:11, Peter Xu wrote:
On Thu, Sep 18, 2025 at 04:03:49PM -0400, Peter Xu wrote:
On Wed, Sep 17, 2025 at 07:13:26PM +0900, Akihiko Odaki wrote:
Children are automatically unparented so manually unparenting is
unnecessary.
Worse, automatic unparenting happens before the instance_finalize()
callback of the parent gets called, so object_unparent() calls in
the callback will refer to objects that are already unparented, which
is semantically incorrect.
Remove the instruction to call object_unparent(), and the exception
of the "do not call object_unparent()" rule for instance_finalize().
Signed-off-by: Akihiko Odaki <od...@rsg.ci.i.u-tokyo.ac.jp>
---
docs/devel/memory.rst | 19 ++++++-------------
1 file changed, 6 insertions(+), 13 deletions(-)
diff --git a/docs/devel/memory.rst b/docs/devel/memory.rst
index 57fb2aec76e0..749f11d8a4dd 100644
--- a/docs/devel/memory.rst
+++ b/docs/devel/memory.rst
@@ -161,18 +161,11 @@ or never.
Destruction of a memory region happens automatically when the owner
object dies.
-If however the memory region is part of a dynamically allocated data
-structure, you should call object_unparent() to destroy the memory region
-before the data structure is freed. For an example see VFIOMSIXInfo
-and VFIOQuirk in hw/vfio/pci.c.
Should we still keep some of these examples? After the series they'll be
doing the right things. Dynamic MRs are still slightly tricky, I think
it's still good to have some references.
I agree. I'll restore it with the next version.
-
You must not destroy a memory region as long as it may be in use by a
device or CPU. In order to do this, as a general rule do not create or
-destroy memory regions dynamically during a device's lifetime, and only
-call object_unparent() in the memory region owner's instance_finalize
-callback. The dynamically allocated data structure that contains the
-memory region then should obviously be freed in the instance_finalize
-callback as well.
+destroy memory regions dynamically during a device's lifetime.
+The dynamically allocated data structure that contains the
+memory region should be freed in the instance_finalize callback.
If you break this rule, the following situation can happen:
@@ -198,9 +191,9 @@ this exception is rarely necessary, and therefore it is discouraged,
but nevertheless it is used in a few places.
For regions that "have no owner" (NULL is passed at creation time), the
-machine object is actually used as the owner. Since instance_finalize is
-never called for the machine object, you must never call object_unparent
-on regions that have no owner, unless they are aliases or containers.
+machine object is actually used as the owner. You must never call
+object_unparent on regions that have no owner, unless they are aliases
+or containers.
This looks like a completely separate change. So we start to allow
machines to be finalized now? I'm not familiar with machine object
lifecycles. Maybe split it out even if it's true?
I intended to remove phrase "since instance_finalize is never called for
the machine object" because whether instance_finalize is called or not
is no longer relevant, and thus object_unparent is always prohibited,
whether regions have owners or not, unless they are aliases or containers.
The statement still mentions "regions that have no owner"; the
restriction of object_unparent is enforced whether the regions have
owners, so it is a bit misleading.
I didn't see anything elsewhere. If you agree with above, I can queue this
series with above touched up, then no need to repost.
I guess I will rewrite this patch, restoring the VFIOQuirk example, and
re-check if this whole section is structured logically and consistently.
Regards,
Akihiko Odaki
