Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1586765 > unrolled thread
| Started by | Dou Liyang <douly.fnst@cn.fujitsu.com> |
|---|---|
| First post | 2017-02-23 10:20 +0100 |
| Last post | 2017-03-01 10:50 +0100 |
| Articles | 4 — 3 participants |
Back to article view | Back to linux.kernel
[PATCH] x86/apic: Remove the extra judgement of skipped IO APIC setup Dou Liyang <douly.fnst@cn.fujitsu.com> - 2017-02-23 10:20 +0100
Re: [PATCH] x86/apic: Remove the extra judgement of skipped IO APIC setup Ingo Molnar <mingo@kernel.org> - 2017-03-01 10:20 +0100
Re: [PATCH] x86/apic: Remove the extra judgement of skipped IO APIC setup Dou Liyang <douly.fnst@cn.fujitsu.com> - 2017-03-01 14:50 +0100
[tip:x86/urgent] x86/apic: Simplify enable_IR_x2apic(), remove try_to_enable_IR() tip-bot for Dou Liyang <tipbot@zytor.com> - 2017-03-01 10:50 +0100
| From | Dou Liyang <douly.fnst@cn.fujitsu.com> |
|---|---|
| Date | 2017-02-23 10:20 +0100 |
| Subject | [PATCH] x86/apic: Remove the extra judgement of skipped IO APIC setup |
| Message-ID | <tdXJ7-7Ko-13@gated-at.bofh.it> |
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
[toc] | [next] | [standalone]
| From | Ingo Molnar <mingo@kernel.org> |
|---|---|
| Date | 2017-03-01 10:20 +0100 |
| Subject | Re: [PATCH] x86/apic: Remove the extra judgement of skipped IO APIC setup |
| Message-ID | <tg8Av-Y7-13@gated-at.bofh.it> |
| In reply to | #1586765 |
* 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
[toc] | [prev] | [next] | [standalone]
| From | Dou Liyang <douly.fnst@cn.fujitsu.com> |
|---|---|
| Date | 2017-03-01 14:50 +0100 |
| Subject | Re: [PATCH] x86/apic: Remove the extra judgement of skipped IO APIC setup |
| Message-ID | <tgcNI-3LX-7@gated-at.bofh.it> |
| In reply to | #1590121 |
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
>
>
>
[toc] | [prev] | [next] | [standalone]
| From | tip-bot for Dou Liyang <tipbot@zytor.com> |
|---|---|
| Date | 2017-03-01 10:50 +0100 |
| Subject | [tip:x86/urgent] x86/apic: Simplify enable_IR_x2apic(), remove try_to_enable_IR() |
| Message-ID | <tg93s-1aM-9@gated-at.bofh.it> |
| In reply to | #1586765 |
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);
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web