* [PATCH] x86/apic: Remove the extra judgement of skipped IO APIC setup
@ 2017-02-23 9:16 Dou Liyang
2017-03-01 9:04 ` Ingo Molnar
2017-03-01 9:14 ` [tip:x86/urgent] x86/apic: Simplify enable_IR_x2apic(), remove try_to_enable_IR() tip-bot for Dou Liyang
0 siblings, 2 replies; 4+ messages in thread
From: Dou Liyang @ 2017-02-23 9:16 UTC (permalink / raw)
To: mingo, tglx, hpa, bp, wanpeng.li, nicstange; +Cc: x86, linux-kernel, Dou Liyang
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");
return;
+ }
ir_stat = irq_remapping_prepare();
if (ir_stat < 0 && !x2apic_supported())
@@ -1645,7 +1636,7 @@ void __init enable_IR_x2apic(void)
/* If irq_remapping_prepare() succeeded, try to enable it */
if (ir_stat >= 0)
- ir_stat = try_to_enable_IR();
+ ir_stat = irq_remapping_enable();
/* ir_stat contains the remap mode or an error code */
try_to_enable_x2apic(ir_stat);
--
2.5.5
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] x86/apic: Remove the extra judgement of skipped IO APIC setup
2017-02-23 9:16 [PATCH] x86/apic: Remove the extra judgement of skipped IO APIC setup Dou Liyang
@ 2017-03-01 9:04 ` Ingo Molnar
2017-03-01 9:31 ` Dou Liyang
2017-03-01 9:14 ` [tip:x86/urgent] x86/apic: Simplify enable_IR_x2apic(), remove try_to_enable_IR() tip-bot for Dou Liyang
1 sibling, 1 reply; 4+ messages in thread
From: Ingo Molnar @ 2017-03-01 9:04 UTC (permalink / raw)
To: Dou Liyang; +Cc: tglx, hpa, bp, wanpeng.li, nicstange, x86, linux-kernel
* 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
^ permalink raw reply [flat|nested] 4+ messages in thread
* [tip:x86/urgent] x86/apic: Simplify enable_IR_x2apic(), remove try_to_enable_IR()
2017-02-23 9:16 [PATCH] x86/apic: Remove the extra judgement of skipped IO APIC setup Dou Liyang
2017-03-01 9:04 ` Ingo Molnar
@ 2017-03-01 9:14 ` tip-bot for Dou Liyang
1 sibling, 0 replies; 4+ messages in thread
From: tip-bot for Dou Liyang @ 2017-03-01 9:14 UTC (permalink / raw)
To: linux-tip-commits
Cc: douly.fnst, torvalds, linux-kernel, peterz, tglx, mingo, hpa
Commit-ID: 11277aabcbbe13916151af897d29a5e9f71ca73f
Gitweb: http://git.kernel.org/tip/11277aabcbbe13916151af897d29a5e9f71ca73f
Author: Dou Liyang <douly.fnst@cn.fujitsu.com>
AuthorDate: Thu, 23 Feb 2017 17:16:41 +0800
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Wed, 1 Mar 2017 10:09:09 +0100
x86/apic: Simplify enable_IR_x2apic(), remove try_to_enable_IR()
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.
Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: bp@alien8.de
Cc: nicstange@gmail.com
Cc: wanpeng.li@hotmail.com
Link: http://lkml.kernel.org/r/1487841401-1543-1-git-send-email-douly.fnst@cn.fujitsu.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
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 11088b8..aee7ded 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 enabling interrupt remapping due to skipped IO-APIC setup\n");
return;
+ }
ir_stat = irq_remapping_prepare();
if (ir_stat < 0 && !x2apic_supported())
@@ -1645,7 +1636,7 @@ void __init enable_IR_x2apic(void)
/* If irq_remapping_prepare() succeeded, try to enable it */
if (ir_stat >= 0)
- ir_stat = try_to_enable_IR();
+ ir_stat = irq_remapping_enable();
/* ir_stat contains the remap mode or an error code */
try_to_enable_x2apic(ir_stat);
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] x86/apic: Remove the extra judgement of skipped IO APIC setup
2017-03-01 9:04 ` Ingo Molnar
@ 2017-03-01 9:31 ` Dou Liyang
0 siblings, 0 replies; 4+ messages in thread
From: Dou Liyang @ 2017-03-01 9:31 UTC (permalink / raw)
To: Ingo Molnar; +Cc: tglx, hpa, bp, wanpeng.li, nicstange, x86, linux-kernel
Dear Ingo,
At 03/01/2017 05:04 PM, Ingo Molnar wrote:
[...]
>> + 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?
I am very sorry.
Because of my weak English skills :) . I am trying to improve my
English ability.
>
> 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:
Thanks very much ! it became very clear.
>
> 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
Could you tell me what is the meaning of "..." . How to use it?
> 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.
Yes. Thanks!
Sincerely,
Liyang
>
> Thanks,
>
> Ingo
>
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-03-01 13:46 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-23 9:16 [PATCH] x86/apic: Remove the extra judgement of skipped IO APIC setup Dou Liyang
2017-03-01 9:04 ` Ingo Molnar
2017-03-01 9:31 ` Dou Liyang
2017-03-01 9:14 ` [tip:x86/urgent] x86/apic: Simplify enable_IR_x2apic(), remove try_to_enable_IR() tip-bot for Dou Liyang
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.