Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932214AbcCMGhC (ORCPT ); Sun, 13 Mar 2016 01:37:02 -0500 Received: from www.linutronix.de ([62.245.132.108]:42549 "EHLO Galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750769AbcCMGgy (ORCPT ); Sun, 13 Mar 2016 01:36:54 -0500 Date: Sun, 13 Mar 2016 07:35:04 +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 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2035 Lines: 60 On Sun, 13 Mar 2016, Jianyu Zhan 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 > I know this is weird, because modern SMP machines implies Local APIC. > But currently we have CONFIG_SMP detangle from CONFIG_X86_LOCAL_APIC, > which I think is fine. Do you actually understand how all that works together? > Another place which is weird is CONFIG_IRQ_WORK. Technically, it > does not depend > on SMP, nor even necessary Local APIC. Actually, it is just a base > configuration selected > by others. But currently we have the Have you tried to enable it independent from CONFIG_X86_LOCAL_APIC? > >> i = FIRST_EXTERNAL_VECTOR; > >> -#ifndef CONFIG_X86_LOCAL_APIC > >> -#define first_system_vector NR_VECTORS > >> -#endif > >> for_each_clear_bit_from(i, used_vectors, first_system_vector) { > > > > And how exactly is this here supposed to compile when CONFIG_X86_LOCAL_APIC=n? > > Dunno. I guess this code on !CONFIG_X86_LOCAL_APIC case hasn't been > tested yet ? It's your job to at least compile test your patches not the job of others. > For CONFIG_X86_LOCAL_APIC case, the define makes sense. > But for ! CONFIG_X86_LOCAL_APIC case, why we confine it to NR_VECTORS > is a mystery > to me. Have digged into git history, but found no proof. And because it's a mystery you can just change it as you think it's fine and thereby break the build? > So to maintain consistency, this patch just retain what it is, but we > do not bother update it for > !CONFIG_X86_LOCAL_APIC case. To maintain consistency we leave it as is, because that actually compiles AND works. Thanks, tglx