public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] x86: Fix XT-PIC-XT-PIC in /proc/interrupts
@ 2014-10-26 16:06 Maciej W. Rozycki
  2014-10-27  7:47 ` Ingo Molnar
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Maciej W. Rozycki @ 2014-10-26 16:06 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86; +Cc: linux-kernel

Fix duplicate XT-PIC seen in /proc/interrupts on x86 systems that make 
use of 8259A Programmable Interrupt Controllers.  Specifically convert 
output like this:

           CPU0
  0:      76573    XT-PIC-XT-PIC    timer
  1:         11    XT-PIC-XT-PIC    i8042
  2:          0    XT-PIC-XT-PIC    cascade
  4:          8    XT-PIC-XT-PIC    serial
  6:          3    XT-PIC-XT-PIC    floppy
  7:          0    XT-PIC-XT-PIC    parport0
  8:          1    XT-PIC-XT-PIC    rtc0
 10:        448    XT-PIC-XT-PIC    fddi0
 12:         23    XT-PIC-XT-PIC    eth0
 14:       2464    XT-PIC-XT-PIC    ide0
NMI:          0   Non-maskable interrupts
ERR:          0

to one like this:

           CPU0
  0:     122033    XT-PIC  timer
  1:         11    XT-PIC  i8042
  2:          0    XT-PIC  cascade
  4:          8    XT-PIC  serial
  6:          3    XT-PIC  floppy
  7:          0    XT-PIC  parport0
  8:          1    XT-PIC  rtc0
 10:        145    XT-PIC  fddi0
 12:         31    XT-PIC  eth0
 14:       2245    XT-PIC  ide0
NMI:          0   Non-maskable interrupts
ERR:          0

that is one like we used to have from ~2.2 till it was changed sometime.

The rationale is there is no value in this duplicate information, it 
merely clutters output and looks ugly.  We only have one handler for 
8259A interrupts so there is no need to give it a name separate from the 
name already given to irq_chip.

We could define meaningful names for handlers based on bits in the ELCR 
register on systems that have it or the value of the LTIM bit we use in 
ICW1 otherwise (hardcoded to 0 though with MCA support gone), to tell 
edge-triggered and level-triggered inputs apart.  While that information 
does not affect 8259A interrupt handlers it could help people determine 
which lines are shareable and which are not.  That is material for a 
separate change though.

Any tools that parse /proc/interrupts are supposed not to be affected 
since it was many years we used the format this change converts back to.

Signed-off-by: Maciej W. Rozycki <macro@linux-mips.org>
---
 Original report recorded here: https://lkml.org/lkml/2014/9/25/190 -- 
and given no feedback here is a fix, as promised.  I'll be happy to 
discuss it further if there is any concern about this change.

 Otherwise, please apply.

  Maciej

linux-x86-xt-pic.patch
Index: linux-3.16-dolch/arch/x86/kernel/i8259.c
===================================================================
--- linux-3.16-dolch.orig/arch/x86/kernel/i8259.c
+++ linux-3.16-dolch/arch/x86/kernel/i8259.c
@@ -111,8 +111,7 @@ static void make_8259A_irq(unsigned int 
 {
 	disable_irq_nosync(irq);
 	io_apic_irqs &= ~(1<<irq);
-	irq_set_chip_and_handler_name(irq, &i8259A_chip, handle_level_irq,
-				      i8259A_chip.name);
+	irq_set_chip_and_handler(irq, &i8259A_chip, handle_level_irq);
 	enable_irq(irq);
 }
 
Index: linux-3.16-dolch/arch/x86/kernel/irqinit.c
===================================================================
--- linux-3.16-dolch.orig/arch/x86/kernel/irqinit.c
+++ linux-3.16-dolch/arch/x86/kernel/irqinit.c
@@ -70,7 +70,6 @@ int vector_used_by_percpu_irq(unsigned i
 void __init init_ISA_irqs(void)
 {
 	struct irq_chip *chip = legacy_pic->chip;
-	const char *name = chip->name;
 	int i;
 
 #if defined(CONFIG_X86_64) || defined(CONFIG_X86_LOCAL_APIC)
@@ -79,7 +78,7 @@ void __init init_ISA_irqs(void)
 	legacy_pic->init(0);
 
 	for (i = 0; i < nr_legacy_irqs(); i++)
-		irq_set_chip_and_handler_name(i, chip, handle_level_irq, name);
+		irq_set_chip_and_handler(i, chip, handle_level_irq);
 }
 
 void __init init_IRQ(void)

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] x86: Fix XT-PIC-XT-PIC in /proc/interrupts
  2014-10-26 16:06 [PATCH] x86: Fix XT-PIC-XT-PIC in /proc/interrupts Maciej W. Rozycki
@ 2014-10-27  7:47 ` Ingo Molnar
  2014-10-27 12:29   ` Maciej W. Rozycki
  2014-10-28 11:13 ` [tip:x86/urgent] x86/irq: " tip-bot for Maciej W. Rozycki
  2014-10-28 11:18 ` tip-bot for Maciej W. Rozycki
  2 siblings, 1 reply; 6+ messages in thread
From: Ingo Molnar @ 2014-10-27  7:47 UTC (permalink / raw)
  To: Maciej W. Rozycki
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, linux-kernel


* Maciej W. Rozycki <macro@linux-mips.org> wrote:

> Fix duplicate XT-PIC seen in /proc/interrupts on x86 systems that make 
> use of 8259A Programmable Interrupt Controllers.  Specifically convert 
> output like this:
> 
>            CPU0
>   0:      76573    XT-PIC-XT-PIC    timer
>   1:         11    XT-PIC-XT-PIC    i8042
>   2:          0    XT-PIC-XT-PIC    cascade
>   4:          8    XT-PIC-XT-PIC    serial
>   6:          3    XT-PIC-XT-PIC    floppy
>   7:          0    XT-PIC-XT-PIC    parport0
>   8:          1    XT-PIC-XT-PIC    rtc0
>  10:        448    XT-PIC-XT-PIC    fddi0
>  12:         23    XT-PIC-XT-PIC    eth0
>  14:       2464    XT-PIC-XT-PIC    ide0
> NMI:          0   Non-maskable interrupts
> ERR:          0
> 
> to one like this:
> 
>            CPU0
>   0:     122033    XT-PIC  timer
>   1:         11    XT-PIC  i8042
>   2:          0    XT-PIC  cascade
>   4:          8    XT-PIC  serial
>   6:          3    XT-PIC  floppy
>   7:          0    XT-PIC  parport0
>   8:          1    XT-PIC  rtc0
>  10:        145    XT-PIC  fddi0
>  12:         31    XT-PIC  eth0
>  14:       2245    XT-PIC  ide0
> NMI:          0   Non-maskable interrupts
> ERR:          0
> 
> that is one like we used to have from ~2.2 till it was changed sometime.
> 
> The rationale is there is no value in this duplicate information, it 
> merely clutters output and looks ugly.  We only have one handler for 
> 8259A interrupts so there is no need to give it a name separate from the 
> name already given to irq_chip.
> 
> We could define meaningful names for handlers based on bits in the ELCR 
> register on systems that have it or the value of the LTIM bit we use in 
> ICW1 otherwise (hardcoded to 0 though with MCA support gone), to tell 
> edge-triggered and level-triggered inputs apart.  While that information 
> does not affect 8259A interrupt handlers it could help people determine 
> which lines are shareable and which are not.  That is material for a 
> separate change though.
> 
> Any tools that parse /proc/interrupts are supposed not to be affected 
> since it was many years we used the format this change converts back to.

What's the effect of this change on the output for non-8259A irq 
controllers?

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] x86: Fix XT-PIC-XT-PIC in /proc/interrupts
  2014-10-27  7:47 ` Ingo Molnar
