On Fri, Oct 27, 2023 at 11:05 AM Eelco Chaudron <echau...@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
