| Message ID | f936d176-c56b-143e-3311-f6df48f633dd@mellanox.com (mailing list archive) |
|---|---|
| State | RFC |
| Headers | show |
On Fri, Jul 20, 2018 at 04:25:32AM +0300, Max Gurtovoy wrote: > > >>>[ 2032.194376] nvme nvme0: failed to connect queue: 9 ret=-18 > >> > >>queue 9 is not mapped (overlap). > >>please try the bellow: > >> > > > >This seems to work. Here are three mapping cases: each vector on its > >own cpu, each vector on 1 cpu within the local numa node, and each > >vector having all cpus in its numa node. The 2nd mapping looks kinda > >funny, but I think it achieved what you wanted? And all the cases > >resulted in successful connections. > > > > Thanks for testing this. > I slightly improved the setting of the left CPUs and actually used Sagi's > initial proposal. > > Sagi, > please review the attached patch and let me know if I should add your > signature on it. > I'll run some perf test early next week on it (meanwhile I run login/logout > with different num_queues successfully and irq settings). > > Steve, > It will be great if you can apply the attached in your system and send your > findings. > > Regards, > Max, So the conlusion to this thread is that Leon's mlx5 patch needs to wait until this block-mq patch is accepted? Thanks, Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 7/23/2018 7:49 PM, Jason Gunthorpe wrote: > On Fri, Jul 20, 2018 at 04:25:32AM +0300, Max Gurtovoy wrote: >> >>>>> [ 2032.194376] nvme nvme0: failed to connect queue: 9 ret=-18 >>>> >>>> queue 9 is not mapped (overlap). >>>> please try the bellow: >>>> >>> >>> This seems to work. Here are three mapping cases: each vector on its >>> own cpu, each vector on 1 cpu within the local numa node, and each >>> vector having all cpus in its numa node. The 2nd mapping looks kinda >>> funny, but I think it achieved what you wanted? And all the cases >>> resulted in successful connections. >>> >> >> Thanks for testing this. >> I slightly improved the setting of the left CPUs and actually used Sagi's >> initial proposal. >> >> Sagi, >> please review the attached patch and let me know if I should add your >> signature on it. >> I'll run some perf test early next week on it (meanwhile I run login/logout >> with different num_queues successfully and irq settings). >> >> Steve, >> It will be great if you can apply the attached in your system and send your >> findings. >> >> Regards, >> Max, > > So the conlusion to this thread is that Leon's mlx5 patch needs to wait > until this block-mq patch is accepted? Yes, since nvmf is the only user of this function. Still waiting for comments on the suggested patch :) > > Thanks, > Jason > -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 7/19/2018 8:25 PM, Max Gurtovoy wrote: > >>>> [ 2032.194376] nvme nvme0: failed to connect queue: 9 ret=-18 >>> >>> queue 9 is not mapped (overlap). >>> please try the bellow: >>> >> >> This seems to work. Here are three mapping cases: each vector on its >> own cpu, each vector on 1 cpu within the local numa node, and each >> vector having all cpus in its numa node. The 2nd mapping looks kinda >> funny, but I think it achieved what you wanted? And all the cases >> resulted in successful connections. >> > > Thanks for testing this. > I slightly improved the setting of the left CPUs and actually used > Sagi's initial proposal. > > Sagi, > please review the attached patch and let me know if I should add your > signature on it. > I'll run some perf test early next week on it (meanwhile I run > login/logout with different num_queues successfully and irq settings). > > Steve, > It will be great if you can apply the attached in your system and send > your findings. Sorry, I got side tracked. I'll try and test this today and report back. Steve. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 7/24/2018 10:24 AM, Steve Wise wrote: > > On 7/19/2018 8:25 PM, Max Gurtovoy wrote: >>>>> [ 2032.194376] nvme nvme0: failed to connect queue: 9 ret=-18 >>>> queue 9 is not mapped (overlap). >>>> please try the bellow: >>>> >>> This seems to work. Here are three mapping cases: each vector on its >>> own cpu, each vector on 1 cpu within the local numa node, and each >>> vector having all cpus in its numa node. The 2nd mapping looks kinda >>> funny, but I think it achieved what you wanted? And all the cases >>> resulted in successful connections. >>> >> Thanks for testing this. >> I slightly improved the setting of the left CPUs and actually used >> Sagi's initial proposal. >> >> Sagi, >> please review the attached patch and let me know if I should add your >> signature on it. >> I'll run some perf test early next week on it (meanwhile I run >> login/logout with different num_queues successfully and irq settings). >> >> Steve, >> It will be great if you can apply the attached in your system and send >> your findings. > Sorry, I got side tracked. I'll try and test this today and report back. > > Steve. ### each vector gets a unique cpu, starting with node-local: [ 754.976577] iw_cxgb4: comp_vector 0, irq 203 mask 0x100 [ 754.982378] iw_cxgb4: comp_vector 1, irq 204 mask 0x200 [ 754.988167] iw_cxgb4: comp_vector 2, irq 205 mask 0x400 [ 754.993935] iw_cxgb4: comp_vector 3, irq 206 mask 0x800 [ 754.999686] iw_cxgb4: comp_vector 4, irq 207 mask 0x1000 [ 755.005509] iw_cxgb4: comp_vector 5, irq 208 mask 0x2000 [ 755.011318] iw_cxgb4: comp_vector 6, irq 209 mask 0x4000 [ 755.017124] iw_cxgb4: comp_vector 7, irq 210 mask 0x8000 [ 755.022915] iw_cxgb4: comp_vector 8, irq 211 mask 0x1 [ 755.028437] iw_cxgb4: comp_vector 9, irq 212 mask 0x2 [ 755.033948] iw_cxgb4: comp_vector 10, irq 213 mask 0x4 [ 755.039543] iw_cxgb4: comp_vector 11, irq 214 mask 0x8 [ 755.045135] iw_cxgb4: comp_vector 12, irq 215 mask 0x10 [ 755.050801] iw_cxgb4: comp_vector 13, irq 216 mask 0x20 [ 755.056464] iw_cxgb4: comp_vector 14, irq 217 mask 0x40 [ 755.062117] iw_cxgb4: comp_vector 15, irq 218 mask 0x80 [ 755.067767] blk_mq_rdma_map_queues: set->mq_map[0] queue 8 vector 8 [ 755.067767] blk_mq_rdma_map_queues: set->mq_map[1] queue 9 vector 9 [ 755.067768] blk_mq_rdma_map_queues: set->mq_map[2] queue 10 vector 10 [ 755.067769] blk_mq_rdma_map_queues: set->mq_map[3] queue 11 vector 11 [ 755.067769] blk_mq_rdma_map_queues: set->mq_map[4] queue 12 vector 12 [ 755.067770] blk_mq_rdma_map_queues: set->mq_map[5] queue 13 vector 13 [ 755.067771] blk_mq_rdma_map_queues: set->mq_map[6] queue 14 vector 14 [ 755.067772] blk_mq_rdma_map_queues: set->mq_map[7] queue 15 vector 15 [ 755.067772] blk_mq_rdma_map_queues: set->mq_map[8] queue 0 vector 0 [ 755.067773] blk_mq_rdma_map_queues: set->mq_map[9] queue 1 vector 1 [ 755.067774] blk_mq_rdma_map_queues: set->mq_map[10] queue 2 vector 2 [ 755.067774] blk_mq_rdma_map_queues: set->mq_map[11] queue 3 vector 3 [ 755.067775] blk_mq_rdma_map_queues: set->mq_map[12] queue 4 vector 4 [ 755.067775] blk_mq_rdma_map_queues: set->mq_map[13] queue 5 vector 5 [ 755.067776] blk_mq_rdma_map_queues: set->mq_map[14] queue 6 vector 6 [ 755.067777] blk_mq_rdma_map_queues: set->mq_map[15] queue 7 vector 7 ### each vector gets one cpu within the local node: [ 777.590913] iw_cxgb4: comp_vector 0, irq 203 mask 0x400 [ 777.596588] iw_cxgb4: comp_vector 1, irq 204 mask 0x800 [ 777.602249] iw_cxgb4: comp_vector 2, irq 205 mask 0x1000 [ 777.607984] iw_cxgb4: comp_vector 3, irq 206 mask 0x2000 [ 777.613708] iw_cxgb4: comp_vector 4, irq 207 mask 0x4000 [ 777.619431] iw_cxgb4: comp_vector 5, irq 208 mask 0x8000 [ 777.625142] iw_cxgb4: comp_vector 6, irq 209 mask 0x100 [ 777.630762] iw_cxgb4: comp_vector 7, irq 210 mask 0x200 [ 777.636373] iw_cxgb4: comp_vector 8, irq 211 mask 0x400 [ 777.641982] iw_cxgb4: comp_vector 9, irq 212 mask 0x800 [ 777.647583] iw_cxgb4: comp_vector 10, irq 213 mask 0x1000 [ 777.653353] iw_cxgb4: comp_vector 11, irq 214 mask 0x2000 [ 777.659119] iw_cxgb4: comp_vector 12, irq 215 mask 0x4000 [ 777.664877] iw_cxgb4: comp_vector 13, irq 216 mask 0x8000 [ 777.670628] iw_cxgb4: comp_vector 14, irq 217 mask 0x100 [ 777.676289] iw_cxgb4: comp_vector 15, irq 218 mask 0x200 [ 777.681946] blk_mq_rdma_map_queues: set->mq_map[0] queue 8 vector 8 [ 777.681947] blk_mq_rdma_map_queues: set->mq_map[1] queue 9 vector 9 [ 777.681947] blk_mq_rdma_map_queues: set->mq_map[2] queue 10 vector 10 [ 777.681948] blk_mq_rdma_map_queues: set->mq_map[3] queue 11 vector 11 [ 777.681948] blk_mq_rdma_map_queues: set->mq_map[4] queue 12 vector 12 [ 777.681949] blk_mq_rdma_map_queues: set->mq_map[5] queue 13 vector 13 [ 777.681950] blk_mq_rdma_map_queues: set->mq_map[6] queue 14 vector 14 [ 777.681950] blk_mq_rdma_map_queues: set->mq_map[7] queue 15 vector 15 [ 777.681951] blk_mq_rdma_map_queues: set->mq_map[8] queue 6 vector 6 [ 777.681952] blk_mq_rdma_map_queues: set->mq_map[9] queue 7 vector 7 [ 777.681952] blk_mq_rdma_map_queues: set->mq_map[10] queue 0 vector 0 [ 777.681953] blk_mq_rdma_map_queues: set->mq_map[11] queue 1 vector 1 [ 777.681953] blk_mq_rdma_map_queues: set->mq_map[12] queue 2 vector 2 [ 777.681954] blk_mq_rdma_map_queues: set->mq_map[13] queue 3 vector 3 [ 777.681955] blk_mq_rdma_map_queues: set->mq_map[14] queue 4 vector 4 [ 777.681955] blk_mq_rdma_map_queues: set->mq_map[15] queue 5 vector 5 ### each vector gets all cpus within the local node: [ 838.251643] iw_cxgb4: comp_vector 0, irq 203 mask 0xff00 [ 838.257346] iw_cxgb4: comp_vector 1, irq 204 mask 0xff00 [ 838.263038] iw_cxgb4: comp_vector 2, irq 205 mask 0xff00 [ 838.268710] iw_cxgb4: comp_vector 3, irq 206 mask 0xff00 [ 838.274351] iw_cxgb4: comp_vector 4, irq 207 mask 0xff00 [ 838.279985] iw_cxgb4: comp_vector 5, irq 208 mask 0xff00 [ 838.285610] iw_cxgb4: comp_vector 6, irq 209 mask 0xff00 [ 838.291234] iw_cxgb4: comp_vector 7, irq 210 mask 0xff00 [ 838.296865] iw_cxgb4: comp_vector 8, irq 211 mask 0xff00 [ 838.302484] iw_cxgb4: comp_vector 9, irq 212 mask 0xff00 [ 838.308109] iw_cxgb4: comp_vector 10, irq 213 mask 0xff00 [ 838.313827] iw_cxgb4: comp_vector 11, irq 214 mask 0xff00 [ 838.319539] iw_cxgb4: comp_vector 12, irq 215 mask 0xff00 [ 838.325250] iw_cxgb4: comp_vector 13, irq 216 mask 0xff00 [ 838.330963] iw_cxgb4: comp_vector 14, irq 217 mask 0xff00 [ 838.336674] iw_cxgb4: comp_vector 15, irq 218 mask 0xff00 [ 838.342385] blk_mq_rdma_map_queues: set->mq_map[0] queue 8 vector 8 [ 838.342385] blk_mq_rdma_map_queues: set->mq_map[1] queue 9 vector 9 [ 838.342386] blk_mq_rdma_map_queues: set->mq_map[2] queue 10 vector 10 [ 838.342387] blk_mq_rdma_map_queues: set->mq_map[3] queue 11 vector 11 [ 838.342387] blk_mq_rdma_map_queues: set->mq_map[4] queue 12 vector 12 [ 838.342388] blk_mq_rdma_map_queues: set->mq_map[5] queue 13 vector 13 [ 838.342389] blk_mq_rdma_map_queues: set->mq_map[6] queue 14 vector 14 [ 838.342390] blk_mq_rdma_map_queues: set->mq_map[7] queue 15 vector 15 [ 838.342391] blk_mq_rdma_map_queues: set->mq_map[8] queue 0 vector 0 [ 838.342391] blk_mq_rdma_map_queues: set->mq_map[9] queue 1 vector 1 [ 838.342392] blk_mq_rdma_map_queues: set->mq_map[10] queue 2 vector 2 [ 838.342392] blk_mq_rdma_map_queues: set->mq_map[11] queue 3 vector 3 [ 838.342393] blk_mq_rdma_map_queues: set->mq_map[12] queue 4 vector 4 [ 838.342394] blk_mq_rdma_map_queues: set->mq_map[13] queue 5 vector 5 [ 838.342394] blk_mq_rdma_map_queues: set->mq_map[14] queue 6 vector 6 [ 838.342395] blk_mq_rdma_map_queues: set->mq_map[15] queue 7 vector 7 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 7/23/2018 11:53 AM, Max Gurtovoy wrote: > > > On 7/23/2018 7:49 PM, Jason Gunthorpe wrote: >> On Fri, Jul 20, 2018 at 04:25:32AM +0300, Max Gurtovoy wrote: >>> >>>>>> [ 2032.194376] nvme nvme0: failed to connect queue: 9 ret=-18 >>>>> >>>>> queue 9 is not mapped (overlap). >>>>> please try the bellow: >>>>> >>>> >>>> This seems to work. Here are three mapping cases: each vector on its >>>> own cpu, each vector on 1 cpu within the local numa node, and each >>>> vector having all cpus in its numa node. The 2nd mapping looks kinda >>>> funny, but I think it achieved what you wanted? And all the cases >>>> resulted in successful connections. >>>> >>> >>> Thanks for testing this. >>> I slightly improved the setting of the left CPUs and actually used >>> Sagi's >>> initial proposal. >>> >>> Sagi, >>> please review the attached patch and let me know if I should add your >>> signature on it. >>> I'll run some perf test early next week on it (meanwhile I run >>> login/logout >>> with different num_queues successfully and irq settings). >>> >>> Steve, >>> It will be great if you can apply the attached in your system and >>> send your >>> findings. >>> >>> Regards, >>> Max, >> >> So the conlusion to this thread is that Leon's mlx5 patch needs to wait >> until this block-mq patch is accepted? > > Yes, since nvmf is the only user of this function. > Still waiting for comments on the suggested patch :) Hey Sagi, what do you think of Max's patch? Max, should you resend this in a form suitable for merging? Thanks, Steve. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 7/30/2018 6:47 PM, Steve Wise wrote: > > > On 7/23/2018 11:53 AM, Max Gurtovoy wrote: >> >> >> On 7/23/2018 7:49 PM, Jason Gunthorpe wrote: >>> On Fri, Jul 20, 2018 at 04:25:32AM +0300, Max Gurtovoy wrote: >>>> >>>>>>> [ 2032.194376] nvme nvme0: failed to connect queue: 9 ret=-18 >>>>>> >>>>>> queue 9 is not mapped (overlap). >>>>>> please try the bellow: >>>>>> >>>>> >>>>> This seems to work. Here are three mapping cases: each vector on its >>>>> own cpu, each vector on 1 cpu within the local numa node, and each >>>>> vector having all cpus in its numa node. The 2nd mapping looks kinda >>>>> funny, but I think it achieved what you wanted? And all the cases >>>>> resulted in successful connections. >>>>> >>>> >>>> Thanks for testing this. >>>> I slightly improved the setting of the left CPUs and actually used >>>> Sagi's >>>> initial proposal. >>>> >>>> Sagi, >>>> please review the attached patch and let me know if I should add your >>>> signature on it. >>>> I'll run some perf test early next week on it (meanwhile I run >>>> login/logout >>>> with different num_queues successfully and irq settings). >>>> >>>> Steve, >>>> It will be great if you can apply the attached in your system and >>>> send your >>>> findings. >>>> >>>> Regards, >>>> Max, >>> >>> So the conlusion to this thread is that Leon's mlx5 patch needs to wait >>> until this block-mq patch is accepted? >> >> Yes, since nvmf is the only user of this function. >> Still waiting for comments on the suggested patch :) > > Hey Sagi, what do you think of Max's patch? > > Max, should you resend this in a form suitable for merging? Yes, we have already a small improvment of the naive step. but first I want to see some feedback from other maintainers as well. > > Thanks, > > Steve. > -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Max, > Yes, since nvmf is the only user of this function. > Still waiting for comments on the suggested patch :) > Sorry for the late response (but I'm on vacation so I have an excuse ;)) I'm thinking that we should avoid trying to find an assignment when stuff like irqbalance daemon is running and changing the affinitization. This extension was made to apply optimal affinity assignment when the device irq affinity is lined up in a vector per core. I'm thinking that when we identify this is not the case, we immediately fallback to the default mapping. 1. when we get a mask, if its weight != 1, we fallback. 2. if a queue was left unmapped, we fallback. Maybe something like the following: -- diff --git a/block/blk-mq-rdma.c b/block/blk-mq-rdma.c index 996167f1de18..1ada6211c55e 100644 --- a/block/blk-mq-rdma.c +++ b/block/blk-mq-rdma.c @@ -35,17 +35,26 @@ int blk_mq_rdma_map_queues(struct blk_mq_tag_set *set, const struct cpumask *mask; unsigned int queue, cpu; + /* reset all CPUs mapping */ + for_each_possible_cpu(cpu) + set->mq_map[cpu] = UINT_MAX; + for (queue = 0; queue < set->nr_hw_queues; queue++) { mask = ib_get_vector_affinity(dev, first_vec + queue); if (!mask) goto fallback; - for_each_cpu(cpu, mask) - set->mq_map[cpu] = queue; + if (cpumask_weight(mask) != 1) + goto fallback; + + cpu = cpumask_first(mask); + if (set->mq_map[cpu] != UINT_MAX) + goto fallback; + + set->mq_map[cpu] = queue; } return 0; - fallback: return blk_mq_map_queues(set); } -- -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 8/1/2018 8:12 AM, Sagi Grimberg wrote: > Hi Max, Hi, > >> Yes, since nvmf is the only user of this function. >> Still waiting for comments on the suggested patch :) >> > > Sorry for the late response (but I'm on vacation so I have > an excuse ;)) NP :) currently the code works.. > > I'm thinking that we should avoid trying to find an assignment > when stuff like irqbalance daemon is running and changing > the affinitization. but this is exactly what Steve complained and Leon try to fix (and break the connection establishment). If this is the case and we all agree then we're good without Leon's patch and without our suggestions. > > This extension was made to apply optimal affinity assignment > when the device irq affinity is lined up in a vector per > core. > > I'm thinking that when we identify this is not the case, we immediately > fallback to the default mapping. > > 1. when we get a mask, if its weight != 1, we fallback. > 2. if a queue was left unmapped, we fallback. > > Maybe something like the following: did you test it ? I think it will not work since you need to map all the queues and all the CPUs. > -- > diff --git a/block/blk-mq-rdma.c b/block/blk-mq-rdma.c > index 996167f1de18..1ada6211c55e 100644 > --- a/block/blk-mq-rdma.c > +++ b/block/blk-mq-rdma.c > @@ -35,17 +35,26 @@ int blk_mq_rdma_map_queues(struct blk_mq_tag_set *set, > const struct cpumask *mask; > unsigned int queue, cpu; > > + /* reset all CPUs mapping */ > + for_each_possible_cpu(cpu) > + set->mq_map[cpu] = UINT_MAX; > + > for (queue = 0; queue < set->nr_hw_queues; queue++) { > mask = ib_get_vector_affinity(dev, first_vec + queue); > if (!mask) > goto fallback; > > - for_each_cpu(cpu, mask) > - set->mq_map[cpu] = queue; > + if (cpumask_weight(mask) != 1) > + goto fallback; > + > + cpu = cpumask_first(mask); > + if (set->mq_map[cpu] != UINT_MAX) > + goto fallback; > + > + set->mq_map[cpu] = queue; > } > > return 0; > - > fallback: > return blk_mq_map_queues(set); > } > -- see attached another algorithem that can improve the mapping (although it's not a short one)... it will try to map according to affinity mask, and also in case the mask weight > 1 it will try to be better than the naive mapping I suggest in the previous email. From 007d773af7b65a1f1ca543f031ca58b3afa5b7d9 Mon Sep 17 00:00:00 2001 From: Max Gurtovoy <maxg@mellanox.com> Date: Thu, 19 Jul 2018 12:42:00 +0000 Subject: [PATCH 1/1] blk-mq: fix RDMA queue/cpu mappings assignments for mq Signed-off-by: Max Gurtovoy <maxg@mellanox.com> Signed-off-by: Israel Rukshin <israelr@mellanox.com> --- block/blk-mq-cpumap.c | 41 ++++++++++++++---------- block/blk-mq-rdma.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++---- include/linux/blk-mq.h | 1 + 3 files changed, 103 insertions(+), 23 deletions(-) diff --git a/block/blk-mq-cpumap.c b/block/blk-mq-cpumap.c index 3eb169f..02b888f 100644 --- a/block/blk-mq-cpumap.c +++ b/block/blk-mq-cpumap.c @@ -30,29 +30,36 @@ static int get_first_sibling(unsigned int cpu) return cpu; } -int blk_mq_map_queues(struct blk_mq_tag_set *set) +void blk_mq_map_queue_to_cpu(struct blk_mq_tag_set *set, unsigned int cpu) { unsigned int *map = set->mq_map; unsigned int nr_queues = set->nr_hw_queues; - unsigned int cpu, first_sibling; + unsigned int first_sibling; - 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 - * performace optimizations. - */ - if (cpu < nr_queues) { + /* + * 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 + * performace optimizations. + */ + if (cpu < nr_queues) { + map[cpu] = cpu_to_queue_index(nr_queues, cpu); + } else { + first_sibling = get_first_sibling(cpu); + if (first_sibling == cpu) map[cpu] = cpu_to_queue_index(nr_queues, cpu); - } else { - first_sibling = get_first_sibling(cpu); - if (first_sibling == cpu) - map[cpu] = cpu_to_queue_index(nr_queues, cpu); - else - map[cpu] = map[first_sibling]; - } + else + map[cpu] = map[first_sibling]; } +} +EXPORT_SYMBOL_GPL(blk_mq_map_queue_to_cpu); + +int blk_mq_map_queues(struct blk_mq_tag_set *set) +{ + unsigned int cpu; + + for_each_possible_cpu(cpu) + blk_mq_map_queue_to_cpu(set, cpu); return 0; } diff --git a/block/blk-mq-rdma.c b/block/blk-mq-rdma.c index 996167f..621d5f0 100644 --- a/block/blk-mq-rdma.c +++ b/block/blk-mq-rdma.c @@ -14,6 +14,33 @@ #include <linux/blk-mq-rdma.h> #include <rdma/ib_verbs.h> +static int blk_mq_rdma_map_queues_by_affinity(struct blk_mq_tag_set *set, + struct ib_device *dev, int first_vec, int max_mapping) +{ + const struct cpumask *mask; + unsigned int queue, cpu; + int num_of_mapping = 0; + + for (queue = 0; queue < set->nr_hw_queues; queue++) { + if (num_of_mapping == max_mapping) + return num_of_mapping; + + mask = ib_get_vector_affinity(dev, first_vec + queue); + if (!mask) + return -1; + + for_each_cpu(cpu, mask) { + if (set->mq_map[cpu] == UINT_MAX) { + set->mq_map[cpu] = queue; + num_of_mapping++; + /* Each queue mapped to 1 cpu */ + break; + } + } + } + return num_of_mapping; +} + /** * blk_mq_rdma_map_queues - provide a default queue mapping for rdma device * @set: tagset to provide the mapping for @@ -32,18 +59,63 @@ int blk_mq_rdma_map_queues(struct blk_mq_tag_set *set, struct ib_device *dev, int first_vec) { - const struct cpumask *mask; unsigned int queue, cpu; + bool mapped; + int remaining_cpus = num_possible_cpus(); + int unmapped_queues = set->nr_hw_queues; + int ret; - for (queue = 0; queue < set->nr_hw_queues; queue++) { - mask = ib_get_vector_affinity(dev, first_vec + queue); - if (!mask) + /* reset all CPUs mapping */ + for_each_possible_cpu(cpu) + set->mq_map[cpu] = UINT_MAX; + + /* Try to map the queues according to affinity */ + ret = blk_mq_rdma_map_queues_by_affinity(set, dev, first_vec, + remaining_cpus); + if (ret == -1) + goto fallback; + remaining_cpus -= ret; + unmapped_queues -= ret; + + /* Map queues with more than one cpu according to affinity */ + while (remaining_cpus > unmapped_queues) { + ret = blk_mq_rdma_map_queues_by_affinity(set, dev, first_vec, + remaining_cpus - unmapped_queues); + if (ret == -1) goto fallback; + if (!ret) + break; + remaining_cpus -= ret; + } - for_each_cpu(cpu, mask) - set->mq_map[cpu] = queue; + /* Map the unmapped queues in a naive way */ + for (queue = 0; queue < set->nr_hw_queues; queue++) { + mapped = false; + for_each_possible_cpu(cpu) { + if (set->mq_map[cpu] == queue) { + mapped = true; + break; + } + } + if (!mapped) { + for_each_possible_cpu(cpu) { + if (set->mq_map[cpu] == UINT_MAX) { + set->mq_map[cpu] = queue; + mapped = true; + break; + } + } + } + /* This case should never happen */ + if (WARN_ON_ONCE(!mapped)) + goto fallback; } + /* set all the rest of the CPUs */ + for_each_possible_cpu(cpu) + if (set->mq_map[cpu] == UINT_MAX) + blk_mq_map_queue_to_cpu(set, cpu); + return 0; fallback: diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h index e3147eb..d6cd114 100644 --- a/include/linux/blk-mq.h +++ b/include/linux/blk-mq.h @@ -282,6 +282,7 @@ void blk_mq_tagset_busy_iter(struct blk_mq_tag_set *tagset, int blk_mq_freeze_queue_wait_timeout(struct request_queue *q, unsigned long timeout); +void blk_mq_map_queue_to_cpu(struct blk_mq_tag_set *set, unsigned int cpu); int blk_mq_map_queues(struct blk_mq_tag_set *set); void blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set, int nr_hw_queues);
On 8/1/2018 9:27 AM, Max Gurtovoy wrote: > > > On 8/1/2018 8:12 AM, Sagi Grimberg wrote: >> Hi Max, > > Hi, > >> >>> Yes, since nvmf is the only user of this function. >>> Still waiting for comments on the suggested patch :) >>> >> >> Sorry for the late response (but I'm on vacation so I have >> an excuse ;)) > > NP :) currently the code works.. > >> >> I'm thinking that we should avoid trying to find an assignment >> when stuff like irqbalance daemon is running and changing >> the affinitization. > > but this is exactly what Steve complained and Leon try to fix (and > break the connection establishment). > If this is the case and we all agree then we're good without Leon's > patch and without our suggestions. > I don't agree. Currently setting certain affinity mappings breaks nvme connectivity. I don't think that is desirable. And mlx5 is broken in that it doesn't allow changing the affinity but silently ignores the change, which misleads the admin or irqbalance... >> >> This extension was made to apply optimal affinity assignment >> when the device irq affinity is lined up in a vector per >> core. >> >> I'm thinking that when we identify this is not the case, we immediately >> fallback to the default mapping. >> >> 1. when we get a mask, if its weight != 1, we fallback. >> 2. if a queue was left unmapped, we fallback. >> >> Maybe something like the following: > > did you test it ? I think it will not work since you need to map all > the queues and all the CPUs. > >> -- >> diff --git a/block/blk-mq-rdma.c b/block/blk-mq-rdma.c >> index 996167f1de18..1ada6211c55e 100644 >> --- a/block/blk-mq-rdma.c >> +++ b/block/blk-mq-rdma.c >> @@ -35,17 +35,26 @@ int blk_mq_rdma_map_queues(struct blk_mq_tag_set >> *set, >> const struct cpumask *mask; >> unsigned int queue, cpu; >> >> + /* reset all CPUs mapping */ >> + for_each_possible_cpu(cpu) >> + set->mq_map[cpu] = UINT_MAX; >> + >> for (queue = 0; queue < set->nr_hw_queues; queue++) { >> mask = ib_get_vector_affinity(dev, first_vec + queue); >> if (!mask) >> goto fallback; >> >> - for_each_cpu(cpu, mask) >> - set->mq_map[cpu] = queue; >> + if (cpumask_weight(mask) != 1) >> + goto fallback; >> + >> + cpu = cpumask_first(mask); >> + if (set->mq_map[cpu] != UINT_MAX) >> + goto fallback; >> + >> + set->mq_map[cpu] = queue; >> } >> >> return 0; >> - >> fallback: >> return blk_mq_map_queues(set); >> } >> -- > > see attached another algorithem that can improve the mapping (although > it's not a short one)... > > it will try to map according to affinity mask, and also in case the > mask weight > 1 it will try to be better than the naive mapping I > suggest in the previous email. > Let me know if you want me to try this or any particular fix. Steve. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Aug 06, 2018 at 02:20:37PM -0500, Steve Wise wrote: > > > On 8/1/2018 9:27 AM, Max Gurtovoy wrote: > > > > > > On 8/1/2018 8:12 AM, Sagi Grimberg wrote: > >> Hi Max, > > > > Hi, > > > >> > >>> Yes, since nvmf is the only user of this function. > >>> Still waiting for comments on the suggested patch :) > >>> > >> > >> Sorry for the late response (but I'm on vacation so I have > >> an excuse ;)) > > > > NP :) currently the code works.. > > > >> > >> I'm thinking that we should avoid trying to find an assignment > >> when stuff like irqbalance daemon is running and changing > >> the affinitization. > > > > but this is exactly what Steve complained and Leon try to fix (and > > break the connection establishment). > > If this is the case and we all agree then we're good without Leon's > > patch and without our suggestions. > > > > I don't agree. Currently setting certain affinity mappings breaks nvme > connectivity. I don't think that is desirable. And mlx5 is broken in > that it doesn't allow changing the affinity but silently ignores the > change, which misleads the admin or irqbalance... Exactly, I completely agree with Steve and don't understand any rationale in the comments above. As a summery from my side: NVMeOF is broken, but we are not going to fix and prohibit from one specific driver to change affinity on the fly. Nice. Thanks
> Let me know if you want me to try this or any particular fix. Steve, can you test this one? -- [PATCH rfc] block: fix rdma queue mapping nvme-rdma attempts to map queues based on irq vector affinity. However, for some devices, completion vector irq affinity is configurable by the user which can break the existing assumption that irq vectors are optimally arranged over the host cpu cores. So we map queues in two stages: First map queues according to corresponding to the completion vector IRQ affinity taking the first cpu in the vector affinity map. if the current irq affinity is arranged such that a vector is not assigned to any distinct cpu, we map it to a cpu that is on the same node. If numa affinity can not be sufficed, we map it to any unmapped cpu we can find. Then, map the remaining cpus in the possible cpumap naively. Signed-off-by: Sagi Grimberg <sagi@grimberg.me> --- Steve, can you test out this patch? block/blk-mq-cpumap.c | 39 +++++++++++++----------- block/blk-mq-rdma.c | 80 +++++++++++++++++++++++++++++++++++++++++++------- include/linux/blk-mq.h | 1 + 3 files changed, 93 insertions(+), 27 deletions(-) diff --git a/block/blk-mq-cpumap.c b/block/blk-mq-cpumap.c index 3eb169f15842..34811db8cba9 100644 --- a/block/blk-mq-cpumap.c +++ b/block/blk-mq-cpumap.c @@ -30,30 +30,35 @@ static int get_first_sibling(unsigned int cpu) return cpu; } -int blk_mq_map_queues(struct blk_mq_tag_set *set) +void blk_mq_map_queue_cpu(struct blk_mq_tag_set *set, unsigned int cpu) { unsigned int *map = set->mq_map; unsigned int nr_queues = set->nr_hw_queues; - unsigned int cpu, first_sibling; + unsigned int first_sibling; - 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 - * performace optimizations. - */ - if (cpu < nr_queues) { + /* + * 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 + * performace optimizations. + */ + if (cpu < nr_queues) { + map[cpu] = cpu_to_queue_index(nr_queues, cpu); + } else { + first_sibling = get_first_sibling(cpu); + if (first_sibling == cpu) map[cpu] = cpu_to_queue_index(nr_queues, cpu); - } else { - first_sibling = get_first_sibling(cpu); - if (first_sibling == cpu) - map[cpu] = cpu_to_queue_index(nr_queues, cpu); - else - map[cpu] = map[first_sibling]; - } + else + map[cpu] = map[first_sibling]; } +} + +int blk_mq_map_queues(struct blk_mq_tag_set *set) +{ + unsigned int cpu; + for_each_possible_cpu(cpu) + blk_mq_map_queue_cpu(set, cpu); return 0; } EXPORT_SYMBOL_GPL(blk_mq_map_queues); diff --git a/block/blk-mq-rdma.c b/block/blk-mq-rdma.c index 996167f1de18..d04cbb1925f5 100644 --- a/block/blk-mq-rdma.c +++ b/block/blk-mq-rdma.c @@ -14,6 +14,61 @@ #include <linux/blk-mq-rdma.h> #include <rdma/ib_verbs.h> +static int blk_mq_rdma_map_queue(struct blk_mq_tag_set *set, + struct ib_device *dev, int first_vec, unsigned int queue) +{ + const struct cpumask *mask; + unsigned int cpu; + bool mapped = false; + + mask = ib_get_vector_affinity(dev, first_vec + queue); + if (!mask) + return -ENOTSUPP; + + /* map with an unmapped cpu according to affinity mask */ + for_each_cpu(cpu, mask) { + if (set->mq_map[cpu] == UINT_MAX) { + set->mq_map[cpu] = queue; + mapped = true; + break; + } + } + + if (!mapped) { + int n; + + /* map with an unmapped cpu in the same numa node */ + for_each_node(n) { + const struct cpumask *node_cpumask = cpumask_of_node(n); + + if (!cpumask_intersects(mask, node_cpumask)) + continue; + + for_each_cpu(cpu, node_cpumask) { + if (set->mq_map[cpu] == UINT_MAX) { + set->mq_map[cpu] = queue; + mapped = true; + break; + } + } + } + } + + if (!mapped) { + /* map with any unmapped cpu we can find */ + for_each_possible_cpu(cpu) { + if (set->mq_map[cpu] == UINT_MAX) { + set->mq_map[cpu] = queue; + mapped = true; + break; + } + } + } + + WARN_ON_ONCE(!mapped); + return 0; +} + /** * blk_mq_rdma_map_queues - provide a default queue mapping for rdma device * @set: tagset to provide the mapping for @@ -21,31 +76,36 @@ * @first_vec: first interrupt vectors to use for queues (usually 0) * * This function assumes the rdma device @dev has at least as many available - * interrupt vetors as @set has queues. It will then query it's affinity mask - * and built queue mapping that maps a queue to the CPUs that have irq affinity - * for the corresponding vector. + * interrupt vetors as @set has queues. It will then query vector affinity mask + * and attempt to build irq affinity aware queue mappings. If optimal affinity + * aware mapping cannot be acheived for a given queue, we look for any unmapped + * cpu to map it. Lastly, we map naively all other unmapped cpus in the mq_map. * * In case either the driver passed a @dev with less vectors than * @set->nr_hw_queues, or @dev does not provide an affinity mask for a * vector, we fallback to the naive mapping. */ int blk_mq_rdma_map_queues(struct blk_mq_tag_set *set, - struct ib_device *dev, int first_vec) + struct ib_device *dev, int first_vec) { - const struct cpumask *mask; unsigned int queue, cpu; + /* reset cpu mapping */ + for_each_possible_cpu(cpu) + set->mq_map[cpu] = UINT_MAX; + for (queue = 0; queue < set->nr_hw_queues; queue++) { - mask = ib_get_vector_affinity(dev, first_vec + queue); - if (!mask) + if (blk_mq_rdma_map_queue(set, dev, first_vec, queue)) goto fallback; + } - for_each_cpu(cpu, mask) - set->mq_map[cpu] = queue; + /* map any remaining unmapped cpus */ + for_each_possible_cpu(cpu) { + if (set->mq_map[cpu] == UINT_MAX) + blk_mq_map_queue_cpu(set, cpu);; } return 0; - fallback: return blk_mq_map_queues(set); } diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h index d710e92874cc..6eb09c4de34f 100644 --- a/include/linux/blk-mq.h +++ b/include/linux/blk-mq.h @@ -285,6 +285,7 @@ int blk_mq_freeze_queue_wait_timeout(struct request_queue *q, unsigned long timeout); int blk_mq_map_queues(struct blk_mq_tag_set *set); +void blk_mq_map_queue_cpu(struct blk_mq_tag_set *set, unsigned int cpu); void blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set, int nr_hw_queues); void blk_mq_quiesce_queue_nowait(struct request_queue *q); --
On 8/16/2018 1:26 PM, Sagi Grimberg wrote: > >> Let me know if you want me to try this or any particular fix. > > Steve, can you test this one? Yes! I'll try it out tomorrow. Stevo > -- > [PATCH rfc] block: fix rdma queue mapping > > nvme-rdma attempts to map queues based on irq vector affinity. > However, for some devices, completion vector irq affinity is > configurable by the user which can break the existing assumption > that irq vectors are optimally arranged over the host cpu cores. > > So we map queues in two stages: > First map queues according to corresponding to the completion > vector IRQ affinity taking the first cpu in the vector affinity map. > if the current irq affinity is arranged such that a vector is not > assigned to any distinct cpu, we map it to a cpu that is on the same > node. If numa affinity can not be sufficed, we map it to any unmapped > cpu we can find. Then, map the remaining cpus in the possible cpumap > naively. > > Signed-off-by: Sagi Grimberg <sagi@grimberg.me> > --- > Steve, can you test out this patch? > block/blk-mq-cpumap.c | 39 +++++++++++++----------- > block/blk-mq-rdma.c | 80 > +++++++++++++++++++++++++++++++++++++++++++------- > include/linux/blk-mq.h | 1 + > 3 files changed, 93 insertions(+), 27 deletions(-) > > diff --git a/block/blk-mq-cpumap.c b/block/blk-mq-cpumap.c > index 3eb169f15842..34811db8cba9 100644 > --- a/block/blk-mq-cpumap.c > +++ b/block/blk-mq-cpumap.c > @@ -30,30 +30,35 @@ static int get_first_sibling(unsigned int cpu) > return cpu; > } > > -int blk_mq_map_queues(struct blk_mq_tag_set *set) > +void blk_mq_map_queue_cpu(struct blk_mq_tag_set *set, unsigned int cpu) > { > unsigned int *map = set->mq_map; > unsigned int nr_queues = set->nr_hw_queues; > - unsigned int cpu, first_sibling; > + unsigned int first_sibling; > > - 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 > - * performace optimizations. > - */ > - if (cpu < nr_queues) { > + /* > + * 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 > + * performace optimizations. > + */ > + if (cpu < nr_queues) { > + map[cpu] = cpu_to_queue_index(nr_queues, cpu); > + } else { > + first_sibling = get_first_sibling(cpu); > + if (first_sibling == cpu) > map[cpu] = cpu_to_queue_index(nr_queues, cpu); > - } else { > - first_sibling = get_first_sibling(cpu); > - if (first_sibling == cpu) > - map[cpu] = > cpu_to_queue_index(nr_queues, cpu); > - else > - map[cpu] = map[first_sibling]; > - } > + else > + map[cpu] = map[first_sibling]; > } > +} > + > +int blk_mq_map_queues(struct blk_mq_tag_set *set) > +{ > + unsigned int cpu; > > + for_each_possible_cpu(cpu) > + blk_mq_map_queue_cpu(set, cpu); > return 0; > } > EXPORT_SYMBOL_GPL(blk_mq_map_queues); > diff --git a/block/blk-mq-rdma.c b/block/blk-mq-rdma.c > index 996167f1de18..d04cbb1925f5 100644 > --- a/block/blk-mq-rdma.c > +++ b/block/blk-mq-rdma.c > @@ -14,6 +14,61 @@ > #include <linux/blk-mq-rdma.h> > #include <rdma/ib_verbs.h> > > +static int blk_mq_rdma_map_queue(struct blk_mq_tag_set *set, > + struct ib_device *dev, int first_vec, unsigned int queue) > +{ > + const struct cpumask *mask; > + unsigned int cpu; > + bool mapped = false; > + > + mask = ib_get_vector_affinity(dev, first_vec + queue); > + if (!mask) > + return -ENOTSUPP; > + > + /* map with an unmapped cpu according to affinity mask */ > + for_each_cpu(cpu, mask) { > + if (set->mq_map[cpu] == UINT_MAX) { > + set->mq_map[cpu] = queue; > + mapped = true; > + break; > + } > + } > + > + if (!mapped) { > + int n; > + > + /* map with an unmapped cpu in the same numa node */ > + for_each_node(n) { > + const struct cpumask *node_cpumask = > cpumask_of_node(n); > + > + if (!cpumask_intersects(mask, node_cpumask)) > + continue; > + > + for_each_cpu(cpu, node_cpumask) { > + if (set->mq_map[cpu] == UINT_MAX) { > + set->mq_map[cpu] = queue; > + mapped = true; > + break; > + } > + } > + } > + } > + > + if (!mapped) { > + /* map with any unmapped cpu we can find */ > + for_each_possible_cpu(cpu) { > + if (set->mq_map[cpu] == UINT_MAX) { > + set->mq_map[cpu] = queue; > + mapped = true; > + break; > + } > + } > + } > + > + WARN_ON_ONCE(!mapped); > + return 0; > +} > + > /** > * blk_mq_rdma_map_queues - provide a default queue mapping for rdma > device > * @set: tagset to provide the mapping for > @@ -21,31 +76,36 @@ > * @first_vec: first interrupt vectors to use for queues (usually 0) > * > * This function assumes the rdma device @dev has at least as many > available > - * interrupt vetors as @set has queues. It will then query it's > affinity mask > - * and built queue mapping that maps a queue to the CPUs that have > irq affinity > - * for the corresponding vector. > + * interrupt vetors as @set has queues. It will then query vector > affinity mask > + * and attempt to build irq affinity aware queue mappings. If optimal > affinity > + * aware mapping cannot be acheived for a given queue, we look for > any unmapped > + * cpu to map it. Lastly, we map naively all other unmapped cpus in > the mq_map. > * > * In case either the driver passed a @dev with less vectors than > * @set->nr_hw_queues, or @dev does not provide an affinity mask for a > * vector, we fallback to the naive mapping. > */ > int blk_mq_rdma_map_queues(struct blk_mq_tag_set *set, > - struct ib_device *dev, int first_vec) > + struct ib_device *dev, int first_vec) > { > - const struct cpumask *mask; > unsigned int queue, cpu; > > + /* reset cpu mapping */ > + for_each_possible_cpu(cpu) > + set->mq_map[cpu] = UINT_MAX; > + > for (queue = 0; queue < set->nr_hw_queues; queue++) { > - mask = ib_get_vector_affinity(dev, first_vec + queue); > - if (!mask) > + if (blk_mq_rdma_map_queue(set, dev, first_vec, queue)) > goto fallback; > + } > > - for_each_cpu(cpu, mask) > - set->mq_map[cpu] = queue; > + /* map any remaining unmapped cpus */ > + for_each_possible_cpu(cpu) { > + if (set->mq_map[cpu] == UINT_MAX) > + blk_mq_map_queue_cpu(set, cpu);; > } > > return 0; > - > fallback: > return blk_mq_map_queues(set); > } > diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h > index d710e92874cc..6eb09c4de34f 100644 > --- a/include/linux/blk-mq.h > +++ b/include/linux/blk-mq.h > @@ -285,6 +285,7 @@ int blk_mq_freeze_queue_wait_timeout(struct > request_queue *q, > unsigned long timeout); > > int blk_mq_map_queues(struct blk_mq_tag_set *set); > +void blk_mq_map_queue_cpu(struct blk_mq_tag_set *set, unsigned int cpu); > void blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set, int > nr_hw_queues); > > void blk_mq_quiesce_queue_nowait(struct request_queue *q); > --
> On 8/16/2018 1:26 PM, Sagi Grimberg wrote: > > > >> Let me know if you want me to try this or any particular fix. > > > > Steve, can you test this one? > > Yes! I'll try it out tomorrow. > > Stevo > Hey Sagi, The patch works allowing connections for the various affinity mappings below: One comp_vector per core across all cores, starting with numa-local cores: [ 3798.494963] iw_cxgb4: comp_vector 0, irq 203 mask 0x100 [ 3798.500717] iw_cxgb4: comp_vector 1, irq 204 mask 0x200 [ 3798.506396] iw_cxgb4: comp_vector 2, irq 205 mask 0x400 [ 3798.512043] iw_cxgb4: comp_vector 3, irq 206 mask 0x800 [ 3798.517675] iw_cxgb4: comp_vector 4, irq 207 mask 0x1000 [ 3798.523382] iw_cxgb4: comp_vector 5, irq 208 mask 0x2000 [ 3798.529075] iw_cxgb4: comp_vector 6, irq 209 mask 0x4000 [ 3798.534754] iw_cxgb4: comp_vector 7, irq 210 mask 0x8000 [ 3798.540425] iw_cxgb4: comp_vector 8, irq 211 mask 0x1 [ 3798.545825] iw_cxgb4: comp_vector 9, irq 212 mask 0x2 [ 3798.551231] iw_cxgb4: comp_vector 10, irq 213 mask 0x4 [ 3798.556713] iw_cxgb4: comp_vector 11, irq 214 mask 0x8 [ 3798.562189] iw_cxgb4: comp_vector 12, irq 215 mask 0x10 [ 3798.567755] iw_cxgb4: comp_vector 13, irq 216 mask 0x20 [ 3798.573312] iw_cxgb4: comp_vector 14, irq 217 mask 0x40 [ 3798.578855] iw_cxgb4: comp_vector 15, irq 218 mask 0x80 [ 3798.584384] set->mq_map[0] queue 8 vector 8 [ 3798.588879] set->mq_map[1] queue 9 vector 9 [ 3798.593358] set->mq_map[2] queue 10 vector 10 [ 3798.598008] set->mq_map[3] queue 11 vector 11 [ 3798.602633] set->mq_map[4] queue 12 vector 12 [ 3798.607260] set->mq_map[5] queue 13 vector 13 [ 3798.611872] set->mq_map[6] queue 14 vector 14 [ 3798.616470] set->mq_map[7] queue 15 vector 15 [ 3798.621059] set->mq_map[8] queue 0 vector 0 [ 3798.625460] set->mq_map[9] queue 1 vector 1 [ 3798.629852] set->mq_map[10] queue 2 vector 2 [ 3798.634331] set->mq_map[11] queue 3 vector 3 [ 3798.638796] set->mq_map[12] queue 4 vector 4 [ 3798.643263] set->mq_map[13] queue 5 vector 5 [ 3798.647727] set->mq_map[14] queue 6 vector 6 [ 3798.652197] set->mq_map[15] queue 7 vector 7 One comp_vector per core, but only numa-local cores: [ 3855.406027] iw_cxgb4: comp_vector 0, irq 203 mask 0x400 [ 3855.411577] iw_cxgb4: comp_vector 1, irq 204 mask 0x800 [ 3855.417057] iw_cxgb4: comp_vector 2, irq 205 mask 0x1000 [ 3855.422618] iw_cxgb4: comp_vector 3, irq 206 mask 0x2000 [ 3855.428176] iw_cxgb4: comp_vector 4, irq 207 mask 0x4000 [ 3855.433731] iw_cxgb4: comp_vector 5, irq 208 mask 0x8000 [ 3855.439293] iw_cxgb4: comp_vector 6, irq 209 mask 0x100 [ 3855.444770] iw_cxgb4: comp_vector 7, irq 210 mask 0x200 [ 3855.450231] iw_cxgb4: comp_vector 8, irq 211 mask 0x400 [ 3855.455691] iw_cxgb4: comp_vector 9, irq 212 mask 0x800 [ 3855.461144] iw_cxgb4: comp_vector 10, irq 213 mask 0x1000 [ 3855.466768] iw_cxgb4: comp_vector 11, irq 214 mask 0x2000 [ 3855.472379] iw_cxgb4: comp_vector 12, irq 215 mask 0x4000 [ 3855.477992] iw_cxgb4: comp_vector 13, irq 216 mask 0x8000 [ 3855.483599] iw_cxgb4: comp_vector 14, irq 217 mask 0x100 [ 3855.489116] iw_cxgb4: comp_vector 15, irq 218 mask 0x200 [ 3855.494644] set->mq_map[0] queue 8 vector 8 [ 3855.499046] set->mq_map[1] queue 9 vector 9 [ 3855.503445] set->mq_map[2] queue 10 vector 10 [ 3855.508025] set->mq_map[3] queue 11 vector 11 [ 3855.512600] set->mq_map[4] queue 12 vector 12 [ 3855.517176] set->mq_map[5] queue 13 vector 13 [ 3855.521750] set->mq_map[6] queue 14 vector 14 [ 3855.526325] set->mq_map[7] queue 15 vector 15 [ 3855.530902] set->mq_map[8] queue 6 vector 6 [ 3855.535306] set->mq_map[9] queue 7 vector 7 [ 3855.539703] set->mq_map[10] queue 0 vector 0 [ 3855.544197] set->mq_map[11] queue 1 vector 1 [ 3855.548670] set->mq_map[12] queue 2 vector 2 [ 3855.553144] set->mq_map[13] queue 3 vector 3 [ 3855.557630] set->mq_map[14] queue 4 vector 4 [ 3855.562105] set->mq_map[15] queue 5 vector 5 Each comp_vector has affinity to all numa-local cores: [ 4010.002954] iw_cxgb4: comp_vector 0, irq 203 mask 0xff00 [ 4010.008606] iw_cxgb4: comp_vector 1, irq 204 mask 0xff00 [ 4010.014179] iw_cxgb4: comp_vector 2, irq 205 mask 0xff00 [ 4010.019741] iw_cxgb4: comp_vector 3, irq 206 mask 0xff00 [ 4010.025310] iw_cxgb4: comp_vector 4, irq 207 mask 0xff00 [ 4010.030881] iw_cxgb4: comp_vector 5, irq 208 mask 0xff00 [ 4010.036448] iw_cxgb4: comp_vector 6, irq 209 mask 0xff00 [ 4010.042012] iw_cxgb4: comp_vector 7, irq 210 mask 0xff00 [ 4010.047562] iw_cxgb4: comp_vector 8, irq 211 mask 0xff00 [ 4010.053103] iw_cxgb4: comp_vector 9, irq 212 mask 0xff00 [ 4010.058632] iw_cxgb4: comp_vector 10, irq 213 mask 0xff00 [ 4010.064248] iw_cxgb4: comp_vector 11, irq 214 mask 0xff00 [ 4010.069863] iw_cxgb4: comp_vector 12, irq 215 mask 0xff00 [ 4010.075462] iw_cxgb4: comp_vector 13, irq 216 mask 0xff00 [ 4010.081066] iw_cxgb4: comp_vector 14, irq 217 mask 0xff00 [ 4010.086676] iw_cxgb4: comp_vector 15, irq 218 mask 0xff00 [ 4010.092283] set->mq_map[0] queue 8 vector 8 [ 4010.096683] set->mq_map[1] queue 9 vector 9 [ 4010.101085] set->mq_map[2] queue 10 vector 10 [ 4010.105663] set->mq_map[3] queue 11 vector 11 [ 4010.110239] set->mq_map[4] queue 12 vector 12 [ 4010.114810] set->mq_map[5] queue 13 vector 13 [ 4010.119379] set->mq_map[6] queue 14 vector 14 [ 4010.123942] set->mq_map[7] queue 15 vector 15 [ 4010.128502] set->mq_map[8] queue 0 vector 0 [ 4010.132903] set->mq_map[9] queue 1 vector 1 [ 4010.137299] set->mq_map[10] queue 2 vector 2 [ 4010.141792] set->mq_map[11] queue 3 vector 3 [ 4010.146274] set->mq_map[12] queue 4 vector 4 [ 4010.150754] set->mq_map[13] queue 5 vector 5 [ 4010.155235] set->mq_map[14] queue 6 vector 6 [ 4010.159716] set->mq_map[15] queue 7 vector 7
> Hey Sagi, > > The patch works allowing connections for the various affinity mappings below: > > One comp_vector per core across all cores, starting with numa-local cores: Thanks Steve, is this your "Tested by:" tag?
On Fri, Aug 17, 2018 at 01:03:20PM -0700, Sagi Grimberg wrote: > > > Hey Sagi, > > > > The patch works allowing connections for the various affinity mappings below: > > > > One comp_vector per core across all cores, starting with numa-local cores: > > Thanks Steve, is this your "Tested by:" tag? The new patchworks doesn't grab patches inlined in messages, so you will need to resend it. Also, can someone remind me what the outcome is here? Does it supersede Leon's patch: https://patchwork.kernel.org/patch/10526167/ ? Thanks, Jason
Hi Jason, > The new patchworks doesn't grab patches inlined in messages, so you > will need to resend it. Yes, just wanted to to add Steve's tested by as its going to lists that did not follow this thread. > Also, can someone remind me what the outcome is here? Does it > supersede Leon's patch: > > https://patchwork.kernel.org/patch/10526167/ Leon's patch is exposing the breakage so I think it would be wise to have it go in after this lands mainline.
> > > > Hey Sagi, > > > > The patch works allowing connections for the various affinity mappings > below: > > > > One comp_vector per core across all cores, starting with numa-local cores: > > Thanks Steve, is this your "Tested by:" tag? Sure: Tested-by: Steve Wise <swise@opengridcomputing.com>
diff --git a/block/blk-mq-cpumap.c b/block/blk-mq-cpumap.c index 3eb169f..02b888f 100644 --- a/block/blk-mq-cpumap.c +++ b/block/blk-mq-cpumap.c @@ -30,29 +30,36 @@ static int get_first_sibling(unsigned int cpu) return cpu; } -int blk_mq_map_queues(struct blk_mq_tag_set *set) +void blk_mq_map_queue_to_cpu(struct blk_mq_tag_set *set, unsigned int cpu) { unsigned int *map = set->mq_map; unsigned int nr_queues = set->nr_hw_queues; - unsigned int cpu, first_sibling; + unsigned int first_sibling; - 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 - * performace optimizations. - */ - if (cpu < nr_queues) { + /* + * 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 + * performace optimizations. + */ + if (cpu < nr_queues) { + map[cpu] = cpu_to_queue_index(nr_queues, cpu); + } else { + first_sibling = get_first_sibling(cpu); + if (first_sibling == cpu) map[cpu] = cpu_to_queue_index(nr_queues, cpu); - } else { - first_sibling = get_first_sibling(cpu); - if (first_sibling == cpu) - map[cpu] = cpu_to_queue_index(nr_queues, cpu); - else - map[cpu] = map[first_sibling]; - } + else + map[cpu] = map[first_sibling]; } +} +EXPORT_SYMBOL_GPL(blk_mq_map_queue_to_cpu); + +int blk_mq_map_queues(struct blk_mq_tag_set *set) +{ + unsigned int cpu; + + for_each_possible_cpu(cpu) + blk_mq_map_queue_to_cpu(set, cpu); return 0; } diff --git a/block/blk-mq-rdma.c b/block/blk-mq-rdma.c index 996167f..10e4f8a 100644 --- a/block/blk-mq-rdma.c +++ b/block/blk-mq-rdma.c @@ -34,14 +34,54 @@ int blk_mq_rdma_map_queues(struct blk_mq_tag_set *set, { const struct cpumask *mask; unsigned int queue, cpu; + bool mapped; + /* reset all CPUs mapping */ + for_each_possible_cpu(cpu) + set->mq_map[cpu] = UINT_MAX; + + /* Try to map the queues according to affinity */ for (queue = 0; queue < set->nr_hw_queues; queue++) { mask = ib_get_vector_affinity(dev, first_vec + queue); if (!mask) goto fallback; - for_each_cpu(cpu, mask) - set->mq_map[cpu] = queue; + for_each_cpu(cpu, mask) { + if (set->mq_map[cpu] == UINT_MAX) { + set->mq_map[cpu] = queue; + /* Initialy each queue mapped to 1 cpu */ + break; + } + } + } + + /* Map the unmapped queues in a naive way */ + for (queue = 0; queue < set->nr_hw_queues; queue++) { + mapped = false; + for_each_possible_cpu(cpu) { + if (set->mq_map[cpu] == queue) { + mapped = true; + break; + } + } + if (!mapped) { + for_each_possible_cpu(cpu) { + if (set->mq_map[cpu] == UINT_MAX) { + set->mq_map[cpu] = queue; + mapped = true; + break; + } + } + } + /* This case should never happen */ + if (WARN_ON_ONCE(!mapped)) + goto fallback; + } + + /* set all the rest of the CPUs */ + for_each_possible_cpu(cpu) { + if (set->mq_map[cpu] == UINT_MAX) + blk_mq_map_queue_to_cpu(set, cpu); } return 0; diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h index e3147eb..d6cd114 100644 --- a/include/linux/blk-mq.h +++ b/include/linux/blk-mq.h @@ -282,6 +282,7 @@ void blk_mq_tagset_busy_iter(struct blk_mq_tag_set *tagset, int blk_mq_freeze_queue_wait_timeout(struct request_queue *q, unsigned long timeout); +void blk_mq_map_queue_to_cpu(struct blk_mq_tag_set *set, unsigned int cpu); int blk_mq_map_queues(struct blk_mq_tag_set *set); void blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set, int nr_hw_queues);