On Fri, Mar 15, 2024 at 5:39 AM Si-Wei Liu <si-wei....@oracle.com> wrote: > > On setups with one or more virtio-net devices with vhost on, > dirty tracking iteration increases cost the bigger the number > amount of queues are set up e.g. on idle guests migration the > following is observed with virtio-net with vhost=on: > > 48 queues -> 78.11% [.] vhost_dev_sync_region.isra.13 > 8 queues -> 40.50% [.] vhost_dev_sync_region.isra.13 > 1 queue -> 6.89% [.] vhost_dev_sync_region.isra.13 > 2 devices, 1 queue -> 18.60% [.] vhost_dev_sync_region.isra.14 > > With high memory rates the symptom is lack of convergence as soon > as it has a vhost device with a sufficiently high number of queues, > the sufficient number of vhost devices. > > On every migration iteration (every 100msecs) it will redundantly > query the *shared log* the number of queues configured with vhost > that exist in the guest. For the virtqueue data, this is necessary, > but not for the memory sections which are the same. So essentially > we end up scanning the dirty log too often. > > To fix that, select a vhost device responsible for scanning the > log with regards to memory sections dirty tracking. It is selected > when we enable the logger (during migration) and cleared when we > disable the logger. If the vhost logger device goes away for some > reason, the logger will be re-selected from the rest of vhost > devices. > > After making mem-section logger a singleton instance, constant cost > of 7%-9% (like the 1 queue report) will be seen, no matter how many > queues or how many vhost devices are configured: > > 48 queues -> 8.71% [.] vhost_dev_sync_region.isra.13 > 2 devices, 8 queues -> 7.97% [.] vhost_dev_sync_region.isra.14 > > Co-developed-by: Joao Martins <joao.m.mart...@oracle.com> > Signed-off-by: Joao Martins <joao.m.mart...@oracle.com> > Signed-off-by: Si-Wei Liu <si-wei....@oracle.com> > > --- > v3 -> v4: > - add comment to clarify effect on cache locality and > performance > > v2 -> v3: > - add after-fix benchmark to commit log > - rename vhost_log_dev_enabled to vhost_dev_should_log > - remove unneeded comparisons for backend_type > - use QLIST array instead of single flat list to store vhost > logger devices > - simplify logger election logic > --- > hw/virtio/vhost.c | 67 > ++++++++++++++++++++++++++++++++++++++++++----- > include/hw/virtio/vhost.h | 1 + > 2 files changed, 62 insertions(+), 6 deletions(-) > > diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c > index 612f4db..58522f1 100644 > --- a/hw/virtio/vhost.c > +++ b/hw/virtio/vhost.c > @@ -45,6 +45,7 @@ > > static struct vhost_log *vhost_log[VHOST_BACKEND_TYPE_MAX]; > static struct vhost_log *vhost_log_shm[VHOST_BACKEND_TYPE_MAX]; > +static QLIST_HEAD(, vhost_dev) vhost_log_devs[VHOST_BACKEND_TYPE_MAX]; > > /* Memslots used by backends that support private memslots (without an fd). > */ > static unsigned int used_memslots; > @@ -149,6 +150,47 @@ bool vhost_dev_has_iommu(struct vhost_dev *dev) > } > } > > +static inline bool vhost_dev_should_log(struct vhost_dev *dev) > +{ > + assert(dev->vhost_ops); > + assert(dev->vhost_ops->backend_type > VHOST_BACKEND_TYPE_NONE); > + assert(dev->vhost_ops->backend_type < VHOST_BACKEND_TYPE_MAX); > + > + return dev == QLIST_FIRST(&vhost_log_devs[dev->vhost_ops->backend_type]);
A dumb question, why not simple check dev->log == vhost_log_shm[dev->vhost_ops->backend_type] ? Thanks