On Sat, Jan 11, 2025 at 01:15:24PM +0900, Akihiko Odaki wrote:
> On 2025/01/11 0:18, Peter Xu wrote:
> > On Fri, Jan 10, 2025 at 05:43:15PM +0900, Akihiko Odaki wrote:
> > > On 2025/01/10 4:37, Peter Xu wrote:
> > > > On Thu, Jan 09, 2025 at 02:29:21PM -0500, Peter Xu wrote:
> > > > > On Thu, Jan 09, 2025 at 01:30:35PM +0100, BALATON Zoltan wrote:
> > > > > > On Thu, 9 Jan 2025, Akihiko Odaki wrote:
> > > > > > > Do not refer to "memory region's reference count"
> > > > > > > -
> > > > > > >
> > > > > > > Now MemoryRegions do have their own reference counts, but they
> > > > > > > will not
> > > > > > > be used when their owners are not themselves. However, the
> > > > > > > documentation
> > > > > > > of memory_region_ref() says it adds "1 to a memory region's
> > > > > > > reference
> > > > > > > count", which is confusing. Avoid referring to "memory region's
> > > > > > > reference count" and just say: "Add a reference to a memory
> > > > > > > region".
> > > > > > > Make a similar change to memory_region_unref() too.
> > > > > > >
> > > > > > > Refer to docs/devel/memory.rst for "owner"
> > > > > > > --
> > > > > > >
> > > > > > > memory_region_ref() and memory_region_unref() used to have their
> > > > > > > own
> > > > > > > descriptions of "owner", but they are somewhat out-of-date and
> > > > > > > misleading.
> > > > > > >
> > > > > > > In particular, they say "whenever memory regions are accessed
> > > > > > > outside
> > > > > > > the BQL, they need to be preserved against hot-unplug", but
> > > > > > > protecting
> > > > > > > against hot-unplug is not mandatory if it is known that they will
> > > > > > > never
> > > > > > > be hot-unplugged. They also say "MemoryRegions actually do not
> > > > > > > have
> > > > > > > their own reference count", but they actually do. They just will
> > > > > > > not be
> > > > > > > used unless their owners are not themselves.
> > > > > > >
> > > > > > > Refer to docs/devel/memory.rst as the single source of truth
> > > > > > > instead of
> > > > > > > maintaining duplicate descriptions of "owner".
> > > > > > >
> > > > > > > Clarify that owner may be missing
> > > > > > >
> > > > > > > -
> > > > > > > A memory region may not have an owner, and memory_region_ref() and
> > > > > > > memory_region_unref() do nothing for such.
> > > > > > >
> > > > > > > memory: Clarify owner must not call memory_region_ref()
> > > > > > >
> > > > > > >
> > > > > > > The owner must not call this function as it results in a circular
> > > > > > > reference.
> > > > > > >
> > > > > > > Signed-off-by: Akihiko Odaki
> > > > > > > Reviewed-by: Peter Xu
> > > > > > > ---
> > > > > > > include/exec/memory.h | 59
> > > > > > > ---
> > > > > > > 1 file changed, 28 insertions(+), 31 deletions(-)
> > > > > > >
> > > > > > > diff --git a/include/exec/memory.h b/include/exec/memory.h
> > > > > > > index 9458e2801d50..ca247343f433 100644
> > > > > > > --- a/include/exec/memory.h
> > > > > > > +++ b/include/exec/memory.h
> > > > > > > @@ -1210,7 +1210,7 @@ void
> > > > > > > memory_region_section_free_copy(MemoryRegionSection *s);
> > > > > > >* memory_region_add_subregion() to add subregions.
> > > > > > >*
> > > > > > >* @mr: the #MemoryRegion to be initialized
> > > > > > > - * @owner: the object that tracks the region's reference count
> > > > > > > + * @owner: the object that keeps the region alive
> > > > > > >* @name: used for debugging; not visible to the user or ABI
> > > > > > >* @size: size of the region; any subregions beyond this size
> > > > > > > will be clipped
> > > > > > >*/
> > > > > > > @@ -1220,29 +1220,26 @@ void memory_region_init(MemoryRegion *mr,
> > > > > > > uint64_t size);
> > > > > > >
> > > > > > > /**
> > > > > > > - * memory_region_ref: Add 1 to a memory region's reference count
> > > > > > > + * memory_region_ref: Add a reference to the owner of a memory
> > > > > > > region
> > > > > > >*
> > > > > > > - * Whenever memory regions are accessed outside the BQL, they
> > > > > > > need to be
> > > > > > > - * preserved against hot-unplug. MemoryRegions actually do not
> > > > > > > have their
> > > > > > > - * own reference count; they piggyback on a QOM object, their
> > > > > > > "owner".
> > > > > > > - * This function adds a reference to the owner.
> > > > > > > - *
> > > > > > > - * All MemoryRegions must have an owner if they can disappear,
> > > > > > > even if the
> > > > > > > - * device they belong to operates exclusively under the BQL.
> > > > > > > This is because
> > > > > > > - * the region could be returned at any time by
> > > > > > > memory_region_find, and this
> > > > > > > - * is usually under guest control.
> > > > > > > + * This func
