On Tue, 13 May 2025 15:12:11 +0200
David Hildenbrand <da...@redhat.com> wrote:
> On 13.05.25 14:13, Igor Mammedov wrote:
> > On Mon, 3 Mar 2025 13:02:17 -0500
> > yuanminghao <yuanm...@chinatelecom.cn> wrote:
> >
> >>>> Global used_memslots or used_shared_memslots is updated to 0 unexpectly
> >>>
> >>> it shouldn't be 0 in practice, as it comes from number of RAM regions VM
> >>> has.
> >>> It's likely a bug somewhere else.
> >
> > I haven't touched this code for a long time, but I'd say if we consider
> > multiple
> > devices, we shouldn't do following:
> >
> > static void vhost_commit(MemoryListener *listener)
> > ...
> > if (dev->vhost_ops->vhost_backend_no_private_memslots &&
> > dev->vhost_ops->vhost_backend_no_private_memslots(dev)) {
> > used_shared_memslots = dev->mem->nregions;
> > } else {
> > used_memslots = dev->mem->nregions;
> > }
> >
> > where value dev->mem->nregions gets is well hidden/obscured
> > and hard to trace where tail ends => fragile.
> >
> > CCing David (accidental victim) who rewrote this part the last time,
> > perhaps he can suggest a better way to fix the issue.
>
> I think the original idea is that all devices (of on type: private vs.
> non-private memslots) have the same number of memslots.
>
> This avoids having to loop over all devices to figure out the number of
> memslots.
>
> ... but in vhost_get_free_memslots() we already loop over all devices.
>
> The check in vhost_dev_init() needs to be taken care of.
>
> So maybe we can get rid of both variables completely?
looks reasonable to me, (instead of current state which is
juggling with dev->mem->nregions that can become 0 on unplug
as it was reported).
David,
do you have time to fix it?
