Re: [PATCH net-next 11/12] net: mvpp2: handle cases where more CPUs are available than s/w threads

[Marc Zyngier]

Antoine,

On 19/09/18 10:27, Antoine Tenart wrote:
> The Marvell PPv2 network controller has 9 internal threads. The driver
> works fine when there are less CPUs available than threads. This isn't
> true if more CPUs are available. As this is a valid use case, handle
> this particular case.
> 
> Signed-off-by: Antoine Tenart <antoine.tenart@xxxxxxxxxxx>
> ---
>  drivers/net/ethernet/marvell/mvpp2/mvpp2.h    |  14 +-
>  .../net/ethernet/marvell/mvpp2/mvpp2_main.c   | 152 ++++++++++++------
>  2 files changed, 112 insertions(+), 54 deletions(-)

[...]

> @@ -3270,9 +3299,18 @@ static int mvpp2_irqs_init(struct mvpp2_port *port)
>  		if (err)
>  			goto err;
>  
> -		if (qv->type == MVPP2_QUEUE_VECTOR_PRIVATE)
> -			irq_set_affinity_hint(qv->irq,
> -					      cpumask_of(qv->sw_thread_id));
> +		if (qv->type == MVPP2_QUEUE_VECTOR_PRIVATE) {
> +			unsigned long mask = 0;
> +			unsigned int cpu;
> +
> +			for_each_present_cpu(cpu) {
> +				if (mvpp2_cpu_to_thread(port->priv, cpu) ==
> +				    qv->sw_thread_id)
> +					mask |= BIT(cpu);
> +			}
> +
> +			irq_set_affinity_hint(qv->irq, to_cpumask(&mask));
> +		}

Really??? How on Earth are you testing this code?

I'll paper over the implicit limitation to 64 CPUs and the open-coded
version of cpumask_set_cpu(). But the to_cpumask on a local variable
passed to the core IRQ code?

All it takes is to boot the box with a non-trivial distro, and here she 
goes:

[   11.779309] Unable to handle kernel paging request at virtual address ffff00000e8db278
[   11.779310] Mem abort info:
[   11.779311]   ESR = 0x96000007
[   11.779313]   Exception class = DABT (current EL), IL = 32 bits
[   11.779314]   SET = 0, FnV = 0
[   11.779315]   EA = 0, S1PTW = 0
[   11.779316] Data abort info:
[   11.779317]   ISV = 0, ISS = 0x00000007
[   11.779319]   CM = 0, WnR = 0
[   11.779321] swapper pgtable: 4k pages, 48-bit VAs, pgdp = 0000000092e8dfda
[   11.779322] [ffff00000e8db278] pgd=00000000effff803, pud=00000000efffe803, pmd=00000000e6795003, pte=0000000000000000
[   11.779328] Internal error: Oops: 96000007 [#1] PREEMPT SMP
[   11.779329] Modules linked in:
[   11.779334] CPU: 1 PID: 2952 Comm: irqbalance Not tainted 4.19.0-09418-g9f51ae62c84a-dirty #273
[   11.779336] Hardware name: Marvell 8040 MACCHIATOBin (DT)
[   11.779338] pstate: 40000085 (nZcv daIf -PAN -UAO)
[   11.779346] pc : irq_affinity_hint_proc_show+0x64/0xc0
[   11.779349] lr : irq_affinity_hint_proc_show+0x5c/0xc0
[   11.779350] sp : ffff00000e97bc70
[   11.779351] x29: ffff00000e97bc70 x28: 0000aaab12101290 
[   11.779354] x27: ffff8000e6ac6b40 x26: 0000000000000400 
[   11.779356] x25: ffff8000e80dd100 x24: ffff00000e97be60 
[   11.779358] x23: 00000000007000c0 x22: ffff8000e80272a4 
[   11.779360] x21: ffff8000e6ac6b00 x20: ffff8000e8027200 
[   11.779362] x19: ffff0000093596c8 x18: ffff000009371000 
[   11.779364] x17: 0000000000000000 x16: 0000000000000000 
[   11.779366] x15: 00000000fffffff0 x14: ffff0000094c3b62 
[   11.779368] x13: 0000000000000000 x12: ffff0000094c3000 
[   11.779370] x11: ffff000009371000 x10: ffff0000094c31b0 
[   11.779372] x9 : 0000000000000000 x8 : ffff0000094cd77b 
[   11.779374] x7 : 0000000000000000 x6 : 000000001c96d97e 
[   11.779376] x5 : 0000000000000000 x4 : ffff8000eff850b0 
[   11.779378] x3 : ffff8000e80272a4 x2 : ffff00000e8db278 
[   11.779380] x1 : 0000000000000000 x0 : 0000000000000000 
[   11.779383] Process irqbalance (pid: 2952, stack limit = 0x000000002121ca97)
[   11.779385] Call trace:
[   11.779387]  irq_affinity_hint_proc_show+0x64/0xc0
[   11.779391]  seq_read+0x130/0x448
[   11.779394]  proc_reg_read+0x6c/0xa8
[   11.779397]  __vfs_read+0x30/0x148
[   11.779398]  vfs_read+0x8c/0x160
[   11.779400]  ksys_read+0x5c/0xc0
[   11.779401]  __arm64_sys_read+0x18/0x20
[   11.779405]  el0_svc_common+0x84/0xd8
[   11.779407]  el0_svc_handler+0x2c/0x80
[   11.779410]  el0_svc+0x8/0xc
[   11.779412] Code: aa1603e0 942650d4 f9405e82 b4000062 (f9400041) 
[   11.779415] ---[ end trace 8648ea6a05ca014e ]---
[   11.779418] note: irqbalance[2952] exited with preempt_count 1

After that, the box is wedged.

I came up with the following fix, which fixes the issue for me.

Thanks,

	M.