From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from wtarreau.pck.nerim.net ([62.212.114.60]:35620 "EHLO 1wt.eu" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932201AbdKBWk1 (ORCPT ); Thu, 2 Nov 2017 18:40:27 -0400 Date: Thu, 2 Nov 2017 23:40:22 +0100 From: Willy Tarreau To: Christoph Biedl Cc: stable@vger.kernel.org Subject: Re: [PATCH 3.10 000/139] 3.10.108-stable review Message-ID: <20171102224022.GA25751@1wt.eu> References: <1509571159-4405-1-git-send-email-w@1wt.eu> <1509609945@msgid.manchmal.in-ulm.de> <20171102081557.GB22399@1wt.eu> <1509653761@msgid.manchmal.in-ulm.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1509653761@msgid.manchmal.in-ulm.de> Sender: stable-owner@vger.kernel.org List-ID: Hi Christoph, On Thu, Nov 02, 2017 at 10:23:05PM +0100, Christoph Biedl wrote: > If I read my archived configurations correctly, that config item was > added (via oldconfig) in 3.19[1], and force-enabled in 4.1[2]. Running > "make oldconfig" might be an ususual use case for git bisect but it > works like a charm. So, [2] came through > > b1da1e715d4faf01468b7f45f7098922bc85ea8e is the first bad commit > Author: Jan Beulich > Date: Thu Feb 5 15:35:21 2015 +0000 > > x86/Kconfig: Simplify X86_IO_APIC dependencies > > But that one doesn't apply cleanly. > > This is the point where I feel I shouldn't touch things without deeper > knowledge. There has been huge rework in the APIC handling and I cannot > tell what is relevant here. > > The change [1] was triggered by 2f600025d but this still leaves a merge > conflict. So either ask someone who has an understanding of the > subsystem - or just do a hack to guard the change: > > --- a/arch/x86/kernel/apic/apic.c > +++ b/arch/x86/kernel/apic/apic.c > @@ -1581,8 +1581,10 @@ void __init enable_IR_x2apic(void) > int ret, x2apic_enabled = 0; > int hardware_init_ret; > > +#ifdef CONFIG_X86_IO_APIC > if (skip_ioapic_setup) > return; > +#endif > > /* Make sure irq_remap_ops are initialized */ > setup_irq_remapping_ops(); > > This at least builds, I haven't tested any further, though. Hehe good catch, now I can reproduce it. You have to disable SMP to be allowed to configure LOCAL_APIC without IO_APIC, and in this case you indeed get this error : arch/x86/kernel/apic/apic.c: In function 'enable_IR_x2apic': arch/x86/kernel/apic/apic.c:1584:6: error: 'skip_ioapic_setup' undeclared (first use in this function) arch/x86/kernel/apic/apic.c:1584:6: note: each undeclared identifier is reported only once for each function it appears in Your fix above looks totally correct given the commit description of the patch introducing these two lines in 3.10.105 [2e63ad4 ("x86/apic: Do not init irq remapping if ioapic is disabled")]. Other parts related to the IO_APIC are also enclosed in the same #ifdef, or within CONFIG_IRQ_REMAP that is selected by IO_APIC. > Otherwise, leaving a buildable kernel is honorable - but don't do this > just for me. The board this kernel configuration was for no longer runs > kernel 3.10. Actually, it's been off for quite a while. I agree, however build breakage is never fun, so I'd rather fix it since the fix is easy. With this I don't have the build issue anymore, I'm attaching it for reference. Thanks a lot for your analysis! Willy --- >>From 68cbe93962196f08a1a52e81dc6d5bedaca09b06 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Thu, 2 Nov 2017 23:22:31 +0100 Subject: x86/apic: fix build breakage caused by incomplete backport to 3.10 Commit 928a277 ("x86/apic: Do not init irq remapping if ioapic is disabled") introduced in 3.10.105 introduced an implicit dependency of CONFIG_X86_LOCAL_APIC to CONFIG_X86_IO_APIC which was later solved as part of simplifications on the config dependencies in more recent kernels. This dependency results in build failure when CONFIG_X86_LOCAL_APIC is set without CONFIG_X86_IO_APIC (this setup requires CONFIG_SMP=n). The reason is that skip_ioapic_setup is declared in apic.c and that the backported code was picked from a context where the #ifdef surrounding the function used to cover this condition. Let's just add the appropriate #ifdef to fix the 3.10 backport. Thanks to Christoph Biedl for reporting and diagnosing this one. Reported-by: Christoph Biedl Cc: Christoph Biedl Cc: Jan Beulich Cc: Wanpeng Li Cc: Thomas Gleixner Cc: Ingo Molnar Cc: "H. Peter Anvin" Signed-off-by: Willy Tarreau --- arch/x86/kernel/apic/apic.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c index 3cd8bfc..bc37dde 100644 --- a/arch/x86/kernel/apic/apic.c +++ b/arch/x86/kernel/apic/apic.c @@ -1581,8 +1581,10 @@ void __init enable_IR_x2apic(void) int ret, x2apic_enabled = 0; int hardware_init_ret; +#ifdef CONFIG_X86_IO_APIC if (skip_ioapic_setup) return; +#endif /* Make sure irq_remap_ops are initialized */ setup_irq_remapping_ops(); -- 2.8.0.rc2.1.gbe9624a