Messages in this thread | | | Date | Wed, 1 Mar 2017 10:04:55 +0100 | From | Ingo Molnar <> | Subject | Re: [PATCH] x86/apic: Remove the extra judgement of skipped IO APIC setup |
| |
* Dou Liyang <douly.fnst@cn.fujitsu.com> wrote:
> As the commit 2e63ad4bd5dd ("x86/apic: Do not init irq remapping > if ioapic is disabled") added the judgement of skipped IO APIC > setup at the beginning of enable_IR_x2apic(). It may be redundant > that we check it again when we try to enable the interrupt mapping. > > So, remove the one in try_to_enable_IR() and refine them for > better readability. > > Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com> > --- > arch/x86/kernel/apic/apic.c | 17 ++++------------- > 1 file changed, 4 insertions(+), 13 deletions(-) > > diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c > index 8567c85..86e7bd8 100644 > --- a/arch/x86/kernel/apic/apic.c > +++ b/arch/x86/kernel/apic/apic.c > @@ -1610,24 +1610,15 @@ static inline void try_to_enable_x2apic(int remap_mode) { } > static inline void __x2apic_enable(void) { } > #endif /* !CONFIG_X86_X2APIC */ > > -static int __init try_to_enable_IR(void) > -{ > -#ifdef CONFIG_X86_IO_APIC > - if (!x2apic_enabled() && skip_ioapic_setup) { > - pr_info("Not enabling interrupt remapping due to skipped IO-APIC setup\n"); > - return -1; > - } > -#endif > - return irq_remapping_enable(); > -} > - > void __init enable_IR_x2apic(void) > { > unsigned long flags; > int ret, ir_stat; > > - if (skip_ioapic_setup) > + if (skip_ioapic_setup) { > + pr_info("Not init interrupt remapping due to skipped IO-APIC setup\n");
So you replaced a perfectly readable kernel message:
- pr_info("Not enabling interrupt remapping due to skipped IO-APIC setup\n");
... with an unreadable one:
+ pr_info("Not init interrupt remapping due to skipped IO-APIC setup\n");
Why?
Also, the changelog is pretty much unreadable as well:
> As the commit 2e63ad4bd5dd ("x86/apic: Do not init irq remapping > if ioapic is disabled") added the judgement of skipped IO APIC > setup at the beginning of enable_IR_x2apic(). It may be redundant > that we check it again when we try to enable the interrupt mapping. > > So, remove the one in try_to_enable_IR() and refine them for > better readability.
I edited it to:
The following commit:
2e63ad4bd5dd ("x86/apic: Do not init irq remapping if ioapic is disabled")
... added a check for skipped IO-APIC setup to enable_IR_x2apic(), but this check is also duplicated in try_to_enable_IR() - and it will never succeed in calling irq_remapping_enable().
Remove the whole irq_remapping_enable() complication: if the IO-APIC is disabled we cannot enable IRQ remapping.
And I restored the original pr_info() message as well.
Thanks,
Ingo
| |