* [PATCH] block/mq: blk map queues by core id @ 2019-03-22 10:09 luferry 2019-03-22 11:53 ` Dongli Zhang 2019-03-22 11:58 ` Dongli Zhang 0 siblings, 2 replies; 9+ messages in thread From: luferry @ 2019-03-22 10:09 UTC (permalink / raw) To: Jens Axboe; +Cc: 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 When attach disk with two queues, all the even numbered cpus will be mapped to queue 0. Under virtual machine, all the cpus is followed by its sibling cpu.Before this patch, all the odd numbered cpus will also be mapped to queue 0, can cause serious imbalance.this will lead to performance impact on system IO So suggest to allocate cpu map by core id, this can be more currency Signed-off-by: luferry <luferry@163.com> --- block/blk-mq-cpumap.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/block/blk-mq-cpumap.c b/block/blk-mq-cpumap.c index 03a534820271..4125e8e77679 100644 --- a/block/blk-mq-cpumap.c +++ b/block/blk-mq-cpumap.c @@ -35,7 +35,7 @@ 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; for_each_possible_cpu(cpu) { /* @@ -48,9 +48,10 @@ int blk_mq_map_queues(struct blk_mq_queue_map *qmap) 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] 9+ messages in thread
* Re: [PATCH] block/mq: blk map queues by core id 2019-03-22 10:09 [PATCH] block/mq: blk map queues by core id luferry @ 2019-03-22 11:53 ` Dongli Zhang 2019-03-22 11:58 ` Dongli Zhang 1 sibling, 0 replies; 9+ messages in thread From: Dongli Zhang @ 2019-03-22 11:53 UTC (permalink / raw) To: luferry Cc: Jens Axboe, Ming Lei, Christoph Hellwig, linux-block, linux-kernel On 03/22/2019 06:09 PM, luferry wrote: > under virtual machine environment, cpu topology may differ from normal Would mind share the name of virtual machine monitor, the command line if available, and which device to reproduce. For instance, I am not able to reproduce with qemu nvme or virtio-blk as I assume they use pci or virtio specific mapper to establish the mapping. E.g., with qemu and nvme: -smp 8,sockets=1,cores=4,threads=2 Indeed I use three queues instead of twp as one is reserved for admin. # ls /sys/block/nvme0n1/mq/* /sys/block/nvme0n1/mq/0: cpu0 cpu1 cpu2 cpu3 cpu_list nr_reserved_tags nr_tags /sys/block/nvme0n1/mq/1: cpu4 cpu5 cpu6 cpu7 cpu_list nr_reserved_tags nr_tags > 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 > > When attach disk with two queues, all the even numbered cpus will be > mapped to queue 0. Under virtual machine, all the cpus is followed by > its sibling cpu.Before this patch, all the odd numbered cpus will also > be mapped to queue 0, can cause serious imbalance.this will lead to > performance impact on system IO > > So suggest to allocate cpu map by core id, this can be more currency > > Signed-off-by: luferry <luferry@163.com> > --- > block/blk-mq-cpumap.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/block/blk-mq-cpumap.c b/block/blk-mq-cpumap.c > index 03a534820271..4125e8e77679 100644 > --- a/block/blk-mq-cpumap.c > +++ b/block/blk-mq-cpumap.c > @@ -35,7 +35,7 @@ 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; > > for_each_possible_cpu(cpu) { > /* > @@ -48,9 +48,10 @@ int blk_mq_map_queues(struct blk_mq_queue_map *qmap) > 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]; > } > } > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] block/mq: blk map queues by core id 2019-03-22 10:09 [PATCH] block/mq: blk map queues by core id luferry 2019-03-22 11:53 ` Dongli Zhang @ 2019-03-22 11:58 ` Dongli Zhang 2019-03-23 6:34 ` luferry 1 sibling, 1 reply; 9+ messages in thread From: Dongli Zhang @ 2019-03-22 11:58 UTC (permalink / raw) To: luferry Cc: Jens Axboe, Ming Lei, Christoph Hellwig, linux-block, linux-kernel On 03/22/2019 06:09 PM, luferry wrote: > under virtual machine environment, cpu topology may differ from normal > physical server. Would mind share the name of virtual machine monitor, the command line if available, and which device to reproduce. For instance, I am not able to reproduce with qemu nvme or virtio-blk as I assume they use pci or virtio specific mapper to establish the mapping. E.g., with qemu and nvme: -smp 8,sockets=1,cores=4,threads=2 Indeed I use three queues instead of twp as one is reserved for admin. # ls /sys/block/nvme0n1/mq/* /sys/block/nvme0n1/mq/0: cpu0 cpu1 cpu2 cpu3 cpu_list nr_reserved_tags nr_tags /sys/block/nvme0n1/mq/1: cpu4 cpu5 cpu6 cpu7 cpu_list nr_reserved_tags nr_tags Thank you very much! Dongli Zhang > 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 > > When attach disk with two queues, all the even numbered cpus will be > mapped to queue 0. Under virtual machine, all the cpus is followed by > its sibling cpu.Before this patch, all the odd numbered cpus will also > be mapped to queue 0, can cause serious imbalance.this will lead to > performance impact on system IO > > So suggest to allocate cpu map by core id, this can be more currency > > Signed-off-by: luferry <luferry@163.com> > --- > block/blk-mq-cpumap.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/block/blk-mq-cpumap.c b/block/blk-mq-cpumap.c > index 03a534820271..4125e8e77679 100644 > --- a/block/blk-mq-cpumap.c > +++ b/block/blk-mq-cpumap.c > @@ -35,7 +35,7 @@ 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; > > for_each_possible_cpu(cpu) { > /* > @@ -48,9 +48,10 @@ int blk_mq_map_queues(struct blk_mq_queue_map *qmap) > 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]; > } > } > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re:Re: [PATCH] block/mq: blk map queues by core id 2019-03-22 11:58 ` Dongli Zhang @ 2019-03-23 6:34 ` luferry 2019-03-23 11:14 ` Dongli Zhang 0 siblings, 1 reply; 9+ messages in thread From: luferry @ 2019-03-23 6:34 UTC (permalink / raw) To: Dongli Zhang Cc: Jens Axboe, Ming Lei, Christoph Hellwig, linux-block, linux-kernel I just bought one vm , so i cannot access to hypervisor. I will try to build the environment on my desktop. I'm sure about something. The hypervisor is KVM and disk driver is virtio-blk. [root@blk-mq ~]# dmesg |grep KVM [ 0.000000] Hypervisor detected: KVM [ 0.186330] Booting paravirtualized kernel on KVM [ 0.279106] KVM setup async PF for cpu 0 [ 0.420819] KVM setup async PF for cpu 1 [ 0.421682] KVM setup async PF for cpu 2 [ 0.422113] KVM setup async PF for cpu 3 [ 0.422434] KVM setup async PF for cpu 4 [ 0.422434] KVM setup async PF for cpu 5 [ 0.423312] KVM setup async PF for cpu 6 [ 0.423394] KVM setup async PF for cpu 7 [ 0.424125] KVM setup async PF for cpu 8 [ 0.424414] KVM setup async PF for cpu 9 [ 0.424415] KVM setup async PF for cpu 10 [ 0.425329] KVM setup async PF for cpu 11 [ 0.425420] KVM setup async PF for cpu 12 [ 0.426156] KVM setup async PF for cpu 13 [ 0.426431] KVM setup async PF for cpu 14 [ 0.426431] KVM setup async PF for cpu 15 [root@blk-mq ~]# lspci |grep block 00:05.0 SCSI storage controller: Red Hat, Inc. Virtio block device 00:06.0 SCSI storage controller: Red Hat, Inc. Virtio block device [root@blk-mq ~]# lscpu Architecture: x86_64 CPU op-mode(s): 32-bit, 64-bit Byte Order: Little Endian CPU(s): 16 On-line CPU(s) list: 0-15 Thread(s) per core: 2 Core(s) per socket: 8 [root@blk-mq ~]# ls /sys/block/vdb/mq/ 0 1 2 3 [root@blk-mq ~]# cat /proc/cpuinfo |egrep 'processor|core id' processor : 0 core id : 0 processor : 1 core id : 0 processor : 2 core id : 1 processor : 3 core id : 1 processor : 4 core id : 2 processor : 5 core id : 2 processor : 6 core id : 3 processor : 7 core id : 3 processor : 8 core id : 4 processor : 9 core id : 4 processor : 10 core id : 5 processor : 11 core id : 5 processor : 12 core id : 6 processor : 13 core id : 6 processor : 14 core id : 7 processor : 15 core id : 7 --before this patch-- [root@blk-mq ~]# cat /sys/block/vdb/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/vdb/mq/*/cpu_list 0, 4, 5, 12, 13 1, 6, 7, 14, 15 2, 8, 9 3, 10, 11 I add dump_stack insert blk_mq_map_queues. [ 1.378680] Call Trace: [ 1.378690] dump_stack+0x5a/0x73 [ 1.378695] blk_mq_map_queues+0x29/0xb0 [ 1.378700] blk_mq_alloc_tag_set+0x1bd/0x2d0 [ 1.378705] virtblk_probe+0x1ae/0x8e4 [virtio_blk] [ 1.378709] virtio_dev_probe+0x18a/0x230 [virtio] [ 1.378713] really_probe+0x215/0x3f0 [ 1.378716] driver_probe_device+0x115/0x130 [ 1.378718] device_driver_attach+0x50/0x60 [ 1.378720] __driver_attach+0xbd/0x140 [ 1.378722] ? device_driver_attach+0x60/0x60 [ 1.378724] bus_for_each_dev+0x67/0xc0 [ 1.378727] ? klist_add_tail+0x57/0x70 At 2019-03-22 19:58:08, "Dongli Zhang" <dongli.zhang@oracle.com> wrote: > > >On 03/22/2019 06:09 PM, luferry wrote: >> under virtual machine environment, cpu topology may differ from normal >> physical server. > >Would mind share the name of virtual machine monitor, the command line if >available, and which device to reproduce. > >For instance, I am not able to reproduce with qemu nvme or virtio-blk as I >assume they use pci or virtio specific mapper to establish the mapping. > >E.g., with qemu and nvme: -smp 8,sockets=1,cores=4,threads=2 > >Indeed I use three queues instead of twp as one is reserved for admin. > ># ls /sys/block/nvme0n1/mq/* >/sys/block/nvme0n1/mq/0: >cpu0 cpu1 cpu2 cpu3 cpu_list nr_reserved_tags nr_tags > >/sys/block/nvme0n1/mq/1: >cpu4 cpu5 cpu6 cpu7 cpu_list nr_reserved_tags nr_tags > > >Thank you very much! > >Dongli Zhang > >> 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 >> >> When attach disk with two queues, all the even numbered cpus will be >> mapped to queue 0. Under virtual machine, all the cpus is followed by >> its sibling cpu.Before this patch, all the odd numbered cpus will also >> be mapped to queue 0, can cause serious imbalance.this will lead to >> performance impact on system IO >> >> So suggest to allocate cpu map by core id, this can be more currency >> >> Signed-off-by: luferry <luferry@163.com> >> --- >> block/blk-mq-cpumap.c | 9 +++++---- >> 1 file changed, 5 insertions(+), 4 deletions(-) >> >> diff --git a/block/blk-mq-cpumap.c b/block/blk-mq-cpumap.c >> index 03a534820271..4125e8e77679 100644 >> --- a/block/blk-mq-cpumap.c >> +++ b/block/blk-mq-cpumap.c >> @@ -35,7 +35,7 @@ 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; >> >> for_each_possible_cpu(cpu) { >> /* >> @@ -48,9 +48,10 @@ int blk_mq_map_queues(struct blk_mq_queue_map *qmap) >> 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]; >> } >> } >> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] block/mq: blk map queues by core id 2019-03-23 6:34 ` luferry @ 2019-03-23 11:14 ` Dongli Zhang 2019-03-25 9:49 ` luferry 0 siblings, 1 reply; 9+ messages in thread From: Dongli Zhang @ 2019-03-23 11:14 UTC (permalink / raw) To: luferry Cc: Jens Axboe, Ming Lei, Christoph Hellwig, linux-block, linux-kernel On 03/23/2019 02:34 PM, luferry wrote: > > I just bought one vm , so i cannot access to hypervisor. I will try to build the environment on my desktop. > I'm sure about something. > The hypervisor is KVM and disk driver is virtio-blk. > [root@blk-mq ~]# dmesg |grep KVM > [ 0.000000] Hypervisor detected: KVM > [ 0.186330] Booting paravirtualized kernel on KVM > [ 0.279106] KVM setup async PF for cpu 0 > [ 0.420819] KVM setup async PF for cpu 1 > [ 0.421682] KVM setup async PF for cpu 2 > [ 0.422113] KVM setup async PF for cpu 3 > [ 0.422434] KVM setup async PF for cpu 4 > [ 0.422434] KVM setup async PF for cpu 5 > [ 0.423312] KVM setup async PF for cpu 6 > [ 0.423394] KVM setup async PF for cpu 7 > [ 0.424125] KVM setup async PF for cpu 8 > [ 0.424414] KVM setup async PF for cpu 9 > [ 0.424415] KVM setup async PF for cpu 10 > [ 0.425329] KVM setup async PF for cpu 11 > [ 0.425420] KVM setup async PF for cpu 12 > [ 0.426156] KVM setup async PF for cpu 13 > [ 0.426431] KVM setup async PF for cpu 14 > [ 0.426431] KVM setup async PF for cpu 15 > [root@blk-mq ~]# lspci |grep block > 00:05.0 SCSI storage controller: Red Hat, Inc. Virtio block device > 00:06.0 SCSI storage controller: Red Hat, Inc. Virtio block device > > [root@blk-mq ~]# lscpu > Architecture: x86_64 > CPU op-mode(s): 32-bit, 64-bit > Byte Order: Little Endian > CPU(s): 16 > On-line CPU(s) list: 0-15 > Thread(s) per core: 2 > Core(s) per socket: 8 > > [root@blk-mq ~]# ls /sys/block/vdb/mq/ > 0 1 2 3 > > [root@blk-mq ~]# cat /proc/cpuinfo |egrep 'processor|core id' > processor : 0 > core id : 0 > processor : 1 > core id : 0 > processor : 2 > core id : 1 > processor : 3 > core id : 1 > processor : 4 > core id : 2 > processor : 5 > core id : 2 > processor : 6 > core id : 3 > processor : 7 > core id : 3 > processor : 8 > core id : 4 > processor : 9 > core id : 4 > processor : 10 > core id : 5 > processor : 11 > core id : 5 > processor : 12 > core id : 6 > processor : 13 > core id : 6 > processor : 14 > core id : 7 > processor : 15 > core id : 7 > > --before this patch-- > [root@blk-mq ~]# cat /sys/block/vdb/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/vdb/mq/*/cpu_list > 0, 4, 5, 12, 13 > 1, 6, 7, 14, 15 > 2, 8, 9 > 3, 10, 11 > > > I add dump_stack insert blk_mq_map_queues. > > [ 1.378680] Call Trace: > [ 1.378690] dump_stack+0x5a/0x73 > [ 1.378695] blk_mq_map_queues+0x29/0xb0 > [ 1.378700] blk_mq_alloc_tag_set+0x1bd/0x2d0 > [ 1.378705] virtblk_probe+0x1ae/0x8e4 [virtio_blk] > [ 1.378709] virtio_dev_probe+0x18a/0x230 [virtio] > [ 1.378713] really_probe+0x215/0x3f0 > [ 1.378716] driver_probe_device+0x115/0x130 > [ 1.378718] device_driver_attach+0x50/0x60 > [ 1.378720] __driver_attach+0xbd/0x140 > [ 1.378722] ? device_driver_attach+0x60/0x60 > [ 1.378724] bus_for_each_dev+0x67/0xc0 > [ 1.378727] ? klist_add_tail+0x57/0x70 I am not able to reproduce above call stack when virtio-blk is assigned 4 queues while my qemu cmdline is "-smp 16,sockets=1,cores=8,threads=2". # cat /sys/block/vda/mq/0/cpu_list 0, 1, 2, 3 # cat /sys/block/vda/mq/1/cpu_list 4, 5, 6, 7 # cat /sys/block/vda/mq/2/cpu_list 8, 9, 10, 11 # cat /sys/block/vda/mq/3/cpu_list 12, 13, 14, 15 I do agree in above case we would have issue if the mapping is established by blk_mq_map_queues(). However, I am just curious how we finally reach at blk_mq_map_queues() from blk_mq_alloc_tag_set()? It should be something like: blk_mq_alloc_tag_set() -> blk_mq_update_queue_map() -> if (set->ops->map_queues && !is_kdump_kernel()) return set->ops->map_queues(set); -> else return blk_mq_map_queues(&set->map[HCTX_TYPE_DEFAULT]); Wouldn't we always have set->ops->map_queues = virtblk_map_queues()? Or the execution reach at: virtblk_map_queues() -> blk_mq_virtio_map_queues() -> if (!vdev->config->get_vq_affinity) return blk_mq_map_queues(qmap); -> else establish the mapping via get_vq_affinity but vdev->config->get_vq_affinity == NULL? For virtio pci, get_vq_affinity is always set. Seems virtio mmio would not set get_vq_affinity. I used to play with firecracker (by amazon) and it is interesting firecracker uses mmio to setup virtio-blk. Sorry for disturbing the review of this patch. I just would like to clarify in which scenario we would hit this issue, e.g., when virtio-blk is based on mmio? Dongli Zhang > > > At 2019-03-22 19:58:08, "Dongli Zhang" <dongli.zhang@oracle.com> wrote: >> >> >> On 03/22/2019 06:09 PM, luferry wrote: >>> under virtual machine environment, cpu topology may differ from normal >>> physical server. >> >> Would mind share the name of virtual machine monitor, the command line if >> available, and which device to reproduce. >> >> For instance, I am not able to reproduce with qemu nvme or virtio-blk as I >> assume they use pci or virtio specific mapper to establish the mapping. >> >> E.g., with qemu and nvme: -smp 8,sockets=1,cores=4,threads=2 >> >> Indeed I use three queues instead of twp as one is reserved for admin. >> >> # ls /sys/block/nvme0n1/mq/* >> /sys/block/nvme0n1/mq/0: >> cpu0 cpu1 cpu2 cpu3 cpu_list nr_reserved_tags nr_tags >> >> /sys/block/nvme0n1/mq/1: >> cpu4 cpu5 cpu6 cpu7 cpu_list nr_reserved_tags nr_tags >> >> >> Thank you very much! >> >> Dongli Zhang >> >>> 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 >>> >>> When attach disk with two queues, all the even numbered cpus will be >>> mapped to queue 0. Under virtual machine, all the cpus is followed by >>> its sibling cpu.Before this patch, all the odd numbered cpus will also >>> be mapped to queue 0, can cause serious imbalance.this will lead to >>> performance impact on system IO >>> >>> So suggest to allocate cpu map by core id, this can be more currency >>> >>> Signed-off-by: luferry <luferry@163.com> >>> --- >>> block/blk-mq-cpumap.c | 9 +++++---- >>> 1 file changed, 5 insertions(+), 4 deletions(-) >>> >>> diff --git a/block/blk-mq-cpumap.c b/block/blk-mq-cpumap.c >>> index 03a534820271..4125e8e77679 100644 >>> --- a/block/blk-mq-cpumap.c >>> +++ b/block/blk-mq-cpumap.c >>> @@ -35,7 +35,7 @@ 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; >>> >>> for_each_possible_cpu(cpu) { >>> /* >>> @@ -48,9 +48,10 @@ int blk_mq_map_queues(struct blk_mq_queue_map *qmap) >>> 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]; >>> } >>> } >>> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re:Re: [PATCH] block/mq: blk map queues by core id 2019-03-23 11:14 ` Dongli Zhang @ 2019-03-25 9:49 ` luferry 2019-03-25 9:53 ` luferry 0 siblings, 1 reply; 9+ messages in thread From: luferry @ 2019-03-25 9:49 UTC (permalink / raw) To: Dongli Zhang Cc: Jens Axboe, Ming Lei, Christoph Hellwig, linux-block, linux-kernel After reading the code and compare pci device info. I can reproduce this imbalance under KVM. When enable virtio-blk with multi queues but with only 2 msix-vector. vp_dev->per_vq_vectors will be false, so blk_mq_virtio_map_queues will fallback to virtio_mq_map_queues Since virtual machine users cannot change the vector numbers, I think virtio_mq_map_queues should be more friendly with different cpu topology. 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 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 } At 2019-03-23 19:14:34, "Dongli Zhang" <dongli.zhang@oracle.com> wrote: > > >On 03/23/2019 02:34 PM, luferry wrote: >> >> I just bought one vm , so i cannot access to hypervisor. I will try to build the environment on my desktop. >> I'm sure about something. >> The hypervisor is KVM and disk driver is virtio-blk. >> [root@blk-mq ~]# dmesg |grep KVM >> [ 0.000000] Hypervisor detected: KVM >> [ 0.186330] Booting paravirtualized kernel on KVM >> [ 0.279106] KVM setup async PF for cpu 0 >> [ 0.420819] KVM setup async PF for cpu 1 >> [ 0.421682] KVM setup async PF for cpu 2 >> [ 0.422113] KVM setup async PF for cpu 3 >> [ 0.422434] KVM setup async PF for cpu 4 >> [ 0.422434] KVM setup async PF for cpu 5 >> [ 0.423312] KVM setup async PF for cpu 6 >> [ 0.423394] KVM setup async PF for cpu 7 >> [ 0.424125] KVM setup async PF for cpu 8 >> [ 0.424414] KVM setup async PF for cpu 9 >> [ 0.424415] KVM setup async PF for cpu 10 >> [ 0.425329] KVM setup async PF for cpu 11 >> [ 0.425420] KVM setup async PF for cpu 12 >> [ 0.426156] KVM setup async PF for cpu 13 >> [ 0.426431] KVM setup async PF for cpu 14 >> [ 0.426431] KVM setup async PF for cpu 15 >> [root@blk-mq ~]# lspci |grep block >> 00:05.0 SCSI storage controller: Red Hat, Inc. Virtio block device >> 00:06.0 SCSI storage controller: Red Hat, Inc. Virtio block device >> >> [root@blk-mq ~]# lscpu >> Architecture: x86_64 >> CPU op-mode(s): 32-bit, 64-bit >> Byte Order: Little Endian >> CPU(s): 16 >> On-line CPU(s) list: 0-15 >> Thread(s) per core: 2 >> Core(s) per socket: 8 >> >> [root@blk-mq ~]# ls /sys/block/vdb/mq/ >> 0 1 2 3 >> >> [root@blk-mq ~]# cat /proc/cpuinfo |egrep 'processor|core id' >> processor : 0 >> core id : 0 >> processor : 1 >> core id : 0 >> processor : 2 >> core id : 1 >> processor : 3 >> core id : 1 >> processor : 4 >> core id : 2 >> processor : 5 >> core id : 2 >> processor : 6 >> core id : 3 >> processor : 7 >> core id : 3 >> processor : 8 >> core id : 4 >> processor : 9 >> core id : 4 >> processor : 10 >> core id : 5 >> processor : 11 >> core id : 5 >> processor : 12 >> core id : 6 >> processor : 13 >> core id : 6 >> processor : 14 >> core id : 7 >> processor : 15 >> core id : 7 >> >> --before this patch-- >> [root@blk-mq ~]# cat /sys/block/vdb/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/vdb/mq/*/cpu_list >> 0, 4, 5, 12, 13 >> 1, 6, 7, 14, 15 >> 2, 8, 9 >> 3, 10, 11 >> >> >> I add dump_stack insert blk_mq_map_queues. >> >> [ 1.378680] Call Trace: >> [ 1.378690] dump_stack+0x5a/0x73 >> [ 1.378695] blk_mq_map_queues+0x29/0xb0 >> [ 1.378700] blk_mq_alloc_tag_set+0x1bd/0x2d0 >> [ 1.378705] virtblk_probe+0x1ae/0x8e4 [virtio_blk] >> [ 1.378709] virtio_dev_probe+0x18a/0x230 [virtio] >> [ 1.378713] really_probe+0x215/0x3f0 >> [ 1.378716] driver_probe_device+0x115/0x130 >> [ 1.378718] device_driver_attach+0x50/0x60 >> [ 1.378720] __driver_attach+0xbd/0x140 >> [ 1.378722] ? device_driver_attach+0x60/0x60 >> [ 1.378724] bus_for_each_dev+0x67/0xc0 >> [ 1.378727] ? klist_add_tail+0x57/0x70 > >I am not able to reproduce above call stack when virtio-blk is assigned 4 queues >while my qemu cmdline is "-smp 16,sockets=1,cores=8,threads=2". > ># cat /sys/block/vda/mq/0/cpu_list >0, 1, 2, 3 ># cat /sys/block/vda/mq/1/cpu_list >4, 5, 6, 7 ># cat /sys/block/vda/mq/2/cpu_list >8, 9, 10, 11 ># cat /sys/block/vda/mq/3/cpu_list >12, 13, 14, 15 > > >I do agree in above case we would have issue if the mapping is established by >blk_mq_map_queues(). > > >However, I am just curious how we finally reach at blk_mq_map_queues() from >blk_mq_alloc_tag_set()? > >It should be something like: > >blk_mq_alloc_tag_set() > -> blk_mq_update_queue_map() > -> if (set->ops->map_queues && !is_kdump_kernel()) > return set->ops->map_queues(set); > -> else > return blk_mq_map_queues(&set->map[HCTX_TYPE_DEFAULT]); > >Wouldn't we always have set->ops->map_queues = virtblk_map_queues()? > >Or the execution reach at: > >virtblk_map_queues() > -> blk_mq_virtio_map_queues() > -> if (!vdev->config->get_vq_affinity) > return blk_mq_map_queues(qmap); > -> else > establish the mapping via get_vq_affinity > >but vdev->config->get_vq_affinity == NULL? > >For virtio pci, get_vq_affinity is always set. Seems virtio mmio would not set >get_vq_affinity. > > >I used to play with firecracker (by amazon) and it is interesting firecracker >uses mmio to setup virtio-blk. > > >Sorry for disturbing the review of this patch. I just would like to clarify in >which scenario we would hit this issue, e.g., when virtio-blk is based on mmio? > >Dongli Zhang > >> >> >> At 2019-03-22 19:58:08, "Dongli Zhang" <dongli.zhang@oracle.com> wrote: >>> >>> >>> On 03/22/2019 06:09 PM, luferry wrote: >>>> under virtual machine environment, cpu topology may differ from normal >>>> physical server. >>> >>> Would mind share the name of virtual machine monitor, the command line if >>> available, and which device to reproduce. >>> >>> For instance, I am not able to reproduce with qemu nvme or virtio-blk as I >>> assume they use pci or virtio specific mapper to establish the mapping. >>> >>> E.g., with qemu and nvme: -smp 8,sockets=1,cores=4,threads=2 >>> >>> Indeed I use three queues instead of twp as one is reserved for admin. >>> >>> # ls /sys/block/nvme0n1/mq/* >>> /sys/block/nvme0n1/mq/0: >>> cpu0 cpu1 cpu2 cpu3 cpu_list nr_reserved_tags nr_tags >>> >>> /sys/block/nvme0n1/mq/1: >>> cpu4 cpu5 cpu6 cpu7 cpu_list nr_reserved_tags nr_tags >>> >>> >>> Thank you very much! >>> >>> Dongli Zhang >>> >>>> 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 >>>> >>>> When attach disk with two queues, all the even numbered cpus will be >>>> mapped to queue 0. Under virtual machine, all the cpus is followed by >>>> its sibling cpu.Before this patch, all the odd numbered cpus will also >>>> be mapped to queue 0, can cause serious imbalance.this will lead to >>>> performance impact on system IO >>>> >>>> So suggest to allocate cpu map by core id, this can be more currency >>>> >>>> Signed-off-by: luferry <luferry@163.com> >>>> --- >>>> block/blk-mq-cpumap.c | 9 +++++---- >>>> 1 file changed, 5 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/block/blk-mq-cpumap.c b/block/blk-mq-cpumap.c >>>> index 03a534820271..4125e8e77679 100644 >>>> --- a/block/blk-mq-cpumap.c >>>> +++ b/block/blk-mq-cpumap.c >>>> @@ -35,7 +35,7 @@ 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; >>>> >>>> for_each_possible_cpu(cpu) { >>>> /* >>>> @@ -48,9 +48,10 @@ int blk_mq_map_queues(struct blk_mq_queue_map *qmap) >>>> 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]; >>>> } >>>> } >>>> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re:Re:Re: [PATCH] block/mq: blk map queues by core id 2019-03-25 9:49 ` luferry @ 2019-03-25 9:53 ` luferry 2019-03-25 13:53 ` Dongli Zhang 0 siblings, 1 reply; 9+ messages in thread From: luferry @ 2019-03-25 9:53 UTC (permalink / raw) To: luferry Cc: Dongli Zhang, Jens Axboe, Ming Lei, Christoph Hellwig, linux-block, linux-kernel sorry, messed up some func name When enable virtio-blk with multi queues but with only 2 msix-vector. vp_dev->per_vq_vectors will be false, so blk_mq_virtio_map_queues will fallback to blk_mq_map_queues Since virtual machine users cannot change the vector numbers, I think blk_mq_map_queues should be more friendly with different cpu topology. At 2019-03-25 17:49:33, "luferry" <luferry@163.com> wrote: > >After reading the code and compare pci device info. >I can reproduce this imbalance under KVM. >When enable virtio-blk with multi queues but with only 2 msix-vector. >vp_dev->per_vq_vectors will be false, so blk_mq_virtio_map_queues will fallback to virtio_mq_map_queues > >Since virtual machine users cannot change the vector numbers, I think virtio_mq_map_queues should be more friendly with different cpu topology. > >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 > 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 } > > > > > > > > >At 2019-03-23 19:14:34, "Dongli Zhang" <dongli.zhang@oracle.com> wrote: >> >> >>On 03/23/2019 02:34 PM, luferry wrote: >>> >>> I just bought one vm , so i cannot access to hypervisor. I will try to build the environment on my desktop. >>> I'm sure about something. >>> The hypervisor is KVM and disk driver is virtio-blk. >>> [root@blk-mq ~]# dmesg |grep KVM >>> [ 0.000000] Hypervisor detected: KVM >>> [ 0.186330] Booting paravirtualized kernel on KVM >>> [ 0.279106] KVM setup async PF for cpu 0 >>> [ 0.420819] KVM setup async PF for cpu 1 >>> [ 0.421682] KVM setup async PF for cpu 2 >>> [ 0.422113] KVM setup async PF for cpu 3 >>> [ 0.422434] KVM setup async PF for cpu 4 >>> [ 0.422434] KVM setup async PF for cpu 5 >>> [ 0.423312] KVM setup async PF for cpu 6 >>> [ 0.423394] KVM setup async PF for cpu 7 >>> [ 0.424125] KVM setup async PF for cpu 8 >>> [ 0.424414] KVM setup async PF for cpu 9 >>> [ 0.424415] KVM setup async PF for cpu 10 >>> [ 0.425329] KVM setup async PF for cpu 11 >>> [ 0.425420] KVM setup async PF for cpu 12 >>> [ 0.426156] KVM setup async PF for cpu 13 >>> [ 0.426431] KVM setup async PF for cpu 14 >>> [ 0.426431] KVM setup async PF for cpu 15 >>> [root@blk-mq ~]# lspci |grep block >>> 00:05.0 SCSI storage controller: Red Hat, Inc. Virtio block device >>> 00:06.0 SCSI storage controller: Red Hat, Inc. Virtio block device >>> >>> [root@blk-mq ~]# lscpu >>> Architecture: x86_64 >>> CPU op-mode(s): 32-bit, 64-bit >>> Byte Order: Little Endian >>> CPU(s): 16 >>> On-line CPU(s) list: 0-15 >>> Thread(s) per core: 2 >>> Core(s) per socket: 8 >>> >>> [root@blk-mq ~]# ls /sys/block/vdb/mq/ >>> 0 1 2 3 >>> >>> [root@blk-mq ~]# cat /proc/cpuinfo |egrep 'processor|core id' >>> processor : 0 >>> core id : 0 >>> processor : 1 >>> core id : 0 >>> processor : 2 >>> core id : 1 >>> processor : 3 >>> core id : 1 >>> processor : 4 >>> core id : 2 >>> processor : 5 >>> core id : 2 >>> processor : 6 >>> core id : 3 >>> processor : 7 >>> core id : 3 >>> processor : 8 >>> core id : 4 >>> processor : 9 >>> core id : 4 >>> processor : 10 >>> core id : 5 >>> processor : 11 >>> core id : 5 >>> processor : 12 >>> core id : 6 >>> processor : 13 >>> core id : 6 >>> processor : 14 >>> core id : 7 >>> processor : 15 >>> core id : 7 >>> >>> --before this patch-- >>> [root@blk-mq ~]# cat /sys/block/vdb/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/vdb/mq/*/cpu_list >>> 0, 4, 5, 12, 13 >>> 1, 6, 7, 14, 15 >>> 2, 8, 9 >>> 3, 10, 11 >>> >>> >>> I add dump_stack insert blk_mq_map_queues. >>> >>> [ 1.378680] Call Trace: >>> [ 1.378690] dump_stack+0x5a/0x73 >>> [ 1.378695] blk_mq_map_queues+0x29/0xb0 >>> [ 1.378700] blk_mq_alloc_tag_set+0x1bd/0x2d0 >>> [ 1.378705] virtblk_probe+0x1ae/0x8e4 [virtio_blk] >>> [ 1.378709] virtio_dev_probe+0x18a/0x230 [virtio] >>> [ 1.378713] really_probe+0x215/0x3f0 >>> [ 1.378716] driver_probe_device+0x115/0x130 >>> [ 1.378718] device_driver_attach+0x50/0x60 >>> [ 1.378720] __driver_attach+0xbd/0x140 >>> [ 1.378722] ? device_driver_attach+0x60/0x60 >>> [ 1.378724] bus_for_each_dev+0x67/0xc0 >>> [ 1.378727] ? klist_add_tail+0x57/0x70 >> >>I am not able to reproduce above call stack when virtio-blk is assigned 4 queues >>while my qemu cmdline is "-smp 16,sockets=1,cores=8,threads=2". >> >># cat /sys/block/vda/mq/0/cpu_list >>0, 1, 2, 3 >># cat /sys/block/vda/mq/1/cpu_list >>4, 5, 6, 7 >># cat /sys/block/vda/mq/2/cpu_list >>8, 9, 10, 11 >># cat /sys/block/vda/mq/3/cpu_list >>12, 13, 14, 15 >> >> >>I do agree in above case we would have issue if the mapping is established by >>blk_mq_map_queues(). >> >> >>However, I am just curious how we finally reach at blk_mq_map_queues() from >>blk_mq_alloc_tag_set()? >> >>It should be something like: >> >>blk_mq_alloc_tag_set() >> -> blk_mq_update_queue_map() >> -> if (set->ops->map_queues && !is_kdump_kernel()) >> return set->ops->map_queues(set); >> -> else >> return blk_mq_map_queues(&set->map[HCTX_TYPE_DEFAULT]); >> >>Wouldn't we always have set->ops->map_queues = virtblk_map_queues()? >> >>Or the execution reach at: >> >>virtblk_map_queues() >> -> blk_mq_virtio_map_queues() >> -> if (!vdev->config->get_vq_affinity) >> return blk_mq_map_queues(qmap); >> -> else >> establish the mapping via get_vq_affinity >> >>but vdev->config->get_vq_affinity == NULL? >> >>For virtio pci, get_vq_affinity is always set. Seems virtio mmio would not set >>get_vq_affinity. >> >> >>I used to play with firecracker (by amazon) and it is interesting firecracker >>uses mmio to setup virtio-blk. >> >> >>Sorry for disturbing the review of this patch. I just would like to clarify in >>which scenario we would hit this issue, e.g., when virtio-blk is based on mmio? >> >>Dongli Zhang >> >>> >>> >>> At 2019-03-22 19:58:08, "Dongli Zhang" <dongli.zhang@oracle.com> wrote: >>>> >>>> >>>> On 03/22/2019 06:09 PM, luferry wrote: >>>>> under virtual machine environment, cpu topology may differ from normal >>>>> physical server. >>>> >>>> Would mind share the name of virtual machine monitor, the command line if >>>> available, and which device to reproduce. >>>> >>>> For instance, I am not able to reproduce with qemu nvme or virtio-blk as I >>>> assume they use pci or virtio specific mapper to establish the mapping. >>>> >>>> E.g., with qemu and nvme: -smp 8,sockets=1,cores=4,threads=2 >>>> >>>> Indeed I use three queues instead of twp as one is reserved for admin. >>>> >>>> # ls /sys/block/nvme0n1/mq/* >>>> /sys/block/nvme0n1/mq/0: >>>> cpu0 cpu1 cpu2 cpu3 cpu_list nr_reserved_tags nr_tags >>>> >>>> /sys/block/nvme0n1/mq/1: >>>> cpu4 cpu5 cpu6 cpu7 cpu_list nr_reserved_tags nr_tags >>>> >>>> >>>> Thank you very much! >>>> >>>> Dongli Zhang >>>> >>>>> 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 >>>>> >>>>> When attach disk with two queues, all the even numbered cpus will be >>>>> mapped to queue 0. Under virtual machine, all the cpus is followed by >>>>> its sibling cpu.Before this patch, all the odd numbered cpus will also >>>>> be mapped to queue 0, can cause serious imbalance.this will lead to >>>>> performance impact on system IO >>>>> >>>>> So suggest to allocate cpu map by core id, this can be more currency >>>>> >>>>> Signed-off-by: luferry <luferry@163.com> >>>>> --- >>>>> block/blk-mq-cpumap.c | 9 +++++---- >>>>> 1 file changed, 5 insertions(+), 4 deletions(-) >>>>> >>>>> diff --git a/block/blk-mq-cpumap.c b/block/blk-mq-cpumap.c >>>>> index 03a534820271..4125e8e77679 100644 >>>>> --- a/block/blk-mq-cpumap.c >>>>> +++ b/block/blk-mq-cpumap.c >>>>> @@ -35,7 +35,7 @@ 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; >>>>> >>>>> for_each_possible_cpu(cpu) { >>>>> /* >>>>> @@ -48,9 +48,10 @@ int blk_mq_map_queues(struct blk_mq_queue_map *qmap) >>>>> 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]; >>>>> } >>>>> } >>>>> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] block/mq: blk map queues by core id 2019-03-25 9:53 ` luferry @ 2019-03-25 13:53 ` Dongli Zhang 2019-03-25 15:17 ` luferry 0 siblings, 1 reply; 9+ messages in thread From: Dongli Zhang @ 2019-03-25 13:53 UTC (permalink / raw) To: luferry Cc: Jens Axboe, Ming Lei, Christoph Hellwig, linux-block, linux-kernel On 03/25/2019 05:53 PM, luferry wrote: > > sorry, messed up some func name > > When enable virtio-blk with multi queues but with only 2 msix-vector. > vp_dev->per_vq_vectors will be false, so blk_mq_virtio_map_queues will fallback to blk_mq_map_queues > > Since virtual machine users cannot change the vector numbers, I think blk_mq_map_queues should be more friendly with different cpu topology. Yes, we will fallback to blk_mq_map_queues if there is lack of vectors. Here is the qemu cmdline: "-smp 8,sockets=1,cores=4,threads=2" "-device virtio-blk-pci,drive=drive0,id=device0,num-queues=4,vectors=2" # cat /sys/block/vda/mq/0/cpu_list 0, 4, 5 root@vm:/home/zhang# cat /sys/block/vda/mq/1/cpu_list 1 root@vm:/home/zhang# cat /sys/block/vda/mq/2/cpu_list 2, 6, 7 root@vm:/home/zhang# cat /sys/block/vda/mq/3/cpu_list 3 How about to put how to hit the issue in commit message so people would be able to know the scenario? Dongli Zhang > > > > > At 2019-03-25 17:49:33, "luferry" <luferry@163.com> wrote: >> >> After reading the code and compare pci device info. >> I can reproduce this imbalance under KVM. >> When enable virtio-blk with multi queues but with only 2 msix-vector. >> vp_dev->per_vq_vectors will be false, so blk_mq_virtio_map_queues will fallback to virtio_mq_map_queues >> >> Since virtual machine users cannot change the vector numbers, I think virtio_mq_map_queues should be more friendly with different cpu topology. >> >> 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 >> 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 } >> >> >> >> >> >> >> >> >> At 2019-03-23 19:14:34, "Dongli Zhang" <dongli.zhang@oracle.com> wrote: >>> >>> >>> On 03/23/2019 02:34 PM, luferry wrote: >>>> >>>> I just bought one vm , so i cannot access to hypervisor. I will try to build the environment on my desktop. >>>> I'm sure about something. >>>> The hypervisor is KVM and disk driver is virtio-blk. >>>> [root@blk-mq ~]# dmesg |grep KVM >>>> [ 0.000000] Hypervisor detected: KVM >>>> [ 0.186330] Booting paravirtualized kernel on KVM >>>> [ 0.279106] KVM setup async PF for cpu 0 >>>> [ 0.420819] KVM setup async PF for cpu 1 >>>> [ 0.421682] KVM setup async PF for cpu 2 >>>> [ 0.422113] KVM setup async PF for cpu 3 >>>> [ 0.422434] KVM setup async PF for cpu 4 >>>> [ 0.422434] KVM setup async PF for cpu 5 >>>> [ 0.423312] KVM setup async PF for cpu 6 >>>> [ 0.423394] KVM setup async PF for cpu 7 >>>> [ 0.424125] KVM setup async PF for cpu 8 >>>> [ 0.424414] KVM setup async PF for cpu 9 >>>> [ 0.424415] KVM setup async PF for cpu 10 >>>> [ 0.425329] KVM setup async PF for cpu 11 >>>> [ 0.425420] KVM setup async PF for cpu 12 >>>> [ 0.426156] KVM setup async PF for cpu 13 >>>> [ 0.426431] KVM setup async PF for cpu 14 >>>> [ 0.426431] KVM setup async PF for cpu 15 >>>> [root@blk-mq ~]# lspci |grep block >>>> 00:05.0 SCSI storage controller: Red Hat, Inc. Virtio block device >>>> 00:06.0 SCSI storage controller: Red Hat, Inc. Virtio block device >>>> >>>> [root@blk-mq ~]# lscpu >>>> Architecture: x86_64 >>>> CPU op-mode(s): 32-bit, 64-bit >>>> Byte Order: Little Endian >>>> CPU(s): 16 >>>> On-line CPU(s) list: 0-15 >>>> Thread(s) per core: 2 >>>> Core(s) per socket: 8 >>>> >>>> [root@blk-mq ~]# ls /sys/block/vdb/mq/ >>>> 0 1 2 3 >>>> >>>> [root@blk-mq ~]# cat /proc/cpuinfo |egrep 'processor|core id' >>>> processor : 0 >>>> core id : 0 >>>> processor : 1 >>>> core id : 0 >>>> processor : 2 >>>> core id : 1 >>>> processor : 3 >>>> core id : 1 >>>> processor : 4 >>>> core id : 2 >>>> processor : 5 >>>> core id : 2 >>>> processor : 6 >>>> core id : 3 >>>> processor : 7 >>>> core id : 3 >>>> processor : 8 >>>> core id : 4 >>>> processor : 9 >>>> core id : 4 >>>> processor : 10 >>>> core id : 5 >>>> processor : 11 >>>> core id : 5 >>>> processor : 12 >>>> core id : 6 >>>> processor : 13 >>>> core id : 6 >>>> processor : 14 >>>> core id : 7 >>>> processor : 15 >>>> core id : 7 >>>> >>>> --before this patch-- >>>> [root@blk-mq ~]# cat /sys/block/vdb/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/vdb/mq/*/cpu_list >>>> 0, 4, 5, 12, 13 >>>> 1, 6, 7, 14, 15 >>>> 2, 8, 9 >>>> 3, 10, 11 >>>> >>>> >>>> I add dump_stack insert blk_mq_map_queues. >>>> >>>> [ 1.378680] Call Trace: >>>> [ 1.378690] dump_stack+0x5a/0x73 >>>> [ 1.378695] blk_mq_map_queues+0x29/0xb0 >>>> [ 1.378700] blk_mq_alloc_tag_set+0x1bd/0x2d0 >>>> [ 1.378705] virtblk_probe+0x1ae/0x8e4 [virtio_blk] >>>> [ 1.378709] virtio_dev_probe+0x18a/0x230 [virtio] >>>> [ 1.378713] really_probe+0x215/0x3f0 >>>> [ 1.378716] driver_probe_device+0x115/0x130 >>>> [ 1.378718] device_driver_attach+0x50/0x60 >>>> [ 1.378720] __driver_attach+0xbd/0x140 >>>> [ 1.378722] ? device_driver_attach+0x60/0x60 >>>> [ 1.378724] bus_for_each_dev+0x67/0xc0 >>>> [ 1.378727] ? klist_add_tail+0x57/0x70 >>> >>> I am not able to reproduce above call stack when virtio-blk is assigned 4 queues >>> while my qemu cmdline is "-smp 16,sockets=1,cores=8,threads=2". >>> >>> # cat /sys/block/vda/mq/0/cpu_list >>> 0, 1, 2, 3 >>> # cat /sys/block/vda/mq/1/cpu_list >>> 4, 5, 6, 7 >>> # cat /sys/block/vda/mq/2/cpu_list >>> 8, 9, 10, 11 >>> # cat /sys/block/vda/mq/3/cpu_list >>> 12, 13, 14, 15 >>> >>> >>> I do agree in above case we would have issue if the mapping is established by >>> blk_mq_map_queues(). >>> >>> >>> However, I am just curious how we finally reach at blk_mq_map_queues() from >>> blk_mq_alloc_tag_set()? >>> >>> It should be something like: >>> >>> blk_mq_alloc_tag_set() >>> -> blk_mq_update_queue_map() >>> -> if (set->ops->map_queues && !is_kdump_kernel()) >>> return set->ops->map_queues(set); >>> -> else >>> return blk_mq_map_queues(&set->map[HCTX_TYPE_DEFAULT]); >>> >>> Wouldn't we always have set->ops->map_queues = virtblk_map_queues()? >>> >>> Or the execution reach at: >>> >>> virtblk_map_queues() >>> -> blk_mq_virtio_map_queues() >>> -> if (!vdev->config->get_vq_affinity) >>> return blk_mq_map_queues(qmap); >>> -> else >>> establish the mapping via get_vq_affinity >>> >>> but vdev->config->get_vq_affinity == NULL? >>> >>> For virtio pci, get_vq_affinity is always set. Seems virtio mmio would not set >>> get_vq_affinity. >>> >>> >>> I used to play with firecracker (by amazon) and it is interesting firecracker >>> uses mmio to setup virtio-blk. >>> >>> >>> Sorry for disturbing the review of this patch. I just would like to clarify in >>> which scenario we would hit this issue, e.g., when virtio-blk is based on mmio? >>> >>> Dongli Zhang >>> >>>> >>>> >>>> At 2019-03-22 19:58:08, "Dongli Zhang" <dongli.zhang@oracle.com> wrote: >>>>> >>>>> >>>>> On 03/22/2019 06:09 PM, luferry wrote: >>>>>> under virtual machine environment, cpu topology may differ from normal >>>>>> physical server. >>>>> >>>>> Would mind share the name of virtual machine monitor, the command line if >>>>> available, and which device to reproduce. >>>>> >>>>> For instance, I am not able to reproduce with qemu nvme or virtio-blk as I >>>>> assume they use pci or virtio specific mapper to establish the mapping. >>>>> >>>>> E.g., with qemu and nvme: -smp 8,sockets=1,cores=4,threads=2 >>>>> >>>>> Indeed I use three queues instead of twp as one is reserved for admin. >>>>> >>>>> # ls /sys/block/nvme0n1/mq/* >>>>> /sys/block/nvme0n1/mq/0: >>>>> cpu0 cpu1 cpu2 cpu3 cpu_list nr_reserved_tags nr_tags >>>>> >>>>> /sys/block/nvme0n1/mq/1: >>>>> cpu4 cpu5 cpu6 cpu7 cpu_list nr_reserved_tags nr_tags >>>>> >>>>> >>>>> Thank you very much! >>>>> >>>>> Dongli Zhang >>>>> >>>>>> 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 >>>>>> >>>>>> When attach disk with two queues, all the even numbered cpus will be >>>>>> mapped to queue 0. Under virtual machine, all the cpus is followed by >>>>>> its sibling cpu.Before this patch, all the odd numbered cpus will also >>>>>> be mapped to queue 0, can cause serious imbalance.this will lead to >>>>>> performance impact on system IO >>>>>> >>>>>> So suggest to allocate cpu map by core id, this can be more currency >>>>>> >>>>>> Signed-off-by: luferry <luferry@163.com> >>>>>> --- >>>>>> block/blk-mq-cpumap.c | 9 +++++---- >>>>>> 1 file changed, 5 insertions(+), 4 deletions(-) >>>>>> >>>>>> diff --git a/block/blk-mq-cpumap.c b/block/blk-mq-cpumap.c >>>>>> index 03a534820271..4125e8e77679 100644 >>>>>> --- a/block/blk-mq-cpumap.c >>>>>> +++ b/block/blk-mq-cpumap.c >>>>>> @@ -35,7 +35,7 @@ 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; >>>>>> >>>>>> for_each_possible_cpu(cpu) { >>>>>> /* >>>>>> @@ -48,9 +48,10 @@ int blk_mq_map_queues(struct blk_mq_queue_map *qmap) >>>>>> 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]; >>>>>> } >>>>>> } >>>>>> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re:Re: [PATCH] block/mq: blk map queues by core id 2019-03-25 13:53 ` Dongli Zhang @ 2019-03-25 15:17 ` luferry 0 siblings, 0 replies; 9+ messages in thread From: luferry @ 2019-03-25 15:17 UTC (permalink / raw) To: Dongli Zhang Cc: Jens Axboe, Ming Lei, Christoph Hellwig, linux-block, linux-kernel Thanks for your kindly suggestions, I will reformat this patch later! At 2019-03-25 21:53:00, "Dongli Zhang" <dongli.zhang@oracle.com> wrote: > > >On 03/25/2019 05:53 PM, luferry wrote: >> >> sorry, messed up some func name >> >> When enable virtio-blk with multi queues but with only 2 msix-vector. >> vp_dev->per_vq_vectors will be false, so blk_mq_virtio_map_queues will fallback to blk_mq_map_queues >> >> Since virtual machine users cannot change the vector numbers, I think blk_mq_map_queues should be more friendly with different cpu topology. > > >Yes, we will fallback to blk_mq_map_queues if there is lack of vectors. > >Here is the qemu cmdline: >"-smp 8,sockets=1,cores=4,threads=2" >"-device virtio-blk-pci,drive=drive0,id=device0,num-queues=4,vectors=2" > ># cat /sys/block/vda/mq/0/cpu_list >0, 4, 5 >root@vm:/home/zhang# cat /sys/block/vda/mq/1/cpu_list >1 >root@vm:/home/zhang# cat /sys/block/vda/mq/2/cpu_list >2, 6, 7 >root@vm:/home/zhang# cat /sys/block/vda/mq/3/cpu_list >3 > >How about to put how to hit the issue in commit message so people would be able >to know the scenario? > >Dongli Zhang > >> >> >> >> >> At 2019-03-25 17:49:33, "luferry" <luferry@163.com> wrote: >>> >>> After reading the code and compare pci device info. >>> I can reproduce this imbalance under KVM. >>> When enable virtio-blk with multi queues but with only 2 msix-vector. >>> vp_dev->per_vq_vectors will be false, so blk_mq_virtio_map_queues will fallback to virtio_mq_map_queues >>> >>> Since virtual machine users cannot change the vector numbers, I think virtio_mq_map_queues should be more friendly with different cpu topology. >>> >>> 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 >>> 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 } >>> >>> >>> >>> >>> >>> >>> >>> >>> At 2019-03-23 19:14:34, "Dongli Zhang" <dongli.zhang@oracle.com> wrote: >>>> >>>> >>>> On 03/23/2019 02:34 PM, luferry wrote: >>>>> >>>>> I just bought one vm , so i cannot access to hypervisor. I will try to build the environment on my desktop. >>>>> I'm sure about something. >>>>> The hypervisor is KVM and disk driver is virtio-blk. >>>>> [root@blk-mq ~]# dmesg |grep KVM >>>>> [ 0.000000] Hypervisor detected: KVM >>>>> [ 0.186330] Booting paravirtualized kernel on KVM >>>>> [ 0.279106] KVM setup async PF for cpu 0 >>>>> [ 0.420819] KVM setup async PF for cpu 1 >>>>> [ 0.421682] KVM setup async PF for cpu 2 >>>>> [ 0.422113] KVM setup async PF for cpu 3 >>>>> [ 0.422434] KVM setup async PF for cpu 4 >>>>> [ 0.422434] KVM setup async PF for cpu 5 >>>>> [ 0.423312] KVM setup async PF for cpu 6 >>>>> [ 0.423394] KVM setup async PF for cpu 7 >>>>> [ 0.424125] KVM setup async PF for cpu 8 >>>>> [ 0.424414] KVM setup async PF for cpu 9 >>>>> [ 0.424415] KVM setup async PF for cpu 10 >>>>> [ 0.425329] KVM setup async PF for cpu 11 >>>>> [ 0.425420] KVM setup async PF for cpu 12 >>>>> [ 0.426156] KVM setup async PF for cpu 13 >>>>> [ 0.426431] KVM setup async PF for cpu 14 >>>>> [ 0.426431] KVM setup async PF for cpu 15 >>>>> [root@blk-mq ~]# lspci |grep block >>>>> 00:05.0 SCSI storage controller: Red Hat, Inc. Virtio block device >>>>> 00:06.0 SCSI storage controller: Red Hat, Inc. Virtio block device >>>>> >>>>> [root@blk-mq ~]# lscpu >>>>> Architecture: x86_64 >>>>> CPU op-mode(s): 32-bit, 64-bit >>>>> Byte Order: Little Endian >>>>> CPU(s): 16 >>>>> On-line CPU(s) list: 0-15 >>>>> Thread(s) per core: 2 >>>>> Core(s) per socket: 8 >>>>> >>>>> [root@blk-mq ~]# ls /sys/block/vdb/mq/ >>>>> 0 1 2 3 >>>>> >>>>> [root@blk-mq ~]# cat /proc/cpuinfo |egrep 'processor|core id' >>>>> processor : 0 >>>>> core id : 0 >>>>> processor : 1 >>>>> core id : 0 >>>>> processor : 2 >>>>> core id : 1 >>>>> processor : 3 >>>>> core id : 1 >>>>> processor : 4 >>>>> core id : 2 >>>>> processor : 5 >>>>> core id : 2 >>>>> processor : 6 >>>>> core id : 3 >>>>> processor : 7 >>>>> core id : 3 >>>>> processor : 8 >>>>> core id : 4 >>>>> processor : 9 >>>>> core id : 4 >>>>> processor : 10 >>>>> core id : 5 >>>>> processor : 11 >>>>> core id : 5 >>>>> processor : 12 >>>>> core id : 6 >>>>> processor : 13 >>>>> core id : 6 >>>>> processor : 14 >>>>> core id : 7 >>>>> processor : 15 >>>>> core id : 7 >>>>> >>>>> --before this patch-- >>>>> [root@blk-mq ~]# cat /sys/block/vdb/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/vdb/mq/*/cpu_list >>>>> 0, 4, 5, 12, 13 >>>>> 1, 6, 7, 14, 15 >>>>> 2, 8, 9 >>>>> 3, 10, 11 >>>>> >>>>> >>>>> I add dump_stack insert blk_mq_map_queues. >>>>> >>>>> [ 1.378680] Call Trace: >>>>> [ 1.378690] dump_stack+0x5a/0x73 >>>>> [ 1.378695] blk_mq_map_queues+0x29/0xb0 >>>>> [ 1.378700] blk_mq_alloc_tag_set+0x1bd/0x2d0 >>>>> [ 1.378705] virtblk_probe+0x1ae/0x8e4 [virtio_blk] >>>>> [ 1.378709] virtio_dev_probe+0x18a/0x230 [virtio] >>>>> [ 1.378713] really_probe+0x215/0x3f0 >>>>> [ 1.378716] driver_probe_device+0x115/0x130 >>>>> [ 1.378718] device_driver_attach+0x50/0x60 >>>>> [ 1.378720] __driver_attach+0xbd/0x140 >>>>> [ 1.378722] ? device_driver_attach+0x60/0x60 >>>>> [ 1.378724] bus_for_each_dev+0x67/0xc0 >>>>> [ 1.378727] ? klist_add_tail+0x57/0x70 >>>> >>>> I am not able to reproduce above call stack when virtio-blk is assigned 4 queues >>>> while my qemu cmdline is "-smp 16,sockets=1,cores=8,threads=2". >>>> >>>> # cat /sys/block/vda/mq/0/cpu_list >>>> 0, 1, 2, 3 >>>> # cat /sys/block/vda/mq/1/cpu_list >>>> 4, 5, 6, 7 >>>> # cat /sys/block/vda/mq/2/cpu_list >>>> 8, 9, 10, 11 >>>> # cat /sys/block/vda/mq/3/cpu_list >>>> 12, 13, 14, 15 >>>> >>>> >>>> I do agree in above case we would have issue if the mapping is established by >>>> blk_mq_map_queues(). >>>> >>>> >>>> However, I am just curious how we finally reach at blk_mq_map_queues() from >>>> blk_mq_alloc_tag_set()? >>>> >>>> It should be something like: >>>> >>>> blk_mq_alloc_tag_set() >>>> -> blk_mq_update_queue_map() >>>> -> if (set->ops->map_queues && !is_kdump_kernel()) >>>> return set->ops->map_queues(set); >>>> -> else >>>> return blk_mq_map_queues(&set->map[HCTX_TYPE_DEFAULT]); >>>> >>>> Wouldn't we always have set->ops->map_queues = virtblk_map_queues()? >>>> >>>> Or the execution reach at: >>>> >>>> virtblk_map_queues() >>>> -> blk_mq_virtio_map_queues() >>>> -> if (!vdev->config->get_vq_affinity) >>>> return blk_mq_map_queues(qmap); >>>> -> else >>>> establish the mapping via get_vq_affinity >>>> >>>> but vdev->config->get_vq_affinity == NULL? >>>> >>>> For virtio pci, get_vq_affinity is always set. Seems virtio mmio would not set >>>> get_vq_affinity. >>>> >>>> >>>> I used to play with firecracker (by amazon) and it is interesting firecracker >>>> uses mmio to setup virtio-blk. >>>> >>>> >>>> Sorry for disturbing the review of this patch. I just would like to clarify in >>>> which scenario we would hit this issue, e.g., when virtio-blk is based on mmio? >>>> >>>> Dongli Zhang >>>> >>>>> >>>>> >>>>> At 2019-03-22 19:58:08, "Dongli Zhang" <dongli.zhang@oracle.com> wrote: >>>>>> >>>>>> >>>>>> On 03/22/2019 06:09 PM, luferry wrote: >>>>>>> under virtual machine environment, cpu topology may differ from normal >>>>>>> physical server. >>>>>> >>>>>> Would mind share the name of virtual machine monitor, the command line if >>>>>> available, and which device to reproduce. >>>>>> >>>>>> For instance, I am not able to reproduce with qemu nvme or virtio-blk as I >>>>>> assume they use pci or virtio specific mapper to establish the mapping. >>>>>> >>>>>> E.g., with qemu and nvme: -smp 8,sockets=1,cores=4,threads=2 >>>>>> >>>>>> Indeed I use three queues instead of twp as one is reserved for admin. >>>>>> >>>>>> # ls /sys/block/nvme0n1/mq/* >>>>>> /sys/block/nvme0n1/mq/0: >>>>>> cpu0 cpu1 cpu2 cpu3 cpu_list nr_reserved_tags nr_tags >>>>>> >>>>>> /sys/block/nvme0n1/mq/1: >>>>>> cpu4 cpu5 cpu6 cpu7 cpu_list nr_reserved_tags nr_tags >>>>>> >>>>>> >>>>>> Thank you very much! >>>>>> >>>>>> Dongli Zhang >>>>>> >>>>>>> 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 >>>>>>> >>>>>>> When attach disk with two queues, all the even numbered cpus will be >>>>>>> mapped to queue 0. Under virtual machine, all the cpus is followed by >>>>>>> its sibling cpu.Before this patch, all the odd numbered cpus will also >>>>>>> be mapped to queue 0, can cause serious imbalance.this will lead to >>>>>>> performance impact on system IO >>>>>>> >>>>>>> So suggest to allocate cpu map by core id, this can be more currency >>>>>>> >>>>>>> Signed-off-by: luferry <luferry@163.com> >>>>>>> --- >>>>>>> block/blk-mq-cpumap.c | 9 +++++---- >>>>>>> 1 file changed, 5 insertions(+), 4 deletions(-) >>>>>>> >>>>>>> diff --git a/block/blk-mq-cpumap.c b/block/blk-mq-cpumap.c >>>>>>> index 03a534820271..4125e8e77679 100644 >>>>>>> --- a/block/blk-mq-cpumap.c >>>>>>> +++ b/block/blk-mq-cpumap.c >>>>>>> @@ -35,7 +35,7 @@ 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; >>>>>>> >>>>>>> for_each_possible_cpu(cpu) { >>>>>>> /* >>>>>>> @@ -48,9 +48,10 @@ int blk_mq_map_queues(struct blk_mq_queue_map *qmap) >>>>>>> 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]; >>>>>>> } >>>>>>> } >>>>>>> ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-03-25 15:25 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-03-22 10:09 [PATCH] block/mq: blk map queues by core id luferry 2019-03-22 11:53 ` Dongli Zhang 2019-03-22 11:58 ` Dongli Zhang 2019-03-23 6:34 ` luferry 2019-03-23 11:14 ` Dongli Zhang 2019-03-25 9:49 ` luferry 2019-03-25 9:53 ` luferry 2019-03-25 13:53 ` Dongli Zhang 2019-03-25 15:17 ` 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).