Re: [RFC v2] memory.c: improve refcounting for RAM vs MMIO regions

[Peter Xu <peterx@redhat.com>]

On Wed, Aug 06, 2025 at 10:36:33PM +0200, David Hildenbrand wrote:
> On 06.08.25 22:15, Peter Xu wrote:
> > On Tue, Aug 05, 2025 at 10:11:23AM +0200, Albert Esteve wrote:
> > > v1->v2:
> > > - Added documentation
> > > - Explained the reasoning in the commit message
> > > 
> > > In the last version of the SHMEM MAP/UNMAP [1] Stefan
> > > raised a concern [2] about dynamically creating and
> > > destroying memory regions and their lifecycle [3].
> > > 
> > > After some discussion, David Hildenbrand proposed
> > > to detect RAM regions and handle refcounting differently.
> > > I tried to extend the reasoning in the commit message
> > > below. If I wrote any innacuracies, please keep me
> > > honest. I hope we can gather some feedback with
> > > this RFC patch before sending it for inclusion.
> > 
> > This seems working.  Looks like so far all RAM MRs are fine with it, but
> > I'm not strongly confident it's true or it'll trivially keep true in the
> > future too.
> > 
> > Besides, this still adds some trivial complexity to memory_region_ref() on
> > treating RAM/MMIO MRs differently.
> > > It also sounds like a pure "accident" that the shmem objects to be
> mapped
> > from the vhost-user devices are RAMs.  I wonder what happens if we want to
> > also support dynmaic MMIO regions.
> 
> Is this use case realistic?

Nop. :) It's a sincere wish that if such a feature to be introduced, it
could work for MMIOs too.  Or better if no need to introduce it.

> 
> If there is a reasonable way to prepare for such hypothetical use cases them
> while solving Albert's immediate use case, I'm all for it.
> 
> > 
> > Would this work even without changing QEMU memory core?
> > 
> > For example, have you thought about creating a VhostUserShmemObject for
> > each of the VHOST_USER_BACKEND_SHMEM_MAP request?
> 
> You mean, adding an intermediate object that remains the parent of these
> MemoryRegion?
> 
> Could work. To free a MemoryRegion, I guess we would unparent that
> intermediate object, and that object would then free the memory region --
> unless something still references that intermediate object. Not sure if the
> memory region might keep the intermediate object still alive (no idea).

It should, as long as memory_region_ref() will boost the tempobj's refcount
properly.

Thanks,

> 
> Certainly something to explore, Albert, can you look into that?
> 
> -- 
> Cheers,
> 
> David / dhildenb
> 

-- 
Peter Xu