[PATCH] turn off irqdebug by default on Altix

[PATCH] turn off irqdebug by default on Altix

[Martin Hicks]

[-- Attachment #1: Type: text/plain, Size: 626 bytes --]


Hello,

A while ago there was a discussion, started by Jack Steiner, about a hot
cache line in the irqdebug code that makes booting on large machines
very slow.  The original thread can be found here:

http://marc.theaimsgroup.com/?l=linux-kernel&m=106858073229831&w=2

The end result was that it was felt that irqdebug sould remain on by
default on small machines, and that large machines should turn it off be
default if it was causing a problem.

Here is a patch that turns it off for sn2.  It applies cleanly against
the linux-2.5 bk tree from this morning.
mh

-- 
Martin Hicks || mort@bork.org || PGP/GnuPG: 0x4C7F2BEE

[-- Attachment #2: sn2-irqdebug-off.diff --]
[-- Type: text/plain, Size: 2847 bytes --]

# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
#	           ChangeSet	1.1597  -> 1.1598 
#	arch/ia64/sn/kernel/setup.c	1.28    -> 1.29   
#	arch/ia64/kernel/irq.c	1.34    -> 1.35   
#	Documentation/kernel-parameters.txt	1.39    -> 1.40   
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 04/01/26	mort@tomahawk.engr.sgi.com	1.1598
# Turn off irqdebug by default on sn2.  Add an "irqdebug" command line
# option so it can be turned back on easily if required.
# --------------------------------------------
#
diff -Nru a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
--- a/Documentation/kernel-parameters.txt	Mon Jan 26 11:51:45 2004
+++ b/Documentation/kernel-parameters.txt	Mon Jan 26 11:51:45 2004
@@ -642,8 +642,13 @@
 			instruction doesn't work correctly and not to
 			use it.
 
-	noirqdebug	[IA-32] Disables the code which attempts to detect and
-			disable unhandled interrupt sources.
+	noirqdebug	[IA-32, IA-64] Disables the code which attempts to detect
+			and disable unhandled interrupt sources.  irqdebug is
+			on by default for all except ia64-sn2.
+
+	irqdebug	[IA-64] This turns on the code which attempts to detect
+			and diable unhandled interrupt sources.	 It is required
+			because the ia64-sn2 platform disables irqdebug by default.
 
 	noisapnp	[ISAPNP] Disables ISA PnP code.
 
diff -Nru a/arch/ia64/kernel/irq.c b/arch/ia64/kernel/irq.c
--- a/arch/ia64/kernel/irq.c	Mon Jan 26 11:51:45 2004
+++ b/arch/ia64/kernel/irq.c	Mon Jan 26 11:51:45 2004
@@ -288,7 +288,7 @@
 	}
 }
 
-static int noirqdebug;
+int noirqdebug;
 
 static int __init noirqdebug_setup(char *str)
 {
@@ -297,7 +297,15 @@
 	return 1;
 }
 
+static int __init irqdebug_setup(char *str)
+{
+	noirqdebug = 0;
+	printk("IRQ lockup detection enabled\n");
+	return 1;
+}
+
 __setup("noirqdebug", noirqdebug_setup);
