Re: [PATCH RFC] virtio-blk: support per-device queue depth

From: Jason Wang <jasowang@redhat.com>
To: Joseph Qi <joseph.qi@linux.alibaba.com>,
	"Michael S. Tsirkin" <mst@redhat.com>
Cc: virtualization@lists.linux-foundation.org,
	linux-block@vger.kernel.org,
	Jeffle Xu <jefflexu@linux.alibaba.com>
Subject: Re: [PATCH RFC] virtio-blk: support per-device queue depth
Date: Mon, 18 Jan 2021 13:25:45 +0800	[thread overview]
Message-ID: <ab4cbc06-b629-dd35-52ac-1246d500d1c4@redhat.com> (raw)
In-Reply-To: <1610942338-78252-1-git-send-email-joseph.qi@linux.alibaba.com>

On 2021/1/18 上午11:58, Joseph Qi wrote:
> module parameter 'virtblk_queue_depth' was firstly introduced for
> testing/benchmarking purposes described in commit fc4324b4597c
> ("virtio-blk: base queue-depth on virtqueue ringsize or module param").
> Since we have different virtio-blk devices which have different
> capabilities, it requires that we support per-device queue depth instead
> of per-module. So defaultly use vq free elements if module parameter
> 'virtblk_queue_depth' is not set.

I wonder if it's better to use sysfs instead (or whether it has already 
had something like this in the blocker layer).

Thanks

>
> Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com>
> ---
>   drivers/block/virtio_blk.c | 12 +++++++-----
>   1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
> index 145606d..f83a417 100644
> --- a/drivers/block/virtio_blk.c
> +++ b/drivers/block/virtio_blk.c
> @@ -705,6 +705,7 @@ static int virtblk_probe(struct virtio_device *vdev)
>   	u32 v, blk_size, max_size, sg_elems, opt_io_size;
>   	u16 min_io_size;
>   	u8 physical_block_exp, alignment_offset;
> +	unsigned int queue_depth;
>   
>   	if (!vdev->config->get) {
>   		dev_err(&vdev->dev, "%s failure: config access disabled\n",
> @@ -755,17 +756,18 @@ static int virtblk_probe(struct virtio_device *vdev)
>   		goto out_free_vq;
>   	}
>   
> -	/* Default queue sizing is to fill the ring. */
> -	if (!virtblk_queue_depth) {
> -		virtblk_queue_depth = vblk->vqs[0].vq->num_free;
> +	if (likely(!virtblk_queue_depth)) {
> +		queue_depth = vblk->vqs[0].vq->num_free;
>   		/* ... but without indirect descs, we use 2 descs per req */
>   		if (!virtio_has_feature(vdev, VIRTIO_RING_F_INDIRECT_DESC))
> -			virtblk_queue_depth /= 2;
> +			queue_depth /= 2;
> +	} else {
> +		queue_depth = virtblk_queue_depth;
>   	}
>   
>   	memset(&vblk->tag_set, 0, sizeof(vblk->tag_set));
>   	vblk->tag_set.ops = &virtio_mq_ops;
> -	vblk->tag_set.queue_depth = virtblk_queue_depth;
> +	vblk->tag_set.queue_depth = queue_depth;
>   	vblk->tag_set.numa_node = NUMA_NO_NODE;
>   	vblk->tag_set.flags = BLK_MQ_F_SHOULD_MERGE;
>   	vblk->tag_set.cmd_size =