All of lore.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 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.