public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Fix off-by-one errors in some pirq warnings
@ 2008-03-31  2:22 Björn Steinbrink
  2008-03-31 12:50 ` Ingo Molnar
  0 siblings, 1 reply; 3+ messages in thread
From: Björn Steinbrink @ 2008-03-31  2:22 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: Thomas Gleixner, Linus Torvalds, Bob Tracy, linux-kernel

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

^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: 2.6.25-rc7: warn_on_slowpath triggered
@ 2008-03-31  2:03 Björn Steinbrink
  2008-03-31  2:27 ` [PATCH] Fix off-by-one errors in some pirq warnings Björn Steinbrink
  0 siblings, 1 reply; 3+ messages in thread
From: Björn Steinbrink @ 2008-03-31  2:03 UTC (permalink / raw)
  To: Bob Tracy; +Cc: linux-kernel, Ingo Molnar, Thomas Gleixner

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

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2008-03-31 12:51 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-03-31  2:22 [PATCH] Fix off-by-one errors in some pirq warnings Björn Steinbrink
2008-03-31 12:50 ` Ingo Molnar
  -- strict thread matches above, loose matches on Subject: below --
2008-03-31  2:03 2.6.25-rc7: warn_on_slowpath triggered Björn Steinbrink
2008-03-31  2:27 ` [PATCH] Fix off-by-one errors in some pirq warnings Björn Steinbrink

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