* [PATCH RFC] virtio-blk: support per-device queue depth
@ 2021-01-18 3:58 Joseph Qi
2021-01-18 5:25 ` Jason Wang
2021-01-19 4:14 ` Jason Wang
0 siblings, 2 replies; 20+ messages in thread
From: Joseph Qi @ 2021-01-18 3:58 UTC (permalink / raw)
To: Michael S. Tsirkin, Jason Wang; +Cc: virtualization, linux-block, Jeffle Xu
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.
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 =
--
1.8.3.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH RFC] virtio-blk: support per-device queue depth 2021-01-18 3:58 [PATCH RFC] virtio-blk: support per-device queue depth Joseph Qi @ 2021-01-18 5:25 ` Jason Wang 2021-01-19 4:14 ` Jason Wang 1 sibling, 0 replies; 20+ messages in thread From: Jason Wang @ 2021-01-18 5:25 UTC (permalink / raw) To: Joseph Qi, Michael S. Tsirkin; +Cc: virtualization, linux-block, Jeffle Xu 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 = ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RFC] virtio-blk: support per-device queue depth @ 2021-01-18 5:25 ` Jason Wang 0 siblings, 0 replies; 20+ messages in thread From: Jason Wang @ 2021-01-18 5:25 UTC (permalink / raw) To: Joseph Qi, Michael S. Tsirkin; +Cc: linux-block, virtualization 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 = _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RFC] virtio-blk: support per-device queue depth 2021-01-18 5:25 ` Jason Wang (?) @ 2021-01-18 6:06 ` Joseph Qi 2021-01-19 4:04 ` Jason Wang -1 siblings, 1 reply; 20+ messages in thread From: Joseph Qi @ 2021-01-18 6:06 UTC (permalink / raw) To: Jason Wang, Michael S. Tsirkin; +Cc: virtualization, linux-block, Jeffle Xu Hi Jason, On 1/18/21 1:25 PM, Jason Wang wrote: > > 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 for quick response. Do you mean adjust /sys/block/vdX/queue/nr_requests? But current logic in virtblk_probe() is, virtblk_queue_depth is used as a saved value for first probed vdev, not purely module parameter. Thanks, Joseph ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RFC] virtio-blk: support per-device queue depth 2021-01-18 6:06 ` Joseph Qi @ 2021-01-19 4:04 ` Jason Wang 0 siblings, 0 replies; 20+ messages in thread From: Jason Wang @ 2021-01-19 4:04 UTC (permalink / raw) To: Joseph Qi, Michael S. Tsirkin; +Cc: virtualization, linux-block, Jeffle Xu On 2021/1/18 下午2:06, Joseph Qi wrote: > Hi Jason, > > On 1/18/21 1:25 PM, Jason Wang wrote: >> 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 for quick response. > Do you mean adjust /sys/block/vdX/queue/nr_requests? > But current logic in virtblk_probe() is, virtblk_queue_depth is > used as a saved value for first probed vdev, not purely module > parameter. Right, I see. So I think the patch is fine. Thanks > > Thanks, > Joseph > ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RFC] virtio-blk: support per-device queue depth @ 2021-01-19 4:04 ` Jason Wang 0 siblings, 0 replies; 20+ messages in thread From: Jason Wang @ 2021-01-19 4:04 UTC (permalink / raw) To: Joseph Qi, Michael S. Tsirkin; +Cc: linux-block, virtualization On 2021/1/18 下午2:06, Joseph Qi wrote: > Hi Jason, > > On 1/18/21 1:25 PM, Jason Wang wrote: >> 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 for quick response. > Do you mean adjust /sys/block/vdX/queue/nr_requests? > But current logic in virtblk_probe() is, virtblk_queue_depth is > used as a saved value for first probed vdev, not purely module > parameter. Right, I see. So I think the patch is fine. Thanks > > Thanks, > Joseph > _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RFC] virtio-blk: support per-device queue depth 2021-01-18 5:25 ` Jason Wang @ 2021-01-19 1:33 ` JeffleXu -1 siblings, 0 replies; 20+ messages in thread From: JeffleXu @ 2021-01-19 1:33 UTC (permalink / raw) To: Jason Wang, Joseph Qi, Michael S. Tsirkin; +Cc: virtualization, linux-block On 1/18/21 1:25 PM, Jason Wang wrote: > > 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). > "/sys/block/<dev>/queue/nr_requests" indeed works, but isn't better to set queue_depth according to the hardware capability at the very first? AFAIK, nvme just set per-device queue_depth at initializing phase. Thanks, Jeffle > > >> >> 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 = -- Thanks, Jeffle ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RFC] virtio-blk: support per-device queue depth @ 2021-01-19 1:33 ` JeffleXu 0 siblings, 0 replies; 20+ messages in thread From: JeffleXu @ 2021-01-19 1:33 UTC (permalink / raw) To: Jason Wang, Joseph Qi, Michael S. Tsirkin; +Cc: linux-block, virtualization On 1/18/21 1:25 PM, Jason Wang wrote: > > 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). > "/sys/block/<dev>/queue/nr_requests" indeed works, but isn't better to set queue_depth according to the hardware capability at the very first? AFAIK, nvme just set per-device queue_depth at initializing phase. Thanks, Jeffle > > >> >> 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 = -- Thanks, Jeffle _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RFC] virtio-blk: support per-device queue depth 2021-01-19 1:33 ` JeffleXu @ 2021-01-19 4:06 ` Jason Wang -1 siblings, 0 replies; 20+ messages in thread From: Jason Wang @ 2021-01-19 4:06 UTC (permalink / raw) To: JeffleXu, Joseph Qi, Michael S. Tsirkin; +Cc: virtualization, linux-block On 2021/1/19 上午9:33, JeffleXu wrote: > > On 1/18/21 1:25 PM, Jason Wang wrote: >> 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). >> > "/sys/block/<dev>/queue/nr_requests" indeed works, but isn't better to > set queue_depth according to the hardware capability at the very first? > AFAIK, nvme just set per-device queue_depth at initializing phase. I agree, the problem is that the current code may modify module parameter. Thanks > > Thanks, > Jeffle > >> >>> 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 = ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RFC] virtio-blk: support per-device queue depth @ 2021-01-19 4:06 ` Jason Wang 0 siblings, 0 replies; 20+ messages in thread From: Jason Wang @ 2021-01-19 4:06 UTC (permalink / raw) To: JeffleXu, Joseph Qi, Michael S. Tsirkin; +Cc: linux-block, virtualization On 2021/1/19 上午9:33, JeffleXu wrote: > > On 1/18/21 1:25 PM, Jason Wang wrote: >> 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). >> > "/sys/block/<dev>/queue/nr_requests" indeed works, but isn't better to > set queue_depth according to the hardware capability at the very first? > AFAIK, nvme just set per-device queue_depth at initializing phase. I agree, the problem is that the current code may modify module parameter. Thanks > > Thanks, > Jeffle > >> >>> 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 = _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RFC] virtio-blk: support per-device queue depth 2021-01-19 4:06 ` Jason Wang @ 2021-01-20 1:51 ` JeffleXu -1 siblings, 0 replies; 20+ messages in thread From: JeffleXu @ 2021-01-20 1:51 UTC (permalink / raw) To: Jason Wang, Joseph Qi, Michael S. Tsirkin; +Cc: virtualization, linux-block On 1/19/21 12:06 PM, Jason Wang wrote: > > On 2021/1/19 上午9:33, JeffleXu wrote: >> >> On 1/18/21 1:25 PM, Jason Wang wrote: >>> 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). >>> >> "/sys/block/<dev>/queue/nr_requests" indeed works, but isn't better to >> set queue_depth according to the hardware capability at the very first? >> AFAIK, nvme just set per-device queue_depth at initializing phase. > > > I agree, the problem is that the current code may modify module parameter. The module parameter 'virtblk_queue_depth' is actually remained untainted. Actually it is the original code before this patch that changes the module parameter. When the module parameter is not set by boot cmdline (i.e., default to 0), it will be initialized to the queue_depth of the vring of the first probed virtio-blk device, and will be revealed to user space through '/sys/module/virtio_blk/parameters/queue_depth'. I'm not sure if this behavior is reasonable or not. The only side effect of this patch is that, now '/sys/module/virtio_blk/parameters/queue_depth' will be kept as '0' when the module parameter is not set manually. Thanks, Jeffle >> >>> >>>> 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 = -- Thanks, Jeffle ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RFC] virtio-blk: support per-device queue depth @ 2021-01-20 1:51 ` JeffleXu 0 siblings, 0 replies; 20+ messages in thread From: JeffleXu @ 2021-01-20 1:51 UTC (permalink / raw) To: Jason Wang, Joseph Qi, Michael S. Tsirkin; +Cc: linux-block, virtualization On 1/19/21 12:06 PM, Jason Wang wrote: > > On 2021/1/19 上午9:33, JeffleXu wrote: >> >> On 1/18/21 1:25 PM, Jason Wang wrote: >>> 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). >>> >> "/sys/block/<dev>/queue/nr_requests" indeed works, but isn't better to >> set queue_depth according to the hardware capability at the very first? >> AFAIK, nvme just set per-device queue_depth at initializing phase. > > > I agree, the problem is that the current code may modify module parameter. The module parameter 'virtblk_queue_depth' is actually remained untainted. Actually it is the original code before this patch that changes the module parameter. When the module parameter is not set by boot cmdline (i.e., default to 0), it will be initialized to the queue_depth of the vring of the first probed virtio-blk device, and will be revealed to user space through '/sys/module/virtio_blk/parameters/queue_depth'. I'm not sure if this behavior is reasonable or not. The only side effect of this patch is that, now '/sys/module/virtio_blk/parameters/queue_depth' will be kept as '0' when the module parameter is not set manually. Thanks, Jeffle >> >>> >>>> 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 = -- Thanks, Jeffle _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RFC] virtio-blk: support per-device queue depth 2021-01-20 1:51 ` JeffleXu @ 2021-01-20 2:47 ` Jason Wang -1 siblings, 0 replies; 20+ messages in thread From: Jason Wang @ 2021-01-20 2:47 UTC (permalink / raw) To: JeffleXu, Joseph Qi, Michael S. Tsirkin; +Cc: virtualization, linux-block On 2021/1/20 上午9:51, JeffleXu wrote: > > On 1/19/21 12:06 PM, Jason Wang wrote: >> On 2021/1/19 上午9:33, JeffleXu wrote: >>> On 1/18/21 1:25 PM, Jason Wang wrote: >>>> 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). >>>> >>> "/sys/block/<dev>/queue/nr_requests" indeed works, but isn't better to >>> set queue_depth according to the hardware capability at the very first? >>> AFAIK, nvme just set per-device queue_depth at initializing phase. >> >> I agree, the problem is that the current code may modify module parameter. > The module parameter 'virtblk_queue_depth' is actually remained untainted. > > Actually it is the original code before this patch that changes the > module parameter. Yes, that's what I meant. > When the module parameter is not set by boot cmdline > (i.e., default to 0), it will be initialized to the queue_depth of the > vring of the first probed virtio-blk device, and will be revealed to > user space through '/sys/module/virtio_blk/parameters/queue_depth'. I'm > not sure if this behavior is reasonable or not. Right, it means the virtio-blk devices that is probed after the first one can only use the queue_depth that is set according to the capability of the first virtio-blk device. > > The only side effect of this patch is that, now > '/sys/module/virtio_blk/parameters/queue_depth' will be kept as '0' when > the module parameter is not set manually. I think it's not an issue, the nr_request should be the correct way to get per device queue depth. Thanks > > > Thanks, > Jeffle > > >>>>> 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 = ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RFC] virtio-blk: support per-device queue depth @ 2021-01-20 2:47 ` Jason Wang 0 siblings, 0 replies; 20+ messages in thread From: Jason Wang @ 2021-01-20 2:47 UTC (permalink / raw) To: JeffleXu, Joseph Qi, Michael S. Tsirkin; +Cc: linux-block, virtualization On 2021/1/20 上午9:51, JeffleXu wrote: > > On 1/19/21 12:06 PM, Jason Wang wrote: >> On 2021/1/19 上午9:33, JeffleXu wrote: >>> On 1/18/21 1:25 PM, Jason Wang wrote: >>>> 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). >>>> >>> "/sys/block/<dev>/queue/nr_requests" indeed works, but isn't better to >>> set queue_depth according to the hardware capability at the very first? >>> AFAIK, nvme just set per-device queue_depth at initializing phase. >> >> I agree, the problem is that the current code may modify module parameter. > The module parameter 'virtblk_queue_depth' is actually remained untainted. > > Actually it is the original code before this patch that changes the > module parameter. Yes, that's what I meant. > When the module parameter is not set by boot cmdline > (i.e., default to 0), it will be initialized to the queue_depth of the > vring of the first probed virtio-blk device, and will be revealed to > user space through '/sys/module/virtio_blk/parameters/queue_depth'. I'm > not sure if this behavior is reasonable or not. Right, it means the virtio-blk devices that is probed after the first one can only use the queue_depth that is set according to the capability of the first virtio-blk device. > > The only side effect of this patch is that, now > '/sys/module/virtio_blk/parameters/queue_depth' will be kept as '0' when > the module parameter is not set manually. I think it's not an issue, the nr_request should be the correct way to get per device queue depth. Thanks > > > Thanks, > Jeffle > > >>>>> 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 = _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RFC] virtio-blk: support per-device queue depth 2021-01-18 3:58 [PATCH RFC] virtio-blk: support per-device queue depth Joseph Qi @ 2021-01-19 4:14 ` Jason Wang 2021-01-19 4:14 ` Jason Wang 1 sibling, 0 replies; 20+ messages in thread From: Jason Wang @ 2021-01-19 4:14 UTC (permalink / raw) To: Joseph Qi, Michael S. Tsirkin; +Cc: virtualization, linux-block, Jeffle Xu 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. > > Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com> Acked-by: Jason Wang <jasowang@redhat.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 = ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RFC] virtio-blk: support per-device queue depth @ 2021-01-19 4:14 ` Jason Wang 0 siblings, 0 replies; 20+ messages in thread From: Jason Wang @ 2021-01-19 4:14 UTC (permalink / raw) To: Joseph Qi, Michael S. Tsirkin; +Cc: linux-block, virtualization 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. > > Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com> Acked-by: Jason Wang <jasowang@redhat.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 = _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RFC] virtio-blk: support per-device queue depth 2021-01-19 4:14 ` Jason Wang (?) @ 2021-01-22 1:43 ` Joseph Qi 2021-01-22 8:34 ` Michael S. Tsirkin -1 siblings, 1 reply; 20+ messages in thread From: Joseph Qi @ 2021-01-22 1:43 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: virtualization, linux-block, Jeffle Xu, Jason Wang Hi Michael, Any comments on this patch? Thanks, Joseph On 1/19/21 12:14 PM, Jason Wang wrote: > > 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. >> >> Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com> > > > Acked-by: Jason Wang <jasowang@redhat.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 = ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RFC] virtio-blk: support per-device queue depth 2021-01-22 1:43 ` Joseph Qi @ 2021-01-22 8:34 ` Michael S. Tsirkin 0 siblings, 0 replies; 20+ messages in thread From: Michael S. Tsirkin @ 2021-01-22 8:34 UTC (permalink / raw) To: Joseph Qi; +Cc: virtualization, linux-block, Jeffle Xu, Jason Wang On Fri, Jan 22, 2021 at 09:43:27AM +0800, Joseph Qi wrote: > Hi Michael, > > Any comments on this patch? > > Thanks, > Joseph Suggest copying all reviewers, including Paolo Bonzini <pbonzini@redhat.com> and Stefan Hajnoczi <stefanha@redhat.com>. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RFC] virtio-blk: support per-device queue depth @ 2021-01-22 8:34 ` Michael S. Tsirkin 0 siblings, 0 replies; 20+ messages in thread From: Michael S. Tsirkin @ 2021-01-22 8:34 UTC (permalink / raw) To: Joseph Qi; +Cc: linux-block, virtualization On Fri, Jan 22, 2021 at 09:43:27AM +0800, Joseph Qi wrote: > Hi Michael, > > Any comments on this patch? > > Thanks, > Joseph Suggest copying all reviewers, including Paolo Bonzini <pbonzini@redhat.com> and Stefan Hajnoczi <stefanha@redhat.com>. _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RFC] virtio-blk: support per-device queue depth 2021-01-22 8:34 ` Michael S. Tsirkin (?) @ 2021-01-22 9:13 ` Joseph Qi -1 siblings, 0 replies; 20+ messages in thread From: Joseph Qi @ 2021-01-22 9:13 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: virtualization, linux-block, Jeffle Xu, Jason Wang On 1/22/21 4:34 PM, Michael S. Tsirkin wrote: > On Fri, Jan 22, 2021 at 09:43:27AM +0800, Joseph Qi wrote: >> Hi Michael, >> >> Any comments on this patch? >> >> Thanks, >> Joseph > > Suggest copying all reviewers, including Paolo Bonzini > <pbonzini@redhat.com> and Stefan Hajnoczi <stefanha@redhat.com>. > Sure, will send v2 including cc reviewers. Thanks, Joseph ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2021-01-22 12:36 UTC | newest] Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-01-18 3:58 [PATCH RFC] virtio-blk: support per-device queue depth Joseph Qi 2021-01-18 5:25 ` Jason Wang 2021-01-18 5:25 ` Jason Wang 2021-01-18 6:06 ` Joseph Qi 2021-01-19 4:04 ` Jason Wang 2021-01-19 4:04 ` Jason Wang 2021-01-19 1:33 ` JeffleXu 2021-01-19 1:33 ` JeffleXu 2021-01-19 4:06 ` Jason Wang 2021-01-19 4:06 ` Jason Wang 2021-01-20 1:51 ` JeffleXu 2021-01-20 1:51 ` JeffleXu 2021-01-20 2:47 ` Jason Wang 2021-01-20 2:47 ` Jason Wang 2021-01-19 4:14 ` Jason Wang 2021-01-19 4:14 ` Jason Wang 2021-01-22 1:43 ` Joseph Qi 2021-01-22 8:34 ` Michael S. Tsirkin 2021-01-22 8:34 ` Michael S. Tsirkin 2021-01-22 9:13 ` Joseph Qi
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.