linux-block.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] make blk_mq_map_queues more friendly for cpu topology
@ 2019-03-25 15:26 luferry
  2019-03-26  7:39 ` Christoph Hellwig
  0 siblings, 1 reply; 7+ messages in thread
From: luferry @ 2019-03-25 15:26 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Dongli Zhang, Ming Lei, Christoph Hellwig, xuyun, linux-block,
	linux-kernel

under virtual machine environment, cpu topology may differ
from normal physical server.

for example (machine with 4 cores, 2 threads per core):

normal physical server:
core-id   thread-0-id  thread-1-id
0         0            4
1         1            5
2         2            6
3         3            7

virtual machine:
core-id   thread-0-id  thread-1-id
0         0            1
1         2            3
2         4            5
3         6            7

If we attach disk with multi queues to virtual machine,
blk_mq_map_queues can cause serious imbalance.this will lead to
performance impact on system IO.

Here is the qemu cmdline:
"-smp 8,sockets=1,cores=8,threads=2"
"-device virtio-blk-pci,drive=drive0,id=device0,num-queues=4,vectors=2"

when vectors less than num-queues, virtio-blk will fallback to
blk_mq_map_queues

Before this patch:
[root@blk-mq ~]# cat /sys/block/vd*/mq/*/cpu_list
0, 4, 5, 8, 9, 12, 13
1
2, 6, 7, 10, 11, 14, 15
3

After this patch:
[root@blk-mq ~]# cat /sys/block/vd*/mq/*/cpu_list
0, 1, 8, 9
2, 3, 10, 11
4, 5, 12, 13
6, 7, 14, 15

Signed-off-by: luferry <luferry@163.com>
---
 block/blk-mq-cpumap.c | 31 +++++++++++++++++++++++--------
 1 file changed, 23 insertions(+), 8 deletions(-)

diff --git a/block/blk-mq-cpumap.c b/block/blk-mq-cpumap.c
index 03a534820271..2eb78ad4a49b 100644
--- a/block/blk-mq-cpumap.c
+++ b/block/blk-mq-cpumap.c
@@ -35,22 +35,37 @@ int blk_mq_map_queues(struct blk_mq_queue_map *qmap)
 {
 	unsigned int *map = qmap->mq_map;
 	unsigned int nr_queues = qmap->nr_queues;
-	unsigned int cpu, first_sibling;
+	unsigned int cpu, first_sibling, core = 0;
+	bool core_map = false;
 
+	/*
+	 * If core num is euqal or over nr_queues,
+	 * there be sure at least per core per queue
+	 */
+	for_each_possible_cpu(cpu) {
+		if (get_first_sibling(cpu) == cpu)
+			core++;
+	}
+	if (core >= nr_queues)
+		core_map = true;
+
+	core = 0;
 	for_each_possible_cpu(cpu) {
 		/*
-		 * First do sequential mapping between CPUs and queues.
-		 * In case we still have CPUs to map, and we have some number of
-		 * threads per cores then map sibling threads to the same queue for
+		 * If cores is enough, just do map between cores and queues
+		 * else will do sequential mapping between CPUs and queues first.
+		 * For other cpus, we have some number of threads per cores
+		 * then map sibling threads to the same queue for
 		 * performace optimizations.
 		 */
-		if (cpu < nr_queues) {
+		if (!core_map && cpu < nr_queues) {
 			map[cpu] = cpu_to_queue_index(qmap, nr_queues, cpu);
 		} else {
 			first_sibling = get_first_sibling(cpu);
-			if (first_sibling == cpu)
-				map[cpu] = cpu_to_queue_index(qmap, nr_queues, cpu);
-			else
+			if (first_sibling == cpu) {
+				map[cpu] = cpu_to_queue_index(qmap, nr_queues, core);
+				core++;
+			} else
 				map[cpu] = map[first_sibling];
 		}
 	}
-- 
2.14.1.40.g8e62ba1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] make blk_mq_map_queues more friendly for cpu topology
  2019-03-25 15:26 [PATCH] make blk_mq_map_queues more friendly for cpu topology luferry
@ 2019-03-26  7:39 ` Christoph Hellwig
  2019-03-26  7:55   ` luferry
  0 siblings, 1 reply; 7+ messages in thread
From: Christoph Hellwig @ 2019-03-26  7:39 UTC (permalink / raw)
  To: luferry
  Cc: Jens Axboe, Dongli Zhang, Ming Lei, Christoph Hellwig,
	linux-block, linux-kernel

Why isn't this using the automatic PCI-level affinity assignment to
start with?

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re:Re: [PATCH] make blk_mq_map_queues more friendly for cpu topology
  2019-03-26  7:39 ` Christoph Hellwig
