2.6.25-rc7: warn_on_slowpath triggered

2.6.25-rc7: warn_on_slowpath triggered

[Bob Tracy]

System is a AMD K6-III/450.  This is actually a 2.6.25-rcX issue
that I'm just getting around to reporting.  Sorry about that...

------------[ cut here ]------------
WARNING: at arch/x86/pci/irq.c:263 pirq_via586_get+0x23/0x42()
Modules linked in: snd_emu10k1(+) snd_rawmidi firmware_class snd_ac97_codec ac97_bus snd_pcm snd_seq_device snd_timer snd_page_alloc snd_util_mem snd_hwdep snd soundcore ipv6 usbmouse usbhid ff_memless uhci_hcd ehci_hcd usbcore binfmt_misc
Pid: 1784, comm: modprobe Not tainted 2.6.25-rc7 #1
 [<c0115b43>] warn_on_slowpath+0x40/0x4f
 [<c012755b>] run_hrtimer_pending+0xac/0xc5
 [<c011cc32>] run_timer_softirq+0x152/0x16b
 [<c0119b8c>] __do_softirq+0x59/0x85
 [<c027246b>] pci_conf1_read+0xac/0xc5
 [<c027334f>] raw_pci_read+0x3f/0x47
 [<c027246b>] pci_conf1_read+0xac/0xc5
 [<c027334f>] raw_pci_read+0x3f/0x47
 [<c0273421>] pci_read+0x1c/0x23
 [<c01f5bce>] pci_bus_read_config_byte+0x37/0x6c
 [<c01f5be8>] pci_bus_read_config_byte+0x51/0x6c
 [<c0272cef>] pirq_via586_get+0x23/0x42
 [<c0272ccc>] pirq_via586_get+0x0/0x42
 [<c0272fa7>] pcibios_lookup_irq+0x1da/0x355
 [<c0273152>] pirq_enable_irq+0x30/0x91
 [<c0273403>] pcibios_enable_device+0x21/0x23
 [<c01f798b>] do_pci_enable_device+0x1f/0x34
 [<c01f79f8>] __pci_enable_device_flags+0x58/0x64
 [<d8c51bb6>] snd_emu10k1_create+0x2f/0x5fb [snd_emu10k1]
 [<d8b87f5b>] snd_card_new+0x1f6/0x24a [snd]
 [<d8c5196e>] snd_card_emu10k1_probe+0xd2/0x2eb [snd_emu10k1]
 [<c01f8d59>] pci_device_probe+0x36/0x57
 [<c022f7d7>] driver_probe_device+0xb7/0x128
 [<c022f8d5>] __driver_attach+0x0/0x75
 [<c022f919>] __driver_attach+0x44/0x75
 [<c022ede5>] bus_for_each_dev+0x38/0x5a
 [<c022f670>] driver_attach+0x11/0x13
 [<c022f8d5>] __driver_attach+0x0/0x75
 [<c022f493>] bus_add_driver+0x8f/0x1ac
 [<c022fae2>] driver_register+0x45/0x99
 [<c01f8f46>] __pci_register_driver+0x2b/0x59
 [<c01316c2>] sys_init_module+0x14cc/0x1624
 [<c01584af>] do_sync_read+0xbe/0xfd
 [<d8c00552>] snd_pcm_format_width+0x0/0x1c [snd_pcm]
 [<c0158edb>] sys_read+0x3e/0x65
 [<c0102a42>] syscall_call+0x7/0xb
 =======================
---[ end trace 2f9d040b1fc63cd8 ]---

-- 
------------------------------------------------------------------------
Bob Tracy          |  "I was a beta tester for dirt.  They never did
rct@frus.com       |   get all the bugs out." - Steve McGrew on /.
------------------------------------------------------------------------

Re: 2.6.25-rc7: warn_on_slowpath triggered

[Björn Steinbrink]

[Added Ingo and Thomas to Cc:]

On 2008.03.29 17:29:55 -0500, Bob Tracy wrote:
> System is a AMD K6-III/450.  This is actually a 2.6.25-rcX issue
> that I'm just getting around to reporting.  Sorry about that...

Not necessarily a regression, it's just that the WARN_ON_ONCE didn't
exist yet in 2.6.24.