@ 2014-10-27 12:29   ` Maciej W. Rozycki
  2014-10-27 12:44     ` Ingo Molnar
  0 siblings, 1 reply; 6+ messages in thread
From: Maciej W. Rozycki @ 2014-10-27 12:29 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, linux-kernel

On Mon, 27 Oct 2014, Ingo Molnar wrote:

> What's the effect of this change on the output for non-8259A irq 
> controllers?

 Good question, I didn't expect any other IRQ controller to be used for 
ISA interrupts (the APIC is handled elsewhere).  I have checked the 
sources and the only other controller that can be used for `legacy_pic' 
is `dummy_irq_chip' (cf. `default_legacy_pic' vs `null_legacy_pic' in 
arch/x86/kernel/i8259.c).  That affects `init_ISA_irqs' only (the other 
place, `make_8259A_irq', has a reference to `i8259A_chip' hardcoded).

 In this case the output would change from "dummy-dummy" to "dummy", 
which IMHO has just as much value as the change from "XT-PIC-XT-PIC" to 
"XT-PIC".  However I don't think you'd be able to request such IRQs so 
they will never show up in /proc/interrupts, making this observation 
largely irrelevant.  If you think I may be wrong here, then can you 
please find such a system and try this change with it (or can someone 
else reading this make this check for me by any chance)?  Unfortunately 
all x86 hardware I have does have an 8259A pair.

 Also please note that both places explicitly refer to the name of the 
