Re: [PATCH v1 1/2] vhost: Defer filtering memory sections until building the vhost memory structure

[Michael S. Tsirkin]

On Thu, Feb 16, 2023 at 01:10:54PM +0100, David Hildenbrand wrote:
> On 16.02.23 13:04, Michael S. Tsirkin wrote:
> > On Thu, Feb 16, 2023 at 12:47:51PM +0100, David Hildenbrand wrote:
> > > Having multiple devices, some filtering memslots and some not filtering
> > > memslots, messes up the "used_memslot" accounting. If we'd have a device
> > > the filters out less memory sections after a device that filters out more,
> > > we'd be in trouble, because our memslot checks stop working reliably.
> > > For example, hotplugging a device that filters out less memslots might end
> > > up passing the checks based on max vs. used memslots, but can run out of
> > > memslots when getting notified about all memory sections.
> > > 
> > > Further, it will be helpful in memory device context in the near future
> > > to know that a RAM memory region section will consume a memslot, and be
> > > accounted for in the used vs. free memslots, such that we can implement
> > > reservation of memslots for memory devices properly. Whether a device
> > > filters this out and would theoretically still have a free memslot is
> > > then hidden internally, making overall vhost memslot accounting easier.
> > > 
> > > Let's filter the memslots when creating the vhost memory array,
> > > accounting all RAM && !ROM memory regions as "used_memslots" even if
> > > vhost_user isn't interested in anonymous RAM regions, because it needs
> > > an fd.
> > > 
> > > When a device actually filters out regions (which should happen rarely
> > > in practice), we might detect a layout change although only filtered
> > > regions changed. We won't bother about optimizing that for now.
> > 
> > That caused trouble in the past when using VGA because it is playing
> > with mappings in weird ways.
> > I think we have to optimize it, sorry.
> 
> We still filter them out, just later.


The issue is sending lots of unnecessary system calls to update the kernel which
goes through a slow RCU.

> > > Note: we cannot simply filter out the region and count them as
> > > "filtered" to add them to used, because filtered regions could get
> > > merged and result in a smaller effective number of memslots. Further,
> > > we won't touch the hmp/qmp virtio introspection output.
> > > 
> > > Fixes: 988a27754bbb ("vhost: allow backends to filter memory sections")
> > > Cc: Tiwei Bie <tiwei.bie@intel.com>
> > > Signed-off-by: David Hildenbrand <david@redhat.com>
> > 
> > I didn't review this yet but maybe you can answer:
> > will this create more slots for the backend?
> > Because some backends are limited in # of slots and breaking them is
> > not a good idea.
> 
> It restores the handling we had before 988a27754bbb. RAM without an fd
> should be rare for vhost-user setups (where we actually filter) I assume?

Hmm, I guess so.

> -- 
> Thanks,
> 
> David / dhildenb