> 
> ------------[ cut here ]------------
> WARNING: at arch/x86/pci/irq.c:263 pirq_via586_get+0x23/0x42()
> Modules linked in: snd_emu10k1(+) snd_rawmidi firmware_class snd_ac97_codec ac97_bus snd_pcm snd_seq_device snd_timer snd_page_alloc snd_util_mem snd_hwdep snd soundcore ipv6 usbmouse usbhid ff_memless uhci_hcd ehci_hcd usbcore binfmt_misc
> Pid: 1784, comm: modprobe Not tainted 2.6.25-rc7 #1
>  [<c0115b43>] warn_on_slowpath+0x40/0x4f
>  [<c012755b>] run_hrtimer_pending+0xac/0xc5
>  [<c011cc32>] run_timer_softirq+0x152/0x16b
>  [<c0119b8c>] __do_softirq+0x59/0x85
>  [<c027246b>] pci_conf1_read+0xac/0xc5
>  [<c027334f>] raw_pci_read+0x3f/0x47
>  [<c027246b>] pci_conf1_read+0xac/0xc5
>  [<c027334f>] raw_pci_read+0x3f/0x47
>  [<c0273421>] pci_read+0x1c/0x23
>  [<c01f5bce>] pci_bus_read_config_byte+0x37/0x6c
>  [<c01f5be8>] pci_bus_read_config_byte+0x51/0x6c
>  [<c0272cef>] pirq_via586_get+0x23/0x42
>  [<c0272ccc>] pirq_via586_get+0x0/0x42
>  [<c0272fa7>] pcibios_lookup_irq+0x1da/0x355
>  [<c0273152>] pirq_enable_irq+0x30/0x91
>  [<c0273403>] pcibios_enable_device+0x21/0x23
>  [<c01f798b>] do_pci_enable_device+0x1f/0x34
>  [<c01f79f8>] __pci_enable_device_flags+0x58/0x64
>  [<d8c51bb6>] snd_emu10k1_create+0x2f/0x5fb [snd_emu10k1]
>  [<d8b87f5b>] snd_card_new+0x1f6/0x24a [snd]
>  [<d8c5196e>] snd_card_emu10k1_probe+0xd2/0x2eb [snd_emu10k1]
>  [<c01f8d59>] pci_device_probe+0x36/0x57
>  [<c022f7d7>] driver_probe_device+0xb7/0x128
>  [<c022f8d5>] __driver_attach+0x0/0x75
>  [<c022f919>] __driver_attach+0x44/0x75
>  [<c022ede5>] bus_for_each_dev+0x38/0x5a
>  [<c022f670>] driver_attach+0x11/0x13
>  [<c022f8d5>] __driver_attach+0x0/0x75
>  [<c022f493>] bus_add_driver+0x8f/0x1ac
>  [<c022fae2>] driver_register+0x45/0x99
>  [<c01f8f46>] __pci_register_driver+0x2b/0x59
>  [<c01316c2>] sys_init_module+0x14cc/0x1624
>  [<c01584af>] do_sync_read+0xbe/0xfd
>  [<d8c00552>] snd_pcm_format_width+0x0/0x1c [snd_pcm]
>  [<c0158edb>] sys_read+0x3e/0x65
>  [<c0102a42>] syscall_call+0x7/0xb
>  =======================
> ---[ end trace 2f9d040b1fc63cd8 ]---
> 

Re: 2.6.25-rc7: warn_on_slowpath triggered

[Björn Steinbrink]

On 2008.03.31 03:40:37 +0200, Björn Steinbrink wrote:
> [Added Ingo and Thomas to Cc:]
> 
> On 2008.03.29 17:29:55 -0500, Bob Tracy wrote:
> > System is a AMD K6-III/450.  This is actually a 2.6.25-rcX issue
> > that I'm just getting around to reporting.  Sorry about that...
> 
> Not necessarily a regression, it's just that the WARN_ON_ONCE didn't
> exist yet in 2.6.24.

Hm, the warnings seem to be a bit broken. They were added in
7d409d6057c7244f8757ce15245f6df27271be0c "x86: add some pirq debugging".

In pirq_via586_{get,set} and pirq_ite_{get,set}, they're missing the
fact that we're using pirq-1 to index the array. So it's spitting out
warnings for valid cases. So Bob's warning can very well be completely
bogus.

In pirq_ali_{get,set} there's probably the same problem, but I can't
really tell for sure, because pirq isn't used to index the array
directly, and the comment above that functions tells me not to guess
anything.

For pirq_vlsi_{get,set} I just wondered why that ">= 9" instead of "> 8"
as it is on the next line, but well, that's just style nit-picking...

Björn

[PATCH] Fix off-by-one errors in some pirq warnings

[Björn Steinbrink]

The conditions for the warnings simply ignored the fact that we later actually
use the original value minus 1, so the warning triggered even for valid values.