@ 2019-03-26  7:55   ` luferry
  2019-03-27  8:16     ` Christoph Hellwig
  0 siblings, 1 reply; 7+ messages in thread
From: luferry @ 2019-03-26  7:55 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Jens Axboe, Dongli Zhang, Ming Lei, linux-block, linux-kernel




At 2019-03-26 15:39:54, "Christoph Hellwig" <hch@lst.de> wrote:
>Why isn't this using the automatic PCI-level affinity assignment to
>start with?

When enable virtio-blk with multi queues but with only 2 msix-vector.
vp_dev->per_vq_vectors will be false, vp_get_vq_affintity will return NULL directly
so blk_mq_virtio_map_queues will fallback to blk_mq_map_queues.


448 const struct cpumask *vp_get_vq_affinity(struct virtio_device *vdev, int index)
449 {
450     struct virtio_pci_device *vp_dev = to_vp_device(vdev);
451
452     if (!vp_dev->per_vq_vectors ||
453         vp_dev->vqs[index]->msix_vector == VIRTIO_MSI_NO_VECTOR)
454         return NULL;
455
456     return pci_irq_get_affinity(vp_dev->pci_dev,
457                     vp_dev->vqs[index]->msix_vector);
458 }

 32 int blk_mq_virtio_map_queues(struct blk_mq_queue_map *qmap,
 33         struct virtio_device *vdev, int first_vec)
 34 {
 35     const struct cpumask *mask;
 36     unsigned int queue, cpu;
 37
 38     if (!vdev->config->get_vq_affinity)
 39         goto fallback;
 40
 41     for (queue = 0; queue < qmap->nr_queues; queue++) {
 42         mask = vdev->config->get_vq_affinity(vdev, first_vec + queue); //vp_get_vq_affinity return NULL
 43         if (!mask)
 44             goto fallback;
 45
 46         for_each_cpu(cpu, mask)
 47             qmap->mq_map[cpu] = qmap->queue_offset + queue;
 48     }
 49
 50     return 0;
 51 fallback:
 52     return blk_mq_map_queues(qmap);
 53 }

here is previous discussion
https://patchwork.kernel.org/patch/10865461/ 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] make blk_mq_map_queues more friendly for cpu topology
  2019-03-26  7:55   ` luferry
@ 2019-03-27  8:16     ` Christoph Hellwig
  2019-03-27  9:17       ` luferry
  0 siblings, 1 reply; 7+ messages in thread
From: Christoph Hellwig @ 2019-03-27  8:16 UTC (permalink / raw)
  To: luferry
  Cc: Christoph Hellwig, Jens Axboe, Dongli Zhang, Ming Lei,
	linux-block, linux-kernel

On Tue, Mar 26, 2019 at 03:55:10PM +0800, luferry wrote:
> 
> 
> 
> At 2019-03-26 15:39:54, "Christoph Hellwig" <hch@lst.de> wrote:
> >Why isn't this using the automatic PCI-level affinity assignment to
> >start with?
> 
> When enable virtio-blk with multi queues but with only 2 msix-vector.
> vp_dev->per_vq_vectors will be false, vp_get_vq_affintity will return NULL directly
> so blk_mq_virtio_map_queues will fallback to blk_mq_map_queues.

What is the point of the multiqueue mode if you don't have enough
(virtual) MSI-X vectors?

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re:Re: [PATCH] make blk_mq_map_queues more friendly for cpu topology
  2019-03-27  8:16     ` Christoph Hellwig
