On Wed, Dec 10, 2025 at 4:47 AM Peter Xu <[email protected]> wrote: > > On Mon, Dec 08, 2025 at 08:09:51PM +0800, Chuang Xu wrote: > > From: xuchuangxclwt <[email protected]> > > > > Although logs can now be shared among multiple vhost devices, > > live migration still performs repeated vhost_log_sync for each > > vhost device during bitmap_sync, which increases the time required > > for bitmap_sync and makes it more difficult for dirty pages to converge.
Please show us how you do the benchmark. > > > > Attempt to eliminate these duplicate sync. I think this is suspicious, more below. > > > > Signed-off-by: Chuang Xu <[email protected]> > > It looks reasonable from migration POV, but I don't know the details. > > Please remember to copy Jason (I added for this email) when repost. Thanks for copying me Peter. > > Thanks, > > > --- > > hw/virtio/vhost.c | 30 ++++++++++++++++++++++-------- > > include/hw/virtio/vhost.h | 1 + > > 2 files changed, 23 insertions(+), 8 deletions(-) > > > > diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c > > index 266a11514a..d397ca327f 100644 > > --- a/hw/virtio/vhost.c > > +++ b/hw/virtio/vhost.c > > @@ -268,14 +268,6 @@ static int vhost_sync_dirty_bitmap(struct vhost_dev > > *dev, > > return 0; > > } > > > > -static void vhost_log_sync(MemoryListener *listener, > > - MemoryRegionSection *section) > > -{ > > - struct vhost_dev *dev = container_of(listener, struct vhost_dev, > > - memory_listener); > > - vhost_sync_dirty_bitmap(dev, section, 0x0, ~0x0ULL); > > -} > > - > > static void vhost_log_sync_range(struct vhost_dev *dev, > > hwaddr first, hwaddr last) > > { > > @@ -287,6 +279,27 @@ static void vhost_log_sync_range(struct vhost_dev *dev, > > } > > } > > > > +static void vhost_log_sync(MemoryListener *listener, > > + MemoryRegionSection *section) > > +{ > > + struct vhost_dev *dev = container_of(listener, struct vhost_dev, > > + memory_listener); > > + struct vhost_log *log = dev->log; > > + > > + if (log && log->refcnt > 1) { > > + /* > > + * When multiple devices use same log, we implement the logic of > > + * vhost_log_sync just like what we do in vhost_log_put. > > + */ We should have already avoided the duplicated syncing with vhost_dev_should_log()? Or anything I miss here? > > + log->sync_cnt = (log->sync_cnt + 1) % log->refcnt; > > + if (!log->sync_cnt) { > > + vhost_log_sync_range(dev, 0, dev->log_size * VHOST_LOG_CHUNK - > > 1); This will lose some syncs when the device is not the logger device (see vhost_dev_should_log())? > > + } > > + } else { > > + vhost_sync_dirty_bitmap(dev, section, 0x0, ~0x0ULL); > > + } > > +} > > + > > static uint64_t vhost_get_log_size(struct vhost_dev *dev) > > { > > uint64_t log_size = 0; > > @@ -383,6 +396,7 @@ static struct vhost_log *vhost_log_get(VhostBackendType > > backend_type, > > ++log->refcnt; > > } > > > > + log->sync_cnt = 0; > > return log; > > } > > > > diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h > > index 08bbb4dfe9..43bf1c2150 100644 > > --- a/include/hw/virtio/vhost.h > > +++ b/include/hw/virtio/vhost.h > > @@ -50,6 +50,7 @@ typedef unsigned long vhost_log_chunk_t; > > struct vhost_log { > > unsigned long long size; > > int refcnt; > > + int sync_cnt; > > int fd; > > vhost_log_chunk_t *log; > > }; > > -- > > 2.20.1 > > > > -- > Peter Xu > Thanks
