On 27 Oct 2023, at 11:22, David Marchand wrote:
> 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.
ACK, keeping this as is fine by me.
Acked-by: Eelco Chaudron <echau...@redhat.com>
>>> + 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
