[PATCH 2/3] vhost: fix virtqueue access lock in datapath

[David Marchand]

On Fri, Oct 27, 2023 at 11:05 AM Eelco Chaudron <echaudro at redhat.com> wrote:
> > diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c
> > index 759a78e3e3..4116f79d4f 100644
> > --- a/lib/vhost/virtio_net.c
> > +++ b/lib/vhost/virtio_net.c
> > @@ -1694,6 +1694,17 @@ virtio_dev_rx_packed(struct virtio_net *dev,
> >       return pkt_idx;
> >  }
> >
> > +static void
> > +virtio_dev_vring_translate(struct virtio_net *dev, struct vhost_virtqueue *vq)
> > +{
>
> Would it be an idea to annotate this function that it needs to be called with the “read locks” (and that it will free them) to avoid the duplicate:
>
> +               vhost_user_iotlb_rd_unlock(vq);
> +               rte_rwlock_read_unlock(&vq->access_lock);

The "unlock" annotations do not express read/write concerns for locks.
So that would make the code less readable and potentially hide some issues.

I prefer to keep as is, with clear calls to rd_lock / rd_unlock in
those functions.

>
> > +     rte_rwlock_write_lock(&vq->access_lock);
> > +     vhost_user_iotlb_rd_lock(vq);
> > +     if (!vq->access_ok)
> > +             vring_translate(dev, vq);
> > +     vhost_user_iotlb_rd_unlock(vq);
> > +     rte_rwlock_write_unlock(&vq->access_lock);
> > +}
> > +

-- 
David Marchand