From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55432) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZK1ql-0005iO-Bc for qemu-devel@nongnu.org; Tue, 28 Jul 2015 06:11:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZK1qh-0003k5-5n for qemu-devel@nongnu.org; Tue, 28 Jul 2015 06:11:07 -0400 Received: from mx1.redhat.com ([209.132.183.28]:41027) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZK1qg-0003jy-UW for qemu-devel@nongnu.org; Tue, 28 Jul 2015 06:11:03 -0400 Date: Tue, 28 Jul 2015 13:10:59 +0300 From: "Michael S. Tsirkin" Message-ID: <20150728130939-mutt-send-email-mst@redhat.com> References: <1437615403-13554-1-git-send-email-marcandre.lureau@redhat.com> <1437615403-13554-5-git-send-email-marcandre.lureau@redhat.com> <55B712E5.8080306@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <55B712E5.8080306@redhat.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH RFC 4/6] vhost: alloc shareable log List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jason Wang Cc: haifeng.lin@huawei.com, =?iso-8859-1?Q?Marc-Andr=E9?= Lureau , pbonzini@redhat.com, qemu-devel@nongnu.org, thibaut.collet@6wind.com On Tue, Jul 28, 2015 at 01:28:05PM +0800, Jason Wang wrote: >=20 >=20 > On 07/23/2015 09:36 AM, Marc-Andr=E9 Lureau wrote: > > If the backend is of type VHOST_BACKEND_TYPE_USER, allocate > > shareable memory. > > > > Note: vhost_log_get() can use a global "vhost_log" that can be shared= by > > several vhost devices. We may want instead a common shareable log and= a > > common non-shareable one. > > > > Signed-off-by: Marc-Andr=E9 Lureau > > --- > > hw/virtio/vhost.c | 42 +++++++++++++++++++++++++++++++++----= ----- > > include/hw/virtio/vhost.h | 3 ++- > > 2 files changed, 35 insertions(+), 10 deletions(-) > > > > diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c > > index 2712c6f..12dd644 100644 > > --- a/hw/virtio/vhost.c > > +++ b/hw/virtio/vhost.c > > @@ -286,20 +286,34 @@ static uint64_t vhost_get_log_size(struct vhost= _dev *dev) > > } > > return log_size; > > } > > -static struct vhost_log *vhost_log_alloc(uint64_t size) > > + > > +static struct vhost_log *vhost_log_alloc(uint64_t size, bool share) > > { > > - struct vhost_log *log =3D g_malloc0(sizeof *log + size * sizeof(= *(log->log))); > > + struct vhost_log *log; > > + uint64_t logsize =3D size * sizeof(*(log->log)); > > + int fd =3D -1; > > + > > + log =3D g_new0(struct vhost_log, 1); > > + if (share) { > > + log->log =3D qemu_memfd_alloc("vhost-log", logsize, > > + F_SEAL_GROW|F_SEAL_SHRINK|F_SEAL= _SEAL, &fd); > > + memset(log->log, 0, logsize); > > + } else { > > + log->log =3D g_malloc0(logsize); > > + } > > =20 > > log->size =3D size; > > log->refcnt =3D 1; > > + log->fd =3D fd; > > =20 > > return log; > > } > > =20 > > -static struct vhost_log *vhost_log_get(uint64_t size) > > +static struct vhost_log *vhost_log_get(uint64_t size, bool share) > > { > > - if (!vhost_log || vhost_log->size !=3D size) { > > - vhost_log =3D vhost_log_alloc(size); > > + if (!vhost_log || vhost_log->size !=3D size || > > + (share && vhost_log->fd =3D=3D -1)) { > > + vhost_log =3D vhost_log_alloc(size, share); > > } else { > > ++vhost_log->refcnt; > > } > > @@ -324,21 +338,30 @@ static void vhost_log_put(struct vhost_dev *dev= , bool sync) > > if (vhost_log =3D=3D log) { > > vhost_log =3D NULL; > > } > > + > > + if (log->fd =3D=3D -1) { > > + g_free(log->log); > > + } else { > > + qemu_memfd_free(log->log, log->size * sizeof(*(log->log)= ), > > + log->fd); > > + } > > g_free(log); > > } > > } > > =20 > > static inline void vhost_dev_log_resize(struct vhost_dev* dev, uint6= 4_t size) > > { > > - struct vhost_log *log =3D vhost_log_get(size); > > + bool share =3D dev->vhost_ops->backend_type =3D=3D VHOST_BACKEND= _TYPE_USER; > > + struct vhost_log *log =3D vhost_log_get(size, share); > > uint64_t log_base =3D (uintptr_t)log->log; > > int r; > > =20 > > - r =3D dev->vhost_ops->vhost_call(dev, VHOST_SET_LOG_BASE, &log_b= ase); > > - assert(r >=3D 0); > > vhost_log_put(dev, true); > > dev->log =3D log; > > dev->log_size =3D size; > > + > > + r =3D dev->vhost_ops->vhost_call(dev, VHOST_SET_LOG_BASE, &log_b= ase); > > + assert(r >=3D 0); > > } >=20 > Why this change is needed? I know why it's needed :) But this needs to be stated in the commit log. Also, it only makes sense if remote supports getting the logfd. > > =20 > > static int vhost_verify_ring_mappings(struct vhost_dev *dev, > > @@ -1136,9 +1159,10 @@ int vhost_dev_start(struct vhost_dev *hdev, Vi= rtIODevice *vdev) > > =20 > > if (hdev->log_enabled) { > > uint64_t log_base; > > + bool share =3D hdev->vhost_ops->backend_type =3D=3D VHOST_BA= CKEND_TYPE_USER; > > =20 > > hdev->log_size =3D vhost_get_log_size(hdev); > > - hdev->log =3D vhost_log_get(hdev->log_size); > > + hdev->log =3D vhost_log_get(hdev->log_size, share); > > log_base =3D (uintptr_t)hdev->log->log; > > r =3D hdev->vhost_ops->vhost_call(hdev, VHOST_SET_LOG_BASE, > > hdev->log_size ? &log_base := NULL); > > diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h > > index 6467c73..ab1dcac 100644 > > --- a/include/hw/virtio/vhost.h > > +++ b/include/hw/virtio/vhost.h > > @@ -31,7 +31,8 @@ typedef unsigned long vhost_log_chunk_t; > > struct vhost_log { > > unsigned long long size; > > int refcnt; > > - vhost_log_chunk_t log[0]; > > + int fd; > > + vhost_log_chunk_t *log; > > }; > > =20 > > struct vhost_memory;