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=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 || [email protected] || PGP/GnuPG: 0x4C7F2BEE

# 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 [email protected] 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
@@ -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
@@ -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

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: [email protected] [mailto:[email protected]] On Behalf Of Martin Hicks
Sent: Wednesday, February 04, 2004 7:29 AM
To: [email protected]; [email protected]
Cc: [email protected]; [email protected]
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&m6858073229831&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 || [email protected] || PGP/GnuPG: 0x4C7F2BEE

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 || [email protected] || PGP/GnuPG: 0x4C7F2BEE

Yup.

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

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).

Thanks

Jack Steiner ([email protected])          651-683-5302
Principal Engineer                      SGI - Silicon Graphics, Inc.

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: [email protected] [mailto:[email protected]] On Behalf Of Jack Steiner
Sent: Wednesday, February 04, 2004 11:41 AM
To: Martin Hicks
Cc: Chen, Kenneth W; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH] turn off irqdebug by default on Altix

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).

Thanks

Jack Steiner ([email protected])          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 [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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.   ||   [email protected]

# 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 [email protected] 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)

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

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 || [email protected] || PGP/GnuPG: 0x4C7F2BEE

# 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 [email protected] 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
@@ -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);