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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <alpine.DEB.2.11.1603301030270.3978@nanos>
Date:	Wed, 30 Mar 2016 10:48:19 +0200 (CEST)
From:	Thomas Gleixner <tglx@...utronix.de>
To:	David Daney <ddaney.cavm@...il.com>
cc:	Ioan Nicu <ioan.nicu.ext@...ia.com>,
	Ralf Baechle <ralf@...ux-mips.org>,
	David Daney <david.daney@...ium.com>,
	Aleksey Makarov <aleksey.makarov@...iga.com>,
	Leonid Rosenboim <lrosenboim@...iumnetworks.com>,
	Jiang Liu <jiang.liu@...ux.intel.com>,
	Marc Zyngier <marc.zyngier@....com>,
	Alexander Sverdlin <alexander.sverdlin@...ia.com>,
	Uwe Duerr <uwe.duerr.ext@...ia.com>, linux-mips@...ux-mips.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] MIPS: Octeon: do not change affinity for disabled irqs

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@...ia.com>
> > Acked-by: Alexander Sverdlin <alexander.sverdlin@...ia.com>
> 
> 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