> -----Original Message-----
> From: Liu, Changpeng <changpeng....@intel.com>
> Sent: Tuesday, September 20, 2022 10:34 AM
> To: Xia, Chenbo <chenbo....@intel.com>; dev@dpdk.org
> Cc: Maxime Coquelin <maxime.coque...@redhat.com>
> Subject: RE: [PATCH] vhost: use try_lock in rte_vhost_vring_call
>
> Hi Bo,
>
> > -----Original Message-----
> > From: Xia, Chenbo <chenbo....@intel.com>
> > Sent: Tuesday, September 20, 2022 10:25 AM
> > To: Liu, Changpeng <changpeng....@intel.com>; dev@dpdk.org
> > Cc: Maxime Coquelin <maxime.coque...@redhat.com>
> > Subject: RE: [PATCH] vhost: use try_lock in rte_vhost_vring_call
> >
> > Hi Changpeng,
> >
> > > -----Original Message-----
> > > From: Liu, Changpeng <changpeng....@intel.com>
> > > Sent: Tuesday, September 6, 2022 10:22 AM
> > > To: dev@dpdk.org
> > > Cc: Liu, Changpeng <changpeng....@intel.com>; Maxime Coquelin
> > > <maxime.coque...@redhat.com>; Xia, Chenbo <chenbo....@intel.com>
> > > Subject: [PATCH] vhost: use try_lock in rte_vhost_vring_call
> > >
> > > Note that this function is in data path, so the thread context
> > > may not same as socket messages processing context, by using
> > > try_lock here, users can have another try in case of VQ's access
> > > lock is held by `vhost-events` thread.
> >
> > Better to describe the issue this patch wants to fix and how does
> > it fix.
> >
> > I remember it's a bz issue, do you want to backport? And it has
> > some bz ID, we need to add it in commit message.
> Actually it's my intention not to add bz ID, as I think for this bz ID,
> It's better not to lock all VQ's access lock for KICK/CALLFD messages,
Do you plan to add this change? I think that may be an improvement to current
locking implementation.
Maxime, what do you think of this idea about only locking specific queue when
handling vring related message (not global config like mem table)?
> What do you think? If this is identified as a fix, I can backport it to
> 22.05.
You can decide, if this is planned to be the fix, just backport. I am just
thinking if this is not the fix for the bz, do we still need this?
Thanks,
Chenbo
> >
> > >
> > > Signed-off-by: Changpeng Liu <changpeng....@intel.com>
> > > ---
> > > lib/vhost/vhost.c | 6 +++++-
> > > 1 file changed, 5 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c
> > > index 60cb05a0ff..072d2acb7b 100644
> > > --- a/lib/vhost/vhost.c
> > > +++ b/lib/vhost/vhost.c
> > > @@ -1329,7 +1329,11 @@ rte_vhost_vring_call(int vid, uint16_t
> vring_idx)
> > > if (!vq)
> > > return -1;
> > >
> > > - rte_spinlock_lock(&vq->access_lock);
> > > + if (!rte_spinlock_trylock(&vq->access_lock)) {
> > > + VHOST_LOG_CONFIG(dev->ifname, DEBUG,
> >
> > Should use VHOST_LOG_DATA
> OK.
> >
> > Thanks,
> > Chenbo
> >
> > > + "failed to kick guest, virtqueue busy.\n");
> > > + return -1;
> > > + }
> > >
> > > if (vq_is_packed(dev))
> > > vhost_vring_call_packed(dev, vq);
> > > --
> > > 2.21.3
