[RFC v1 1/2] vhost: eliminate duplicate dirty_bitmap sync when log shared by multiple devices

Chuang Xu posted 2 patches 6 days, 12 hours ago
Maintainers: Richard Henderson <richard.henderson@linaro.org>, Paolo Bonzini <pbonzini@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, Stefano Garzarella <sgarzare@redhat.com>, Peter Xu <peterx@redhat.com>, David Hildenbrand <david@kernel.org>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Fabiano Rosas <farosas@suse.de>
[RFC v1 1/2] vhost: eliminate duplicate dirty_bitmap sync when log shared by multiple devices
Posted by Chuang Xu 6 days, 12 hours ago
From: xuchuangxclwt <xuchuangxclwt@bytedance.com>

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.

Attempt to eliminate these duplicate sync.

Signed-off-by: Chuang Xu <xuchuangxclwt@bytedance.com>
---
 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.
+         */
+        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);
+        }
+    } 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
Re: [RFC v1 1/2] vhost: eliminate duplicate dirty_bitmap sync when log shared by multiple devices
Posted by Peter Xu 5 days, 3 hours ago
On Mon, Dec 08, 2025 at 08:09:51PM +0800, Chuang Xu wrote:
> From: xuchuangxclwt <xuchuangxclwt@bytedance.com>
> 
> 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.
> 
> Attempt to eliminate these duplicate sync.
> 
> Signed-off-by: Chuang Xu <xuchuangxclwt@bytedance.com>

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,

> ---
>  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.
> +         */
> +        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);
> +        }
> +    } 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
Re: [RFC v1 1/2] vhost: eliminate duplicate dirty_bitmap sync when log shared by multiple devices
Posted by Jason Wang 4 days, 17 hours ago
On Wed, Dec 10, 2025 at 4:47 AM Peter Xu <peterx@redhat.com> wrote:
>
> On Mon, Dec 08, 2025 at 08:09:51PM +0800, Chuang Xu wrote:
> > From: xuchuangxclwt <xuchuangxclwt@bytedance.com>
> >
> > 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 <xuchuangxclwt@bytedance.com>
>
> 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
Re: [RFC v1 1/2] vhost: eliminate duplicate dirty_bitmap sync when log shared by multiple devices
Posted by Chuang Xu 4 days, 10 hours ago
Hi, Jason

On 10/12/2025 14:51, Jason Wang wrote:
>>> ---
>>>   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?

You're right, in my environment, I used version 8.1 for daily testing. 
When I checked

the latest version in the community, I didn't notice that optimization 
and naturally

assumed that the latest version of vhost_log_sync also had the same issue.


I apologize for sending a duplicate optimization for vhost_log_sync.

Hope the formula derivation in the cover will provide some inspiration 
for future optimization work.


Thank you for the reminder.