Signed-off-by: Björn Steinbrink <B.Steinbrink@gmx.de>
---
[Sorry for the resend, I messed up the first one with a Reply-To instead
of an In-Reply-To header. I'll put on my brown paper bag...]

> In pirq_ali_{get,set} there's probably the same problem, but I can't
> really tell for sure, because pirq isn't used to index the array
> directly, and the comment above that functions tells me not to guess
> anything.

OK, so I made a guess anyway, the new condition at least makes more
sense to me.

 arch/x86/pci/irq.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/x86/pci/irq.c b/arch/x86/pci/irq.c
index a871586..579745c 100644
--- a/arch/x86/pci/irq.c
+++ b/arch/x86/pci/irq.c
@@ -200,7 +200,7 @@ static int pirq_ali_get(struct pci_dev *router, struct pci_dev *dev, int pirq)
 {
 	static const unsigned char irqmap[16] = { 0, 9, 3, 10, 4, 5, 7, 6, 1, 11, 0, 12, 0, 14, 0, 15 };
 
-	WARN_ON_ONCE(pirq >= 16);
+	WARN_ON_ONCE(pirq > 16);
 	return irqmap[read_config_nybble(router, 0x48, pirq-1)];
 }
 
@@ -209,7 +209,7 @@ static int pirq_ali_set(struct pci_dev *router, struct pci_dev *dev, int pirq, i
 	static const unsigned char irqmap[16] = { 0, 8, 0, 2, 4, 5, 7, 6, 0, 1, 3, 9, 11, 0, 13, 15 };
 	unsigned int val = irqmap[irq];
 
-	WARN_ON_ONCE(pirq >= 16);
+	WARN_ON_ONCE(pirq > 16);
 	if (val) {
 		write_config_nybble(router, 0x48, pirq-1, val);
 		return 1;
@@ -260,7 +260,7 @@ static int pirq_via586_get(struct pci_dev *router, struct pci_dev *dev, int pirq
 {
 	static const unsigned int pirqmap[5] = { 3, 2, 5, 1, 1 };
 
-	WARN_ON_ONCE(pirq >= 5);
+	WARN_ON_ONCE(pirq > 5);
 	return read_config_nybble(router, 0x55, pirqmap[pirq-1]);
 }
 
@@ -268,7 +268,7 @@ static int pirq_via586_set(struct pci_dev *router, struct pci_dev *dev, int pirq
 {
 	static const unsigned int pirqmap[5] = { 3, 2, 5, 1, 1 };
 
-	WARN_ON_ONCE(pirq >= 5);
+	WARN_ON_ONCE(pirq > 5);
 	write_config_nybble(router, 0x55, pirqmap[pirq-1], irq);
 	return 1;
 }
@@ -282,7 +282,7 @@ static int pirq_ite_get(struct pci_dev *router, struct pci_dev *dev, int pirq)
 {
 	static const unsigned char pirqmap[4] = { 1, 0, 2, 3 };
 
-	WARN_ON_ONCE(pirq >= 4);
+	WARN_ON_ONCE(pirq > 4);
 	return read_config_nybble(router,0x43, pirqmap[pirq-1]);
 }
 
@@ -290,7 +290,7 @@ static int pirq_ite_set(struct pci_dev *router, struct pci_dev *dev, int pirq, i
 {
 	static const unsigned char pirqmap[4] = { 1, 0, 2, 3 };
 
-	WARN_ON_ONCE(pirq >= 4);
+	WARN_ON_ONCE(pirq > 4);
 	write_config_nybble(router, 0x43, pirqmap[pirq-1], irq);
 	return 1;
 }
-- 
1.5.5.rc2

Re: 2.6.25-rc7: warn_on_slowpath triggered

[Bob Tracy]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=UNKNOWN-8BIT, Size: 1815 bytes --]

Björn Steinbrink wrote:
> On 2008.03.31 03:40:37 +0200, Bj?rn Steinbrink wrote:
> > [Added Ingo and Thomas to Cc:]
> > 
> > On 2008.03.29 17:29:55 -0500, Bob Tracy wrote:
> > > System is a AMD K6-III/450.  This is actually a 2.6.25-rcX issue
> > > that I'm just getting around to reporting.  Sorry about that...
> > 
> > Not necessarily a regression, it's just that the WARN_ON_ONCE didn't
> > exist yet in 2.6.24.
> 
> Hm, the warnings seem to be a bit broken. They were added in
> 7d409d6057c7244f8757ce15245f6df27271be0c "x86: add some pirq debugging".
> 
> In pirq_via586_{get,set} and pirq_ite_{get,set}, they're missing the
> fact that we're using pirq-1 to index the array. So it's spitting out
> warnings for valid cases. So Bob's warning can very well be completely
> bogus.
> 
> In pirq_ali_{get,set} there's probably the same problem, but I can't
> really tell for sure, because pirq isn't used to index the array
> directly, and the comment above that functions tells me not to guess
> anything.
> 
> For pirq_vlsi_{get,set} I just wondered why that ">= 9" instead of "> 8"
> as it is on the next line, but well, that's just style nit-picking...
> 
> Bj?rn

I've been out of town for the past week and a half, and had no way to
test the "linux/arch/x86/pci/irq.c" patch you provided.  Since this is
still being tracked as an unresolved issue, I guess I need to say "it
fixes the problem for me." Your fix didn't make it into mainline for
2.6.25-rc8, but it applies cleanly and works properly there as well.

Thanks!

-- 
------------------------------------------------------------------------
Bob Tracy          |  "I was a beta tester for dirt.  They never did
rct@frus.com       |   get all the bugs out." - Steve McGrew on /.
------------------------------------------------------------------------

[PATCH] Fix off-by-one errors in some pirq warnings

[Björn Steinbrink]

The conditions for the warnings simply ignored the fact that we later actually
use the original value minus 1, so the warning triggered even for valid values.

Signed-off-by: Björn Steinbrink <B.Steinbrink@gmx.de>
---
> In pirq_ali_{get,set} there's probably the same problem, but I can't
> really tell for sure, because pirq isn't used to index the array
> directly, and the comment above that functions tells me not to guess
> anything.

OK, so I made a guess anyway, the new condition at least makes more
sense to me.

 arch/x86/pci/irq.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/x86/pci/irq.c b/arch/x86/pci/irq.c
index a871586..579745c 100644
--- a/arch/x86/pci/irq.c
+++ b/arch/x86/pci/irq.c
@@ -200,7 +200,7 @@ static int pirq_ali_get(struct pci_dev *router, struct pci_dev *dev, int pirq)
 {
 	static const unsigned char irqmap[16] = { 0, 9, 3, 10, 4, 5, 7, 6, 1, 11, 0, 12, 0, 14, 0, 15 };
 
-	WARN_ON_ONCE(pirq >= 16);
+	WARN_ON_ONCE(pirq > 16);
 	return irqmap[read_config_nybble(router, 0x48, pirq-1)];
 }
 
@@ -209,7 +209,7 @@ static int pirq_ali_set(struct pci_dev *router, struct pci_dev *dev, int pirq, i
 	static const unsigned char irqmap[16] = { 0, 8, 0, 2, 4, 5, 7, 6, 0, 1, 3, 9, 11, 0, 13, 15 };
 	unsigned int val = irqmap[irq];
 
-	WARN_ON_ONCE(pirq >= 16);
+	WARN_ON_ONCE(pirq > 16);
 	if (val) {
 		write_config_nybble(router, 0x48, pirq-1, val);
 		return 1;
@@ -260,7 +260,7 @@ static int pirq_via586_get(struct pci_dev *router, struct pci_dev *dev, int pirq
 {
 	static const unsigned int pirqmap[5] = { 3, 2, 5, 1, 1 };
 
-	WARN_ON_ONCE(pirq >= 5);
+	WARN_ON_ONCE(pirq > 5);
 	return read_config_nybble(router, 0x55, pirqmap[pirq-1]);
 }
 
@@ -268,7 +268,7 @@ static int pirq_via586_set(struct pci_dev *router, struct pci_dev *dev, int pirq
 {
 	static const unsigned int pirqmap[5] = { 3, 2, 5, 1, 1 };
 
-	WARN_ON_ONCE(pirq >= 5);
+	WARN_ON_ONCE(pirq > 5);
 	write_config_nybble(router, 0x55, pirqmap[pirq-1], irq);
 	return 1;
 }
@@ -282,7 +282,7 @@ static int pirq_ite_get(struct pci_dev *router, struct pci_dev *dev, int pirq)
 {
 	static const unsigned char pirqmap[4] = { 1, 0, 2, 3 };
 
-	WARN_ON_ONCE(pirq >= 4);
+	WARN_ON_ONCE(pirq > 4);
 	return read_config_nybble(router,0x43, pirqmap[pirq-1]);
 }
 
@@ -290,7 +290,7 @@ static int pirq_ite_set(struct pci_dev *router, struct pci_dev *dev, int pirq, i
 {
 	static const unsigned char pirqmap[4] = { 1, 0, 2, 3 };
 
-	WARN_ON_ONCE(pirq >= 4);
+	WARN_ON_ONCE(pirq > 4);
 	write_config_nybble(router, 0x43, pirqmap[pirq-1], irq);
 	return 1;
 }
-- 
1.5.5.rc2

Re: [PATCH] Fix off-by-one errors in some pirq warnings

[Ingo Molnar]

* Björn Steinbrink <B.Steinbrink@gmx.de> wrote:

> The conditions for the warnings simply ignored the fact that we later 
> actually use the original value minus 1, so the warning triggered even 
> for valid values.

thanks Björn, applied.

	Ingo