IRQ chip being installed as the name of the handler as well, with 
`i8259A_chip.name' and `chip->name' respectively, this is hardcoded.  
So no matter which chip is used there'll be a duplicate name in 
/proc/interrupts.

 Does this answer address your concern?

  Maciej

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] x86: Fix XT-PIC-XT-PIC in /proc/interrupts
  2014-10-27 12:29   ` Maciej W. Rozycki
@ 2014-10-27 12:44     ` Ingo Molnar
  0 siblings, 0 replies; 6+ messages in thread
From: Ingo Molnar @ 2014-10-27 12:44 UTC (permalink / raw)
  To: Maciej W. Rozycki
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, linux-kernel


* Maciej W. Rozycki <macro@linux-mips.org> wrote:

> On Mon, 27 Oct 2014, Ingo Molnar wrote:
> 
> > What's the effect of this change on the output for non-8259A irq 
> > controllers?
> 
>  Good question, I didn't expect any other IRQ controller to be used for 
> ISA interrupts (the APIC is handled elsewhere).  I have checked the 
> sources and the only other controller that can be used for `legacy_pic' 
> is `dummy_irq_chip' (cf. `default_legacy_pic' vs `null_legacy_pic' in 
> arch/x86/kernel/i8259.c).  That affects `init_ISA_irqs' only (the other 
> place, `make_8259A_irq', has a reference to `i8259A_chip' hardcoded).
> 
>  In this case the output would change from "dummy-dummy" to "dummy", 
> which IMHO has just as much value as the change from "XT-PIC-XT-PIC" to 
> "XT-PIC".  However I don't think you'd be able to request such IRQs so 
> they will never show up in /proc/interrupts, making this observation 
> largely irrelevant.  If you think I may be wrong here, then can you 
> please find such a system and try this change with it (or can someone 
> else reading this make this check for me by any chance)?  Unfortunately 
> all x86 hardware I have does have an 8259A pair.
> 
>  Also please note that both places explicitly refer to the name of the 
> IRQ chip being installed as the name of the handler as well, with 
> `i8259A_chip.name' and `chip->name' respectively, this is hardcoded.  
> So no matter which chip is used there'll be a duplicate name in 
> /proc/interrupts.
> 
>  Does this answer address your concern?

