qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH V2] vhost: logs sharing


From: Jason Wang
Subject: Re: [Qemu-devel] [PATCH V2] vhost: logs sharing
Date: Thu, 30 Apr 2015 17:22:33 +0800



On Thu, Apr 30, 2015 at 4:09 PM, Michael S. Tsirkin <address@hidden> wrote:
On Thu, Apr 30, 2015 at 04:05:09PM +0800, Jason Wang wrote:
On Tue, Apr 28, 2015 at 6:30 PM, Michael S. Tsirkin <address@hidden> wrote:
 >On Tue, Apr 28, 2015 at 05:58:28PM +0800, Jason Wang wrote:
 >>     On Tue, Apr 28, 2015 at 5:37 PM, Michael S. Tsirkin
 >><address@hidden> wrote:
 >> >On Fri, Apr 10, 2015 at 05:33:35PM +0800, Jason Wang wrote:
>> >> Currently we allocate one vhost log per vhost device. This is sub
 >> >> optimal when:
 >> >> - Guest has several device with vhost as backend
 >> >> - Guest has multiqueue devices
>> >> In the above cases, we can avoid the memory allocation by sharing a >> >> single vhost log among all the vhost devices. This is done through:
 >> >> - Introducing a new vhost_log structure with refcnt inside.
>> >> - Using a global pointer to vhost_log structure that will be used.
 >>And
>> >> introduce helper to get the log with expected log size and helper
 >>to
 >> >> - drop the refcnt to the old log.
>> >> - Each vhost device still keep track of a pointer to the log that
 >>was
 >> >>   used.
>> >> With above, if no resize happens, all vhost device will share a
 >> >>single
 >> >> vhost log. During resize, a new vhost_log structure will be
 >>allocated
>> >> and made for the global pointer. And each vhost devices will drop
 >>the
 >> >> refcnt to the old log.
>> >> Tested by doing scp during migration for a 2 queues virtio-net-pci.
 >> >> Cc: Michael S. Tsirkin <address@hidden>
 >> >> Signed-off-by: Jason Wang <address@hidden>
 >> >> ---
 >> >> Changes from V1:
 >> >> - Drop the list of vhost log, instead, using a global pointer
 >>instead
 >> >
 >> >I don't think it works like this. If you have a global pointer,
 >> >you also need a global listener, have that sync all devices.
 >> It doesn't conflict, see my comments below.
 >> >
 >> >
 >> >
 >> >> ---
 >> >>  hw/virtio/vhost.c         | 66
 >> >>++++++++++++++++++++++++++++++++++++++---------
 >> >>  include/hw/virtio/vhost.h |  9 ++++++-
 >> >>  2 files changed, 62 insertions(+), 13 deletions(-)
 >> >> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
 >> >> index 5a12861..e16c2db 100644
 >> >> --- a/hw/virtio/vhost.c
 >> >> +++ b/hw/virtio/vhost.c
 >> >> @@ -22,15 +22,19 @@
 >> >>  #include "hw/virtio/virtio-bus.h"
 >> >>  #include "migration/migration.h"
 >> >> +static struct vhost_log *vhost_log;
 >> >> +
 >> >>  static void vhost_dev_sync_region(struct vhost_dev *dev,
>> >> MemoryRegionSection *section, >> >> uint64_t mfirst, uint64_t mlast, >> >> uint64_t rfirst, uint64_t rlast)
 >> >>  {
 >> >> +    vhost_log_chunk_t *log = dev->log->log;
 >> >> +
 >> >>      uint64_t start = MAX(mfirst, rfirst);
 >> >>      uint64_t end = MIN(mlast, rlast);
>> >> - vhost_log_chunk_t *from = dev->log + start / VHOST_LOG_CHUNK; >> >> - vhost_log_chunk_t *to = dev->log + end / VHOST_LOG_CHUNK + 1;
 >> >> +    vhost_log_chunk_t *from = log + start / VHOST_LOG_CHUNK;
 >> >> +    vhost_log_chunk_t *to = log + end / VHOST_LOG_CHUNK + 1;
>> >> uint64_t addr = (start / VHOST_LOG_CHUNK) * VHOST_LOG_CHUNK;
 >> >>      if (end < start) {
>> >> @@ -280,22 +284,55 @@ static uint64_t vhost_get_log_size(struct
 >> >>vhost_dev *dev)
 >> >>      }
 >> >>      return log_size;
 >> >>  }
 >> >> +static struct vhost_log *vhost_log_alloc(uint64_t size)
 >> >> +{
 >> >> +    struct vhost_log *log = g_malloc0(sizeof *log + size *
 >> >>sizeof(*(log->log)));
 >> >> +
 >> >> +    log->size = size;
 >> >> +    log->refcnt = 1;
 >> >> +
 >> >> +    return log;
 >> >> +}
 >> >> +
 >> >> +static struct vhost_log *vhost_log_get(uint64_t size)
 >> >> +{
 >> >> +    if (!vhost_log || vhost_log->size != size) {
 >> >> +        vhost_log = vhost_log_alloc(size);
 >> >
 >> >This just leaks the old log if size != size.
 >>   But old log is reference counted and will be freed during
 >>vhost_log_put() if
 >> refcnt drops to zero.
 >
 >You need a pointer to reference-count it.
 >You return pointer to new object, no one references the old one.
The pointer is just vhost_log->log. The old pointer will be kept in vhost_dev_log_resize() until 1) new log was set through ioctl and 2) the old
 log was synced.

vhost_dev_log_resize is per device, isn't it?

Yes.


So the log is synced in device 1, but not in device 2.

But we will do resizing one by one for all listeners. So the sync of device 2 will happen soon afterwards.




reply via email to

[Prev in Thread] Current Thread [Next in Thread]