Re: [dpdk-dev] [PATCH] vhost: fix wait for valid descriptor
On Thu, Jul 26, 2018 at 05:48:04PM +0200, Thomas Monjalon wrote:
> What is the status of this patch?
>
>
> 11/02/2018 05:21, Tan, Jianfeng:
> >
> > > -Original Message-
> > > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Tomasz Kulasek
> > > Sent: Saturday, February 10, 2018 1:35 AM
> > > To: y...@fridaylinux.org
> > > Cc: dev@dpdk.org; yuanhan@linux.intel.com; sta...@dpdk.org; Liu,
> > > Changpeng
> > > Subject: [dpdk-dev] [PATCH] vhost: fix wait for valid descriptor
> > >
> > > For each virt queue's kickfd and callfd, there are 2 invalid
> > > status: VIRTIO_UNINITIALIZED_EVENTFD and VIRTIO_INVALID_EVENTFD.
> > > Don't set the virt queue to ready status until got the valid
> > > descriptor.
> >
> > Cannot remember why we introduce two invalid status.
> >
> > If we treat "VIRTIO_INVALID_EVENTFD" as an indicator to judge if device is
> > ready, then defining one status (invalid) is enough.
> >
> > Thanks,
> > Jianfeng
> >
> > >
> > > This is safe for polling mode drivers in Guest OS, the backend
> > > vhost process will not post notification to interrupt vector for
> > > PMD mode in guest, but the interrupt vector still valid.
> > >
> > > Fixes: e049ca6d10e0 ("vhost-user: prepare multiple queue setup")
This fixes line isn't right. It's the below commit [1]
that introduced the code changed by this patch. The code
was written in this way intentionally by that commit.
With a quick glance at that commit, it seems that this
patch isn't valid.
[1]
https://git.dpdk.org/dpdk/commit/?id=fb871d0a4dc1c038a381c524cdb86fe83d21d842
commit fb871d0a4dc1c038a381c524cdb86fe83d21d842
Author: Tetsuya Mukawa
Date: Mon Mar 14 17:53:32 2016 +0900
vhost: fix default value of kickfd and callfd
Currently, default values of kickfd and callfd are -1.
If the values are -1, current code guesses kickfd and callfd haven't
been initialized yet. Then vhost library will guess the virtqueue isn't
ready for processing.
But callfd and kickfd will be set as -1 when "--enable-kvm"
isn't specified in QEMU command line. It means we cannot treat -1 as
uninitialized state.
The patch defines -1 and -2 as VIRTIO_INVALID_EVENTFD and
VIRTIO_UNINITIALIZED_EVENTFD, and uses VIRTIO_UNINITIALIZED_EVENTFD for
the default values of kickfd and callfd.
Signed-off-by: Tetsuya Mukawa
Acked-by: Yuanhan Liu
> > > Cc: yuanhan@linux.intel.com
> > > Cc: sta...@dpdk.org
> > >
> > > Signed-off-by: Changpeng Liu
> > > Signed-off-by: Tomasz Kulasek
> > > ---
> > > lib/librte_vhost/vhost_user.c | 4 +++-
> > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
> > > index 65ee33919..4508f697b 100644
> > > --- a/lib/librte_vhost/vhost_user.c
> > > +++ b/lib/librte_vhost/vhost_user.c
> > > @@ -766,7 +766,9 @@ vq_is_ready(struct vhost_virtqueue *vq)
> > > {
> > > return vq && vq->desc && vq->avail && vq->used &&
> > > vq->kickfd != VIRTIO_UNINITIALIZED_EVENTFD &&
> > > -vq->callfd != VIRTIO_UNINITIALIZED_EVENTFD;
> > > +vq->callfd != VIRTIO_UNINITIALIZED_EVENTFD &&
> > > +vq->kickfd != VIRTIO_INVALID_EVENTFD &&
> > > +vq->callfd != VIRTIO_INVALID_EVENTFD;
> > > }
> > >
> > > static int
> > > --
> > > 2.14.1
> >
>
Re: [dpdk-dev] [PATCH] vhost: fix wait for valid descriptor
What is the status of this patch?
11/02/2018 05:21, Tan, Jianfeng:
>
> > -Original Message-
> > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Tomasz Kulasek
> > Sent: Saturday, February 10, 2018 1:35 AM
> > To: y...@fridaylinux.org
> > Cc: dev@dpdk.org; yuanhan@linux.intel.com; sta...@dpdk.org; Liu,
> > Changpeng
> > Subject: [dpdk-dev] [PATCH] vhost: fix wait for valid descriptor
> >
> > For each virt queue's kickfd and callfd, there are 2 invalid
> > status: VIRTIO_UNINITIALIZED_EVENTFD and VIRTIO_INVALID_EVENTFD.
> > Don't set the virt queue to ready status until got the valid
> > descriptor.
>
> Cannot remember why we introduce two invalid status.
>
> If we treat "VIRTIO_INVALID_EVENTFD" as an indicator to judge if device is
> ready, then defining one status (invalid) is enough.
>
> Thanks,
> Jianfeng
>
> >
> > This is safe for polling mode drivers in Guest OS, the backend
> > vhost process will not post notification to interrupt vector for
> > PMD mode in guest, but the interrupt vector still valid.
> >
> > Fixes: e049ca6d10e0 ("vhost-user: prepare multiple queue setup")
> > Cc: yuanhan@linux.intel.com
> > Cc: sta...@dpdk.org
> >
> > Signed-off-by: Changpeng Liu
> > Signed-off-by: Tomasz Kulasek
> > ---
> > lib/librte_vhost/vhost_user.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
> > index 65ee33919..4508f697b 100644
> > --- a/lib/librte_vhost/vhost_user.c
> > +++ b/lib/librte_vhost/vhost_user.c
> > @@ -766,7 +766,9 @@ vq_is_ready(struct vhost_virtqueue *vq)
> > {
> > return vq && vq->desc && vq->avail && vq->used &&
> >vq->kickfd != VIRTIO_UNINITIALIZED_EVENTFD &&
> > - vq->callfd != VIRTIO_UNINITIALIZED_EVENTFD;
> > + vq->callfd != VIRTIO_UNINITIALIZED_EVENTFD &&
> > + vq->kickfd != VIRTIO_INVALID_EVENTFD &&
> > + vq->callfd != VIRTIO_INVALID_EVENTFD;
> > }
> >
> > static int
> > --
> > 2.14.1
>
Re: [dpdk-dev] [PATCH] vhost: fix wait for valid descriptor
> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Tomasz Kulasek
> Sent: Saturday, February 10, 2018 1:35 AM
> To: y...@fridaylinux.org
> Cc: dev@dpdk.org; yuanhan@linux.intel.com; sta...@dpdk.org; Liu,
> Changpeng
> Subject: [dpdk-dev] [PATCH] vhost: fix wait for valid descriptor
>
> For each virt queue's kickfd and callfd, there are 2 invalid
> status: VIRTIO_UNINITIALIZED_EVENTFD and VIRTIO_INVALID_EVENTFD.
> Don't set the virt queue to ready status until got the valid
> descriptor.
Cannot remember why we introduce two invalid status.
If we treat "VIRTIO_INVALID_EVENTFD" as an indicator to judge if device is
ready, then defining one status (invalid) is enough.
Thanks,
Jianfeng
>
> This is safe for polling mode drivers in Guest OS, the backend
> vhost process will not post notification to interrupt vector for
> PMD mode in guest, but the interrupt vector still valid.
>
> Fixes: e049ca6d10e0 ("vhost-user: prepare multiple queue setup")
> Cc: yuanhan@linux.intel.com
> Cc: sta...@dpdk.org
>
> Signed-off-by: Changpeng Liu
> Signed-off-by: Tomasz Kulasek
> ---
> lib/librte_vhost/vhost_user.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
> index 65ee33919..4508f697b 100644
> --- a/lib/librte_vhost/vhost_user.c
> +++ b/lib/librte_vhost/vhost_user.c
> @@ -766,7 +766,9 @@ vq_is_ready(struct vhost_virtqueue *vq)
> {
> return vq && vq->desc && vq->avail && vq->used &&
> vq->kickfd != VIRTIO_UNINITIALIZED_EVENTFD &&
> -vq->callfd != VIRTIO_UNINITIALIZED_EVENTFD;
> +vq->callfd != VIRTIO_UNINITIALIZED_EVENTFD &&
> +vq->kickfd != VIRTIO_INVALID_EVENTFD &&
> +vq->callfd != VIRTIO_INVALID_EVENTFD;
> }
>
> static int
> --
> 2.14.1