Yeah, it addresses it, I'll queue up your fix, thanks!

	Ingo

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [tip:x86/urgent] x86/irq: Fix XT-PIC-XT-PIC in /proc/interrupts
  2014-10-26 16:06 [PATCH] x86: Fix XT-PIC-XT-PIC in /proc/interrupts Maciej W. Rozycki
  2014-10-27  7:47 ` Ingo Molnar
@ 2014-10-28 11:13 ` tip-bot for Maciej W. Rozycki
  2014-10-28 11:18 ` tip-bot for Maciej W. Rozycki
  2 siblings, 0 replies; 6+ messages in thread
From: tip-bot for Maciej W. Rozycki @ 2014-10-28 11:13 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: macro, hpa, torvalds, linux-kernel, tglx, mingo

Commit-ID:  0c7ba5008365b49c55f5bb9d9e859785b1f5cffe
Gitweb:     http://git.kernel.org/tip/0c7ba5008365b49c55f5bb9d9e859785b1f5cffe
Author:     Maciej W. Rozycki <macro@linux-mips.org>
AuthorDate: Sun, 26 Oct 2014 16:06:28 +0000
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 28 Oct 2014 11:04:55 +0100

x86/irq: Fix XT-PIC-XT-PIC in /proc/interrupts

Fix duplicate XT-PIC seen in /proc/interrupts on x86 systems
that make  use of 8259A Programmable Interrupt Controllers.
Specifically convert  output like this:

           CPU0
  0:      76573    XT-PIC-XT-PIC    timer
  1:         11    XT-PIC-XT-PIC    i8042
  2:          0    XT-PIC-XT-PIC    cascade
  4:          8    XT-PIC-XT-PIC    serial
  6:          3    XT-PIC-XT-PIC    floppy
  7:          0    XT-PIC-XT-PIC    parport0
  8:          1    XT-PIC-XT-PIC    rtc0
 10:        448    XT-PIC-XT-PIC    fddi0
 12:         23    XT-PIC-XT-PIC    eth0
 14:       2464    XT-PIC-XT-PIC    ide0
NMI:          0   Non-maskable interrupts
ERR:          0

to one like this:

           CPU0
  0:     122033    XT-PIC  timer
  1:         11    XT-PIC  i8042
  2:          0    XT-PIC  cascade
  4:          8    XT-PIC  serial
  6:          3    XT-PIC  floppy
  7:          0    XT-PIC  parport0
  8:          1    XT-PIC  rtc0
 10:        145    XT-PIC  fddi0
 12:         31    XT-PIC  eth0
 14:       2245    XT-PIC  ide0
NMI:          0   Non-maskable interrupts
ERR:          0

that is one like we used to have from ~2.2 till it was changed
sometime.

The rationale is there is no value in this duplicate
information, it  merely clutters output and looks ugly.  We only
have one handler for  8259A interrupts so there is no need to
give it a name separate from the  name already given to
irq_chip.

We could define meaningful names for handlers based on bits in
the ELCR  register on systems that have it or the value of the
LTIM bit we use in  ICW1 otherwise (hardcoded to 0 though with
MCA support gone), to tell  edge-triggered and level-triggered
inputs apart.  While that information  does not affect 8259A
interrupt handlers it could help people determine  which lines
are shareable and which are not.  That is material for a
separate change though.

Any tools that parse /proc/interrupts are supposed not to be
affected  since it was many years we used the format this change
converts back to.

Signed-off-by: Maciej W. Rozycki <macro@linux-mips.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Link: http://lkml.kernel.org/r/alpine.LFD.2.11.1410260147190.21390@eddie.linux-mips.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/i8259.c   | 3 +--
 arch/x86/kernel/irqinit.c | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/i8259.c b/arch/x86/kernel/i8259.c
index 8af8171..e7cc537 100644
--- a/arch/x86/kernel/i8259.c
+++ b/arch/x86/kernel/i8259.c
@@ -111,8 +111,7 @@ static void make_8259A_irq(unsigned int irq)
 {
 	disable_irq_nosync(irq);
 	io_apic_irqs &= ~(1<<irq);
-	irq_set_chip_and_handler_name(irq, &i8259A_chip, handle_level_irq,
-				      i8259A_chip.name);
+	irq_set_chip_and_handler(irq, &i8259A_chip, handle_level_irq);
 	enable_irq(irq);
 }
 
diff --git a/arch/x86/kernel/irqinit.c b/arch/x86/kernel/irqinit.c
index 44f1ed4..4de73ee 100644
--- a/arch/x86/kernel/irqinit.c
+++ b/arch/x86/kernel/irqinit.c
@@ -70,7 +70,6 @@ int vector_used_by_percpu_irq(unsigned int vector)
 void __init init_ISA_irqs(void)
 {
 	struct irq_chip *chip = legacy_pic->chip;
-	const char *name = chip->name;
 	int i;
 
 #if defined(CONFIG_X86_64) || defined(CONFIG_X86_LOCAL_APIC)
@@ -79,7 +78,7 @@ void __init init_ISA_irqs(void)
 	legacy_pic->init(0);
 
 	for (i = 0; i < nr_legacy_irqs(); i++)
-		irq_set_chip_and_handler_name(i, chip, handle_level_irq, name);
+		irq_set_chip_and_handler(i, chip, handle_level_irq);
 }
 
 void __init init_IRQ(void)

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [tip:x86/urgent] x86/irq: Fix XT-PIC-XT-PIC in /proc/interrupts
  2014-10-26 16:06 [PATCH] x86: Fix XT-PIC-XT-PIC in /proc/interrupts Maciej W. Rozycki
  2014-10-27  7:47 ` Ingo Molnar
  2014-10-28 11:13 ` [tip:x86/urgent] x86/irq: " tip-bot for Maciej W. Rozycki
@ 2014-10-28 11:18 ` tip-bot for Maciej W. Rozycki
  2 siblings, 0 replies; 6+ messages in thread
From: tip-bot for Maciej W. Rozycki @ 2014-10-28 11:18 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: torvalds, hpa, tglx, mingo, macro, linux-kernel

Commit-ID:  60e684f0d66369add7aa49fc39785d2f26fe9169
Gitweb:     http://git.kernel.org/tip/60e684f0d66369add7aa49fc39785d2f26fe9169
Author:     Maciej W. Rozycki <macro@linux-mips.org>
AuthorDate: Sun, 26 Oct 2014 16:06:28 +0000
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 28 Oct 2014 12:01:08 +0100

x86/irq: Fix XT-PIC-XT-PIC in /proc/interrupts

