From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758945AbcC3IuJ (ORCPT ); Wed, 30 Mar 2016 04:50:09 -0400 Received: from www.linutronix.de ([62.245.132.108]:37998 "EHLO Galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753397AbcC3IuD (ORCPT ); Wed, 30 Mar 2016 04:50:03 -0400 Date: Wed, 30 Mar 2016 10:48:19 +0200 (CEST) From: Thomas Gleixner To: David Daney cc: Ioan Nicu , Ralf Baechle , David Daney , Aleksey Makarov , Leonid Rosenboim , Jiang Liu , Marc Zyngier , Alexander Sverdlin , Uwe Duerr , linux-mips@linux-mips.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] MIPS: Octeon: do not change affinity for disabled irqs In-Reply-To: <56C3C9A4.1040109@gmail.com> Message-ID: References: <20160215154513.GF25050@ulegcpding.emea.nsn-net.net> <56C3C9A4.1040109@gmail.com> User-Agent: Alpine 2.11 (DEB 23 2013-08-11) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-Linutronix-Spam-Score: -1.0 X-Linutronix-Spam-Level: - X-Linutronix-Spam-Status: No , -1.0 points, 5.0 required, ALL_TRUSTED=-1,SHORTCIRCUIT=-0.0001,URIBL_BLOCKED=0.001 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > > Acked-by: Alexander Sverdlin > > 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