+__setup("irqdebug", irqdebug_setup);
 
 /*
  * If 99,900 of the previous 100,000 interrupts have not been handled then
diff -Nru a/arch/ia64/sn/kernel/setup.c b/arch/ia64/sn/kernel/setup.c
--- a/arch/ia64/sn/kernel/setup.c	Mon Jan 26 11:51:45 2004
+++ b/arch/ia64/sn/kernel/setup.c	Mon Jan 26 11:51:45 2004
@@ -61,6 +61,7 @@
 extern void init_platform_hubinfo(nodepda_t **nodepdaindr);
 extern void (*ia64_mark_idle)(int);
 extern void snidle(int);
+extern int noirqdebug;
 
 unsigned long sn_rtc_cycles_per_second;   
 
@@ -265,6 +266,11 @@
 		printk(KERN_DEBUG "sn_setup: setting master_node_bedrock_address to 0x%lx\n",
 		       master_node_bedrock_address);
 	}
+
+	/*
+	 * Turn off irqdebug by default on sn
+	 */
+	noirqdebug = 1;
 
 	/*
 	 * we set the default root device to /dev/hda

RE: [PATCH] turn off irqdebug by default on Altix

[Chen, Kenneth W]

This is really confusing with two parameters that do the same thing.
What will happen if someone pass noirqdebug and irqdebug at the same
time?

What's wrong with boot time parameter of noirqdebug?

- Ken


-----Original Message-----
From: linux-ia64-owner@vger.kernel.org
[mailto:linux-ia64-owner@vger.kernel.org] On Behalf Of Martin Hicks
Sent: Wednesday, February 04, 2004 7:29 AM
To: linux-ia64@vger.kernel.org; akpm@osdl.org
Cc: steiner@sgi.com; jbarnes@sgi.com
Subject: [PATCH] turn off irqdebug by default on Altix

Hello,

A while ago there was a discussion, started by Jack Steiner, about a hot
cache line in the irqdebug code that makes booting on large machines
very slow.  The original thread can be found here:

http://marc.theaimsgroup.com/?l=linux-kernel&m\x106858073229831&w=2

The end result was that it was felt that irqdebug sould remain on by
default on small machines, and that large machines should turn it off be
default if it was causing a problem.

Here is a patch that turns it off for sn2.  It applies cleanly against
the linux-2.5 bk tree from this morning.
mh

-- 
Martin Hicks || mort@bork.org || PGP/GnuPG: 0x4C7F2BEE

Re: [PATCH] turn off irqdebug by default on Altix

[Martin Hicks]

On Wed, Feb 04, 2004 at 11:04:43AM -0800, Chen, Kenneth W wrote:
> This is really confusing with two parameters that do the same thing.
> What will happen if someone pass noirqdebug and irqdebug at the same
> time?
> 
> What's wrong with boot time parameter of noirqdebug?

It just means that if you forget to pass this option then your machine
never boots.  Well, it will boot.  Eventually.  Jack is a patient man,
and he said he never waited long enough for it to boot.

mh

-- 
Martin Hicks || mort@bork.org || PGP/GnuPG: 0x4C7F2BEE

Re: [PATCH] turn off irqdebug by default on Altix

[Andrew Morton]

Martin Hicks <mort@bork.org> wrote:
>
> A while ago there was a discussion, started by Jack Steiner, about a hot
>  cache line in the irqdebug code that makes booting on large machines
>  very slow.  The original thread can be found here:
> 
>  http://marc.theaimsgroup.com/?l=linux-kernel&m\x106858073229831&w=2
> 
>  The end result was that it was felt that irqdebug sould remain on by
>  default on small machines, and that large machines should turn it off be
>  default if it was causing a problem.

Yup.

>  Here is a patch that turns it off for sn2.  It applies cleanly against
>  the linux-2.5 bk tree from this morning.

I think it would be better to kill noirqdebug altogether, and to simply
require irqdebug=N.

Re: [PATCH] turn off irqdebug by default on Altix

[Jack Steiner]

On Wed, Feb 04, 2004 at 02:08:05PM -0500, Martin Hicks wrote:
> 
> 
> On Wed, Feb 04, 2004 at 11:04:43AM -0800, Chen, Kenneth W wrote:
> > This is really confusing with two parameters that do the same thing.
> > What will happen if someone pass noirqdebug and irqdebug at the same
> > time?
> > 
> > What's wrong with boot time parameter of noirqdebug?
> 
> It just means that if you forget to pass this option then your machine
> never boots.  Well, it will boot.  Eventually.  Jack is a patient man,
> and he said he never waited long enough for it to boot.

Actually, I determined that a big system would never boot. Each cpu would have to
do about 1500usec of work every 1000usec.  (highly dependent on the
system interconnect).

Each clock tick, the desc->irq_count is incremented by each cpu. On a big
system, this creates an incredibly hot cache line. On a big system, it takes
more than a millisecond to do the update. At that point, another clock
interrupt is already pending. Forward progress stops.

Another possible approach might be to special case note_interrupt() to
skip the update for the timer tick. (I havent actually looked at the code, but
this should be possible). 


> 
> mh
> 
> -- 
> Martin Hicks || mort@bork.org || PGP/GnuPG: 0x4C7F2BEE

-- 
Thanks

Jack Steiner (steiner@sgi.com)          651-683-5302
Principal Engineer                      SGI - Silicon Graphics, Inc.

RE: [PATCH] turn off irqdebug by default on Altix

[Chen, Kenneth W]

Don't take me wrong, I do want these irqdebug off too on one of our
large performance setup here as well :-)  Even reading from variable
noirqdebug is costing us some cache misses.  Can this be made compile
time option?  So when it is off, it is really off unless someone need to
turn this debug option on and off dynamically.

- Ken

-----Original Message-----
From: linux-ia64-owner@vger.kernel.org
[mailto:linux-ia64-owner@vger.kernel.org] On Behalf Of Jack Steiner
Sent: Wednesday, February 04, 2004 11:41 AM
To: Martin Hicks
Cc: Chen, Kenneth W; linux-ia64@vger.kernel.org; akpm@osdl.org;
jbarnes@sgi.com
Subject: Re: [PATCH] turn off irqdebug by default on Altix


On Wed, Feb 04, 2004 at 02:08:05PM -0500, Martin Hicks wrote:
> 
> 
> On Wed, Feb 04, 2004 at 11:04:43AM -0800, Chen, Kenneth W wrote:
> > This is really confusing with two parameters that do the same thing.
> > What will happen if someone pass noirqdebug and irqdebug at the same
> > time?
> > 
> > What's wrong with boot time parameter of noirqdebug?
> 
> It just means that if you forget to pass this option then your machine
> never boots.  Well, it will boot.  Eventually.  Jack is a patient man,
> and he said he never waited long enough for it to boot.

Actually, I determined that a big system would never boot. Each cpu
would have to
do about 1500usec of work every 1000usec.  (highly dependent on the
system interconnect).

Each clock tick, the desc->irq_count is incremented by each cpu. On a
big
system, this creates an incredibly hot cache line. On a big system, it
takes
more than a millisecond to do the update. At that point, another clock
interrupt is already pending. Forward progress stops.

Another possible approach might be to special case note_interrupt() to
skip the update for the timer tick. (I havent actually looked at the
code, but
this should be possible). 


> 
> mh
> 
> -- 
> Martin Hicks || mort@bork.org || PGP/GnuPG: 0x4C7F2BEE

-- 
Thanks

Jack Steiner (steiner@sgi.com)          651-683-5302
Principal Engineer                      SGI - Silicon Graphics, Inc.


-
To unsubscribe from this list: send the line "unsubscribe linux-ia64" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] turn off irqdebug by default on Altix

[Martin Hicks]

[-- Attachment #1: Type: text/plain, Size: 1509 bytes --]



On Wed, Feb 04, 2004 at 01:40:47PM -0600, Jack Steiner wrote:
> On Wed, Feb 04, 2004 at 02:08:05PM -0500, Martin Hicks wrote:
> > 
> > 
> > On Wed, Feb 04, 2004 at 11:04:43AM -0800, Chen, Kenneth W wrote:
> > > This is really confusing with two parameters that do the same thing.
> > > What will happen if someone pass noirqdebug and irqdebug at the same
> > > time?
> > > 
> > > What's wrong with boot time parameter of noirqdebug?
> > 
> > It just means that if you forget to pass this option then your machine
> > never boots.  Well, it will boot.  Eventually.  Jack is a patient man,
> > and he said he never waited long enough for it to boot.
> 
> Actually, I determined that a big system would never boot. Each cpu would have to
> do about 1500usec of work every 1000usec.  (highly dependent on the
> system interconnect).
> 
> Each clock tick, the desc->irq_count is incremented by each cpu. On a big
> system, this creates an incredibly hot cache line. On a big system, it takes
> more than a millisecond to do the update. At that point, another clock
> interrupt is already pending. Forward progress stops.
> 
> Another possible approach might be to special case note_interrupt() to
> skip the update for the timer tick. (I havent actually looked at the code, but
> this should be possible). 

Okay.  This is certainly less messy, and is IA64 specific.

How about this patch?  I just tested it on a 512p machine and it booted.

mh

-- 
Martin Hicks   ||   Silicon Graphics Inc.   ||   mort@sgi.com

[-- Attachment #2: no-timer-interrupt-in-note-interrupt.diff --]
[-- Type: text/plain, Size: 1080 bytes --]

# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
#	           ChangeSet	1.1543  -> 1.1544 
#	arch/ia64/kernel/irq.c	1.34    -> 1.35   
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 04/02/04	mort@green.i.bork.org	1.1544
# Don't debug the timer interrupt in note_interrupt().  This
# should allow forward progress on large machine with irqdebug
# enabled.
# --------------------------------------------
#
diff -Nru a/arch/ia64/kernel/irq.c b/arch/ia64/kernel/irq.c
--- a/arch/ia64/kernel/irq.c	Wed Feb  4 15:43:12 2004
+++ b/arch/ia64/kernel/irq.c	Wed Feb  4 15:43:12 2004
@@ -311,6 +311,10 @@
  */
 static void note_interrupt(int irq, irq_desc_t *desc, irqreturn_t action_ret)
 {
+	/* Don't debug the timer interrupt */
+	if (irq == IA64_TIMER_VECTOR)
+		return;
+
 	if (action_ret != IRQ_HANDLED) {
 		desc->irqs_unhandled++;
 		if (action_ret != IRQ_NONE)

Re: [PATCH] turn off irqdebug by default on Altix

[David Mosberger]

>>>>> On Thu, 5 Feb 2004 13:37:28 -0500, Martin Hicks <mort@sgi.com> said:

  Martin> How about this patch?  I just tested it on a 512p machine
  Martin> and it booted.

I suspect a better fix would be to change do_IRQ() such that it
doesn't call note_interrupt() for IRQ_PER_CPU interrupts.  For per-CPU
interrupts, the note_interrupt() code is racy and rather bogus anyhow.

	--david

Re: [PATCH] turn off irqdebug by default on Altix

[Martin Hicks]

[-- Attachment #1: Type: text/plain, Size: 642 bytes --]



On Tue, Feb 10, 2004 at 02:43:14PM -0800, David Mosberger wrote:
> >>>>> On Thu, 5 Feb 2004 13:37:28 -0500, Martin Hicks <mort@sgi.com> said:
> 
>   Martin> How about this patch?  I just tested it on a 512p machine
>   Martin> and it booted.
> 
> I suspect a better fix would be to change do_IRQ() such that it
> doesn't call note_interrupt() for IRQ_PER_CPU interrupts.  For per-CPU
> interrupts, the note_interrupt() code is racy and rather bogus anyhow.

Okay....Here it is.

I didn't test it on a big machine yet, but it certainly seems like it
takes care of the problem.

mh

-- 
Martin Hicks || mort@bork.org || PGP/GnuPG: 0x4C7F2BEE

[-- Attachment #2: no-note-interrupt.patch --]
[-- Type: text/plain, Size: 977 bytes --]

# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
#	           ChangeSet	1.1609  -> 1.1610 
#	arch/ia64/kernel/irq.c	1.34    -> 1.35   
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 04/02/11	mort@green.i.bork.org	1.1610
# Don't call note_interrupt() for per-cpu interrupts.
# --------------------------------------------
#
diff -Nru a/arch/ia64/kernel/irq.c b/arch/ia64/kernel/irq.c
--- a/arch/ia64/kernel/irq.c	Wed Feb 11 16:17:22 2004
+++ b/arch/ia64/kernel/irq.c	Wed Feb 11 16:17:22 2004
@@ -465,8 +465,6 @@
 		desc->handler->ack(irq);
 		action_ret = handle_IRQ_event(irq, regs, desc->action);
 		desc->handler->end(irq);
-		if (!noirqdebug)
-			note_interrupt(irq, desc, action_ret);
 	} else {
 		spin_lock(&desc->lock);
 		desc->handler->ack(irq);