Re: [dpdk-dev] [PATCH] vhost: fix wait for valid descriptor

[Tiwei Bie <tiwei.bie@intel.com>]

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-bounces@dpdk.org] On Behalf Of Tomasz Kulasek
> > > Sent: Saturday, February 10, 2018 1:35 AM
> > > To: yliu@fridaylinux.org
> > > Cc: dev@dpdk.org; yuanhan.liu@linux.intel.com; stable@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 <mukawa@igel.co.jp>
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 <mukawa@igel.co.jp>
    Acked-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>



> > > Cc: yuanhan.liu@linux.intel.com
> > > Cc: stable@dpdk.org
> > > 
> > > Signed-off-by: Changpeng Liu <changpeng.liu@intel.com>
> > > Signed-off-by: Tomasz Kulasek <tomaszx.kulasek@intel.com>
> > > ---
> > >  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
> > 
>