[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 1/3] hw/virtio: Comment virtqueue_flush() must be called w
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [PATCH v3 1/3] hw/virtio: Comment virtqueue_flush() must be called with RCU read lock |
Date: |
Mon, 27 Sep 2021 13:21:53 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.1.0 |
On 9/27/21 12:18, Cornelia Huck wrote:
> On Mon, Sep 06 2021, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
>> Reported-by: Stefano Garzarella <sgarzare@redhat.com>
>> Suggested-by: Stefan Hajnoczi <stefanha@redhat.com>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>> include/hw/virtio/virtio.h | 7 +++++++
>> hw/virtio/virtio.c | 1 +
>> 2 files changed, 8 insertions(+)
>>
>> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
>> index 8bab9cfb750..c1c5f6e53c8 100644
>> --- a/include/hw/virtio/virtio.h
>> +++ b/include/hw/virtio/virtio.h
>> @@ -186,6 +186,13 @@ void virtio_delete_queue(VirtQueue *vq);
>>
>> void virtqueue_push(VirtQueue *vq, const VirtQueueElement *elem,
>> unsigned int len);
>> +/**
>> + * virtqueue_flush:
>> + * @vq: The #VirtQueue
>> + * @count: Number of elements to flush
>> + *
>> + * Must be called within RCU critical section.
>> + */
>
> Hm... do these doc comments belong into .h files, or rather into .c files?
Maybe we should restart this old thread, vote(?) and settle on a style?
https://lore.kernel.org/qemu-devel/349cd87b-0526-30b8-d9cd-0eee537ab5a4@redhat.com/
>> void virtqueue_flush(VirtQueue *vq, unsigned int count);
>> void virtqueue_detach_element(VirtQueue *vq, const VirtQueueElement *elem,
>> unsigned int len);
>> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
>> index 3a1f6c520cb..97f60017466 100644
>> --- a/hw/virtio/virtio.c
>> +++ b/hw/virtio/virtio.c
>> @@ -896,6 +896,7 @@ static void virtqueue_packed_flush(VirtQueue *vq,
>> unsigned int count)
>> }
>> }
>>
>> +/* Called within rcu_read_lock(). */
>> void virtqueue_flush(VirtQueue *vq, unsigned int count)
>> {
>> if (virtio_device_disabled(vq->vdev)) {
>
> The content of the change looks good to me, I'm only wondering about
> the style...
>