Fix duplicate XT-PIC seen in /proc/interrupts on x86 systems
that make  use of 8259A Programmable Interrupt Controllers.
Specifically convert  output like this:

           CPU0
  0:      76573    XT-PIC-XT-PIC    timer
  1:         11    XT-PIC-XT-PIC    i8042
  2:          0    XT-PIC-XT-PIC    cascade
  4:          8    XT-PIC-XT-PIC    serial
  6:          3    XT-PIC-XT-PIC    floppy
  7:          0    XT-PIC-XT-PIC    parport0
  8:          1    XT-PIC-XT-PIC    rtc0
 10:        448    XT-PIC-XT-PIC    fddi0
 12:         23    XT-PIC-XT-PIC    eth0
 14:       2464    XT-PIC-XT-PIC    ide0
NMI:          0   Non-maskable interrupts
ERR:          0

to one like this:

           CPU0
  0:     122033    XT-PIC  timer
  1:         11    XT-PIC  i8042
  2:          0    XT-PIC  cascade
  4:          8    XT-PIC  serial
  6:          3    XT-PIC  floppy
  7:          0    XT-PIC  parport0
  8:          1    XT-PIC  rtc0
 10:        145    XT-PIC  fddi0
 12:         31    XT-PIC  eth0
 14:       2245    XT-PIC  ide0
NMI:          0   Non-maskable interrupts
ERR:          0

that is one like we used to have from ~2.2 till it was changed
sometime.

The rationale is there is no value in this duplicate
information, it  merely clutters output and looks ugly.  We only
have one handler for  8259A interrupts so there is no need to
give it a name separate from the  name already given to
irq_chip.

We could define meaningful names for handlers based on bits in
the ELCR  register on systems that have it or the value of the
LTIM bit we use in  ICW1 otherwise (hardcoded to 0 though with
MCA support gone), to tell  edge-triggered and level-triggered
inputs apart.  While that information  does not affect 8259A
interrupt handlers it could help people determine  which lines
are shareable and which are not.  That is material for a
separate change though.

Any tools that parse /proc/interrupts are supposed not to be
affected  since it was many years we used the format this change
converts back to.

Signed-off-by: Maciej W. Rozycki <macro@linux-mips.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Link: http://lkml.kernel.org/r/alpine.LFD.2.11.1410260147190.21390@eddie.linux-mips.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/i8259.c   | 3 +--
 arch/x86/kernel/irqinit.c | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/i8259.c b/arch/x86/kernel/i8259.c
index 8af8171..e7cc537 100644
--- a/arch/x86/kernel/i8259.c
+++ b/arch/x86/kernel/i8259.c
@@ -111,8 +111,7 @@ static void make_8259A_irq(unsigned int irq)
 {
 	disable_irq_nosync(irq);
 	io_apic_irqs &= ~(1<<irq);
-	irq_set_chip_and_handler_name(irq, &i8259A_chip, handle_level_irq,
-				      i8259A_chip.name);
+	irq_set_chip_and_handler(irq, &i8259A_chip, handle_level_irq);
 	enable_irq(irq);
 }
 
diff --git a/arch/x86/kernel/irqinit.c b/arch/x86/kernel/irqinit.c
index 44f1ed4..4de73ee 100644
--- a/arch/x86/kernel/irqinit.c
+++ b/arch/x86/kernel/irqinit.c
@@ -70,7 +70,6 @@ int vector_used_by_percpu_irq(unsigned int vector)
 void __init init_ISA_irqs(void)
 {
 	struct irq_chip *chip = legacy_pic->chip;
-	const char *name = chip->name;
 	int i;
 
 #if defined(CONFIG_X86_64) || defined(CONFIG_X86_LOCAL_APIC)
@@ -79,7 +78,7 @@ void __init init_ISA_irqs(void)
 	legacy_pic->init(0);
 
 	for (i = 0; i < nr_legacy_irqs(); i++)
-		irq_set_chip_and_handler_name(i, chip, handle_level_irq, name);
+		irq_set_chip_and_handler(i, chip, handle_level_irq);
 }
 
 void __init init_IRQ(void)

^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2014-10-28 11:18 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-10-26 16:06 [PATCH] x86: Fix XT-PIC-XT-PIC in /proc/interrupts Maciej W. Rozycki
2014-10-27  7:47 ` Ingo Molnar
2014-10-27 12:29   ` Maciej W. Rozycki
2014-10-27 12:44     ` Ingo Molnar
2014-10-28 11:13 ` [tip:x86/urgent] x86/irq: " tip-bot for Maciej W. Rozycki
2014-10-28 11:18 ` tip-bot for Maciej W. Rozycki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox