Re: [PATCH] MIPS: Octeon: do not change affinity for disabled irqs

[Thomas Gleixner]

On Tue, 16 Feb 2016, David Daney wrote:
> On 02/15/2016 07:45 AM, Ioan Nicu wrote:
> > Octeon sets the default irq affinity to value 1 in the early arch init
> > code, so by default all irqs get registered with their affinity set to
> > core 0.
> > 
> > When setting one CPU ofline, octeon_irq_cpu_offline_ciu() calls
> > irq_set_affinity_locked(), but this function sets the IRQD_AFFINITY_SET bit
> > in the irq descriptor. This has the side effect that if one irq is
> > requested later, after putting one CPU offline, the affinity of this irq
> > would not be the default anymore, but rather forced to "all cores - the
> > offline core".
> > 
> > This patch sets the IRQCHIP_ONOFFLINE_ENABLED flag in octeon irq
> > controllers, so that the kernel would call the irq_cpu_[on|off]line()
> > callbacks only for enabled irqs. If some other irq is requested after
> > setting one cpu offline, it would use the default irq affinity, same as it
> > would do in the normal case where there is no CPU hotplug operation.
> > 
> > Signed-off-by: Ioan Nicu <ioan.nicu.ext@xxxxxxxxx>
> > Acked-by: Alexander Sverdlin <alexander.sverdlin@xxxxxxxxx>
> 
> In principle, I don't object.
> 
> I would like to see what tglx has to say about this though.  If we are
> worried about the IRQD_AFFINITY_SET bit, I am not convinced that this is the
> best place to be tweaking code.  Are we papering over something that should
> be handled in a more general manner?  I don't know.

Hmm. Good question. We probably should not set IRQD_AFFINITY_SET when called
from the offline code. The flag was originally meant to preserve user space
affinity settings across request/free_irq.

Though it gets set via irq_set_affinity() as well and therefor via
irq_set_affinity_locked().

Now we could move that IRQD_AFFINITY_SET flip to the user space interface, but
we have to look at all the kernel internal call sites of irq_set_affinity()
and irq_set_affinity_locked() whether any of those relies on affinity settings
being preserved. irq_set_affinity_locked() probably not, as the only non core
user is the octeon code, but you probably can answer that question :)

Thanks,

	tglx