[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <47178d4d-f730-6e59-5c19-58331cc3864a@opengridcomputing.com>
Date: Mon, 6 Aug 2018 14:20:37 -0500
From: Steve Wise <swise@...ngridcomputing.com>
To: Max Gurtovoy <maxg@...lanox.com>, Sagi Grimberg <sagi@...mberg.me>,
Jason Gunthorpe <jgg@...lanox.com>
Cc: 'Leon Romanovsky' <leon@...nel.org>,
'Doug Ledford' <dledford@...hat.com>,
'RDMA mailing list' <linux-rdma@...r.kernel.org>,
'Saeed Mahameed' <saeedm@...lanox.com>,
'linux-netdev' <netdev@...r.kernel.org>
Subject: Re: [PATCH mlx5-next] RDMA/mlx5: Don't use cached IRQ affinity mask
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.
Powered by blists - more mailing lists