Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753986AbcCMH5A (ORCPT ); Sun, 13 Mar 2016 03:57:00 -0400 Received: from www.linutronix.de ([62.245.132.108]:42679 "EHLO Galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752010AbcCMH4z (ORCPT ); Sun, 13 Mar 2016 03:56:55 -0400 Date: Sun, 13 Mar 2016 08:55:07 +0100 (CET) From: Thomas Gleixner To: Jianyu Zhan cc: mingo@redhat.com, "H. Peter Anvin" , Aravind.Gopalakrishnan@amd.com, brgerst@gmail.com, bp@suse.de, feng.wu@intel.com, jiang.liu@linux.intel.com, Tejun Heo , dvlasenk@redhat.com, penberg@cs.helsinki.fi, Yinghai Lu , andi@firstfloor.org, Andy Lutomirski , ajm@sgi.com, Yinghai Lu , Akinobu Mita , x86@kernel.org, LKML Subject: Re: [PATCH 3/3] x86/irq: update first_system_vector only when X86_LOCAL_PIC is on In-Reply-To: Message-ID: References: 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 Content-Length: 2326 Lines: 72 On Sun, 13 Mar 2016, Jianyu Zhan wrote: > On Sun, Mar 13, 2016 at 2:35 PM, Thomas Gleixner wrote: > >> On Sun, Mar 13, 2016 at 4:08 AM, Thomas Gleixner wrote: > >> > This is pointless, because it's only called when local apic is enabled as all > >> > call sites of alloc_intr_gate() depend on CONFIG_X86_LOCAL_APIC .... > >> > >> Not exactly, currently at least smp_intr_init() DOES NOT depend on > >> CONFIG_X86_LOCAL_APIC: > >> > >> static void __init smp_intr_init(void) > >> { > >> #ifdef CONFIG_SMP > > > > It does, because CONFIG_SMP enables CONFIG_X86_LOCAL_APIC > > > > It is. Once CONFIG_SMP is on, CONFIG_X86_LOCAL_APIC will be turned on. > > So the situation should be described as: first_system_vector will be updated > on system vector init, on SMP case(which indirectly implies > CONFIG_X86_LOCAL_APIC) > and on explicit CONFIG_X86_LOCAL_APIC(X86_UP_APIC). > > So initial code: > > #ifndef CONFIG_X86_LOCAL_APIC > #define first_system_vector NR_VECTORS > #endif > > is actually not needed, becaused it won't ever change on ! > CONFIG_X86_LOCAL_APIC > case. It will never ever be updated in that case, simply because nothing uses it. > > Do you actually understand how all that works together? > > > > So the dependency should be reversed, and it should be like this: > > Currently we have CONFIG_X86_LOCAL_APIC detangle from CONFIG_SMP > (we can enable CONFIG_X86_LOCAL_APIC on 32-bit uniprocessor). And what's the benefit of this? > Among these stands out IRQ_WORK, which neither depends on SMP nor > CONFIG_X86_LOCAL_APIC. > > So the new init funciton is look like(from [2/3]): > > static void __init system_intr_init(void) > { > smp_intr_init(); > > #ifdef CONFIG_IRQ_WORK > alloc_intr_gate(IRQ_WORK_VECTOR, irq_work_interrupt); > #endif And that is completely wrong. IRQ_WORK can work independent of LOCAL_APIC, but if LOCAL_APIC is disabled it does not use the interrupt, simply because there is no way to trigger it. That setup is inside #ifdef CONFIG_X86_LOCAL_APIC for exactly that reason. Just because IRQ_WORK has no config dependency on LOCAL APIC that does not mean it uses the interrupt gate unconditionally. The code is correct as is and there is no reason to shuffle it in circles for no value. Thanks, tglx