Messages in this thread | | | Date | Fri, 3 Apr 2015 08:55:57 +0200 | From | Ingo Molnar <> | Subject | Re: [PATCH] irq: revert non-working patch to affinity defaults |
| |
* 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
| |