Re: [PATCH 3.10 000/139] 3.10.108-stable review

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 <JBeulich@xxxxxxxx>
> 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 <w@xxxxxx>
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 <linux-kernel.bfrz@xxxxxxxxxxxxxxxxxx>
Cc: Christoph Biedl <linux-kernel.bfrz@xxxxxxxxxxxxxxxxxx>
Cc: Jan Beulich <JBeulich@xxxxxxxx>
Cc: Wanpeng Li <wanpeng.li@xxxxxxxxxxx>
Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
Cc: Ingo Molnar <mingo@xxxxxxxxxx>
Cc: "H. Peter Anvin" <hpa@xxxxxxxxx>
Signed-off-by: Willy Tarreau <w@xxxxxx>
---
 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




[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]