Re: [PATCH] irq: revert non-working patch to affinity defaults

Ingo Molnar
2015-04-03T06:56:07Z

* Jesse Brandeburg <jesse.brandeburg@intel.com> wrote:

> I've seen a couple of reports of issues since commit e2e64a932556 ("genirq:
> Set initial affinity in irq_set_affinity_hint()") where the
> affinity for the interrupt when programmed via
> /proc/irq/<nnn>/smp_affinity will not be able to stick. It changes back
> to some previous value at the next interrupt on that IRQ.
>
> The original intent was to fix the broken default behavior of all IRQs
> for a device starting up on CPU0. With a network card with 64 or more
> queues, all 64 queue's interrupt vectors end up on CPU0 which can have
> bad side effects, and has to be fixed by the irqbalance daemon, or by
> the user at every boot with some kind of affinity script.
>
> The symptom is that after a driver calls set_irq_affinity_hint, the
> affinity will be set for that interrupt (and readable via /proc/...),
> but on the first irq for that vector, the affinity for CPU0 or CPU1
> resets to the default. The rest of the irq affinites seem to work and
> everything is fine.
>
> Impact if we don't fix this for 4.0.0:
> Some users won't be able to set irq affinity as expected, on
> some cpus.
>
> I've spent a chunk of time trying to debug this with no luck and suggest
> that we revert the change if no-one else can help me debug what is going
> wrong, we can pick up the change later.
>
> This commit would also revert commit 4fe7ffb7e17ca ("genirq: Fix null pointer
> reference in irq_set_affinity_hint()") which was a bug fix to the original
> patch.

So the original commit also has the problem that it unnecessary
drops/retakes the descriptor lock:

> irq_put_desc_unlock(desc, flags);
> - /* set the initial affinity to prevent every interrupt being on CPU0 */
> - if (m)
> - __irq_set_affinity(irq, m, false);

i.e. why not just call into irq_set_affinity_locked() while we still
have the descriptor locked?

Now this is just a small annoyance that should not really matter - it
would be nice to figure out the real reason for why the irqs move back
to CPU#0.

In theory the same could happen to 'irqbalanced' as well, if it calls
shortly after an irq was registered - so this is not a bug we want to
ignore.

Also, worst case we are back to where v3.19 was, right? So could we
try to analyze this a bit more?

Thanks,

Ingo