Re: [PATCH v3 1/3] hw/virtio: Comment virtqueue_flush() must be called with RCU read lock

[Cornelia Huck]

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?

>  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...