[PATCH vhost v4 1/6] virtio_balloon: remove the dependence where names[] is null

[David Hildenbrand]

On 21.03.24 11:15, Xuan Zhuo wrote:
> Currently, the init_vqs function within the virtio_balloon driver relies
> on the condition that certain names array entries are null in order to
> skip the initialization of some virtual queues (vqs). This behavior is
> unique to this part of the codebase. In an upcoming commit, we plan to
> eliminate this dependency by removing the function entirely. Therefore,
> with this change, we are ensuring that the virtio_balloon no longer
> depends on the aforementioned function.
> 
> Signed-off-by: Xuan Zhuo <xuanzhuo at linux.alibaba.com>
> ---
>   drivers/virtio/virtio_balloon.c | 41 +++++++++++++++------------------
>   1 file changed, 19 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index 1f5b3dd31fcf..becc12a05407 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -531,49 +531,46 @@ static int init_vqs(struct virtio_balloon *vb)
>   	struct virtqueue *vqs[VIRTIO_BALLOON_VQ_MAX];
>   	vq_callback_t *callbacks[VIRTIO_BALLOON_VQ_MAX];
>   	const char *names[VIRTIO_BALLOON_VQ_MAX];
> -	int err;
> +	int err, nvqs, idx;
>   
> -	/*
> -	 * Inflateq and deflateq are used unconditionally. The names[]
> -	 * will be NULL if the related feature is not enabled, which will
> -	 * cause no allocation for the corresponding virtqueue in find_vqs.
> -	 */
>   	callbacks[VIRTIO_BALLOON_VQ_INFLATE] = balloon_ack;
>   	names[VIRTIO_BALLOON_VQ_INFLATE] = "inflate";
>   	callbacks[VIRTIO_BALLOON_VQ_DEFLATE] = balloon_ack;
>   	names[VIRTIO_BALLOON_VQ_DEFLATE] = "deflate";

I'd remove the static dependencies here completely and do it
consistently:

nvqs = 0;

callbacks[nvqs] = balloon_ack;
names[nvqs++] = "inflate";
callbacks[nvqs] = balloon_ack;
names[nvqs++] = "deflate";


> -	callbacks[VIRTIO_BALLOON_VQ_STATS] = NULL;
> -	names[VIRTIO_BALLOON_VQ_STATS] = NULL;
> -	callbacks[VIRTIO_BALLOON_VQ_FREE_PAGE] = NULL;
> -	names[VIRTIO_BALLOON_VQ_FREE_PAGE] = NULL;
> -	names[VIRTIO_BALLOON_VQ_REPORTING] = NULL;
> +
> +	nvqs = VIRTIO_BALLOON_VQ_DEFLATE + 1;
>   
>   	if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_STATS_VQ)) {
> -		names[VIRTIO_BALLOON_VQ_STATS] = "stats";
> -		callbacks[VIRTIO_BALLOON_VQ_STATS] = stats_request;
> +		names[nvqs] = "stats";
> +		callbacks[nvqs] = stats_request;
> +		++nvqs;

callbacks[nvqs++] = stats_request;

If you prefer to keep it separate, "nvqs++" please.

... same here:

idx = 0;
vb->inflate_vq = vqs[idx++];
vb->deflate_vq = vqs[idx++];

...

>   
>   	vb->inflate_vq = vqs[VIRTIO_BALLOON_VQ_INFLATE];
>   	vb->deflate_vq = vqs[VIRTIO_BALLOON_VQ_DEFLATE];
> +
> +	idx = VIRTIO_BALLOON_VQ_DEFLATE + 1;
> +
>   	if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_STATS_VQ)) {
>   		struct scatterlist sg;
>   		unsigned int num_stats;
> -		vb->stats_vq = vqs[VIRTIO_BALLOON_VQ_STATS];
> +		vb->stats_vq = vqs[idx++];
>   
>   		/*
>   		 * Prime this virtqueue with one buffer so the hypervisor can
> @@ -593,10 +590,10 @@ static int init_vqs(struct virtio_balloon *vb)
>   	}
>   
>   	if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT))
> -		vb->free_page_vq = vqs[VIRTIO_BALLOON_VQ_FREE_PAGE];
> +		vb->free_page_vq = vqs[idx++];
>   
>   	if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_REPORTING))
> -		vb->reporting_vq = vqs[VIRTIO_BALLOON_VQ_REPORTING];
> +		vb->reporting_vq = vqs[idx++];
>   

Apart from that LGTM

-- 
Cheers,

David / dhildenb