@ 2019-03-27  9:17       ` luferry
  2019-03-27  9:39         ` Dongli Zhang
  0 siblings, 1 reply; 7+ messages in thread
From: luferry @ 2019-03-27  9:17 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Jens Axboe, Dongli Zhang, Ming Lei, linux-block, linux-kernel




Actually,  I just bought one vm from public cloud provider and run into this problem.
after reading code and compare pci device info, I reproduce this scenario.

Since common users cannot change msi vector numbers, so I suggest blk_mq_map_queues to be 
more friendly. blk_mq_map_queues may be the  last choice. 





At 2019-03-27 16:16:19, "Christoph Hellwig" <hch@lst.de> wrote:
>On Tue, Mar 26, 2019 at 03:55:10PM +0800, luferry wrote:
>> 
>> 
>> 
>> At 2019-03-26 15:39:54, "Christoph Hellwig" <hch@lst.de> wrote:
>> >Why isn't this using the automatic PCI-level affinity assignment to
>> >start with?
>> 
>> When enable virtio-blk with multi queues but with only 2 msix-vector.
>> vp_dev->per_vq_vectors will be false, vp_get_vq_affintity will return NULL directly
>> so blk_mq_virtio_map_queues will fallback to blk_mq_map_queues.
>
>What is the point of the multiqueue mode if you don't have enough
>(virtual) MSI-X vectors?

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] make blk_mq_map_queues more friendly for cpu topology
  2019-03-27  9:17       ` luferry
@ 2019-03-27  9:39         ` Dongli Zhang
  2019-03-27  9:53           ` luferry
  0 siblings, 1 reply; 7+ messages in thread
From: Dongli Zhang @ 2019-03-27  9:39 UTC (permalink / raw)
  To: luferry
  Cc: Christoph Hellwig, Jens Axboe, Ming Lei, linux-block, linux-kernel



On 03/27/2019 05:17 PM, luferry wrote:
> 
> 
> 
> Actually,  I just bought one vm from public cloud provider and run into this problem.
> after reading code and compare pci device info, I reproduce this scenario.

There is lack of vector issue when number of queues is more than maxcpus as well.

https://lore.kernel.org/lkml/e4afe4c5-0262-4500-aeec-60f30734b4fc@default

I was going to send out patch to fix that specific case on virtio side.

BTW, is your VM's virtio mmio based or pci based?

As mentioned in previous email, when I was playing with firecracker by amazon,
the virtio-blk is mmio based. Not sure if that would lead to the lack of vector
issue.

Dongli Zhang

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re:Re: [PATCH] make blk_mq_map_queues more friendly for cpu topology
  2019-03-27  9:39         ` Dongli Zhang
@ 2019-03-27  9:53           ` luferry
  0 siblings, 0 replies; 7+ messages in thread
From: luferry @ 2019-03-27  9:53 UTC (permalink / raw)
  To: Dongli Zhang
  Cc: Christoph Hellwig, Jens Axboe, Ming Lei, linux-block, linux-kernel


>
>There is lack of vector issue when number of queues is more than maxcpus as well.
>
>https://lore.kernel.org/lkml/e4afe4c5-0262-4500-aeec-60f30734b4fc@default
>
>I was going to send out patch to fix that specific case on virtio side.
>
>BTW, is your VM's virtio mmio based or pci based?
>
>As mentioned in previous email, when I was playing with firecracker by amazon,
>the virtio-blk is mmio based. Not sure if that would lead to the lack of vector
>issue.
>
>Dongli Zhang

It's pci based. 

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2019-03-27  9:54 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-25 15:26 [PATCH] make blk_mq_map_queues more friendly for cpu topology luferry
2019-03-26  7:39 ` Christoph Hellwig
2019-03-26  7:55   ` luferry
2019-03-27  8:16     ` Christoph Hellwig
2019-03-27  9:17       ` luferry
2019-03-27  9:39         ` Dongli Zhang
2019-03-27  9:53           ` luferry

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).