lkml.org 
[lkml]   [2020]   [Jan]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] irqchip/gic-v3-its: Balance initial LPI affinity across CPUs
Hi John,

On 2020-01-20 18:21, John Garry wrote:
> On 20/01/2020 17:42, Marc Zyngier wrote:
>
> Hi Marc,
>
>>>> static u64 its_irq_get_msi_base(struct its_device *its_dev)
>>>> @@ -2773,28 +2829,34 @@ static int its_irq_domain_activate(struct
>>>> irq_domain *domain,
>>>> {
>>>> struct its_device *its_dev = irq_data_get_irq_chip_data(d);
>>>> u32 event = its_get_event_id(d);
>>>> - const struct cpumask *cpu_mask = cpu_online_mask;
>>>> - int cpu;
>>>> + int ret = 0, cpu = nr_cpu_ids;
>>>> + const struct cpumask *reqmask;
>>>> + cpumask_var_t mask;
>>>> - /* get the cpu_mask of local node */
>>>> - if (its_dev->its->numa_node >= 0)
>>>> - cpu_mask = cpumask_of_node(its_dev->its->numa_node);
>>>> + if (irqd_affinity_is_managed(d))
>>>> + reqmask = irq_data_get_affinity_mask(d);
>>>> + else
>>>> + reqmask = cpu_online_mask;
>>>> - /* Bind the LPI to the first possible CPU */
>>>> - cpu = cpumask_first_and(cpu_mask, cpu_online_mask);
>>>> - if (cpu >= nr_cpu_ids) {
>>>> - if (its_dev->its->flags & ITS_FLAGS_WORKAROUND_CAVIUM_23144)
>>>> - return -EINVAL;
>>>> + if (!alloc_cpumask_var(&mask, GFP_KERNEL))
>>>> + return -ENOMEM;
>>>> - cpu = cpumask_first(cpu_online_mask);
>>>> + its_compute_affinity(d, reqmask, mask);
>>>> + cpu = its_pick_target_cpu(mask);
>>>> + if (cpu >= nr_cpu_ids) {
>>>> + ret = -EINVAL;
>>>> + goto out;
>>>> }
>>>> + atomic_inc(per_cpu_ptr(&cpu_lpi_count, cpu));
>>>
>>> I wonder if we should only consider managed interrupts in this
>>> accounting?
>>>
>>> So cpu0 is effectively going to be excluded from the balancing, as it
>>> will have so many lpis targeted.
>>
>> Maybe, but only if the provided managed affinity gives you the
>> opportunity of placing the LPI somewhere else.
>
> Of course, if there's no other cpu in the mask then so be it.
>
> If the managed
>> affinity says CPU0 only, then that's where you end up.
>>
>
> If my debug code is correct (with the above fix), cpu0 had 763
> interrupts targeted on my D06 initially :)

You obviously have too many devices in this machine... ;-)

> But it's not just cpu0. I find initial non-managed interrupt affinity
> masks are set generally on cpu cluster/numa node masks, so the first
> cpus in those masks are bit over-subscribed, so then we may be
> spreading the managed interrupts over less cpus in the mask.
>
> This is a taste of lpi distribution on my 96 core system:
> cpu0 763
> cpu1 2
> cpu3 1
> cpu4 2
> cpu5 2
> cpu6 0
> cpu7 0
> cpu8 2
> cpu9 1
> cpu10 0
> ...
> cpu16 2
> ...
> cpu24 8
> ...
> cpu48 10 (numa node boundary)
> ...

We're stuck between a rock and a hard place here:

(1) We place all interrupts on the least loaded CPU that matches
the affinity -> results in performance issues on some funky
HW (like D05's SAS controller).

(2) We place managed interrupts on the least loaded CPU that matches
the affinity -> we have artificial load on NUMA boundaries, and
reduced spread of overlapping managed interrupts.

(3) We don't account for non-managed LPIs, and we run the risk of
unpredictable performance because we don't really know where
the *other* interrupts are.

My personal preference would be to go for (1), as in my original post.
I find (3) the least appealing, because we don't track things anymore.
(2) feels like "the least of all evils", as it is a decent performance
gain, seems to give predictable performance, and doesn't regress lesser
systems...

I'm definitely open to suggestions here.

Thanks,

M.
--
Jazz is not dead. It just smells funny...

\
 
 \ /
  Last update: 2020-01-20 19:46    [W:0.053 / U:3.132 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site