Re: PATCH: Minimal fix for sysrq on serial console hang

Re: PATCH: Minimal fix for sysrq on serial console hang

[Russell King]

On Wed, Jul 12, 2006 at 02:54:21PM +0100, Alan Cox wrote:
> When I originally did this change I used oops_in_progress as a locking
> guide. However it turns out there is one other place that turns all the
> locking on its head and that is sysrq.

Well, akpm's had a fix in his tree for some time, which he's been
pestering me with, so I committed that a few days ago:

# Author:    Andrew Morton (Fri Jun 30 10:29:59 BST 2006)
# Committer: Russell King (Sun Jul  9 21:11:10 BST 2006)
#	
#	[SERIAL] 8250: sysrq deadlock fix
#	
#	Fix http://bugzilla.kernel.org/show_bug.cgi?id=6716
#	
#	Doing a sysrq over a serial line into an SMP machine presently deadlocks.
#	
#	Signed-off-by: Andrew Morton
#	Signed-off-by: Russell King
#
#	 drivers/serial/8250.c |   13 +++++++++----
#	 1 files changed, 9 insertions(+), 4 deletions(-)
#
diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c
index 0995430..0ae9ced 100644
--- a/drivers/serial/8250.c
+++ b/drivers/serial/8250.c
@@ -2240,10 +2252,14 @@ serial8250_console_write(struct console 
 
 	touch_nmi_watchdog();
 
-	if (oops_in_progress) {
-		locked = spin_trylock_irqsave(&up->port.lock, flags);
+	local_irq_save(flags);
+	if (up->port.sysrq) {
+		/* serial8250_handle_port() already took the lock */
+		locked = 0;
+	} else if (oops_in_progress) {
+		locked = spin_trylock(&up->port.lock);
 	} else
-		spin_lock_irqsave(&up->port.lock, flags);
+		spin_lock(&up->port.lock);
 
 	/*
 	 *	First save the IER then disable the interrupts
@@ -2265,7 +2281,8 @@ serial8250_console_write(struct console 
 	serial_out(up, UART_IER, ier);
 
 	if (locked)
-		spin_unlock_irqrestore(&up->port.lock, flags);
+		spin_unlock(&up->port.lock);
+	local_irq_restore(flags);
 }
 
 static int serial8250_console_setup(struct console *co, char *options)


-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core

PATCH: Minimal fix for sysrq on serial console hang

[Alan Cox]

When I originally did this change I used oops_in_progress as a locking
guide. However it turns out there is one other place that turns all the
locking on its head and that is sysrq.

The fix below is a minimal fix to avoid this hanging and worst case may
lead to the very rare stuck character as it did pre 2.6.16 anyway but
only when using sysrq [which right now hangs the box]

The right fix is probably to unlock on the sysrq path and relock and
reconfigure the uart but thats more complex and not low risk at this
point in time although I will revisit it after (or during KS/OLS)
depending on rmk's view.

Signed-off-by: Alan Cox <alan@redhat.com>

diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.18-rc1/drivers/serial/8250.c linux-2.6.18-rc1/drivers/serial/8250.c
--- linux.vanilla-2.6.18-rc1/drivers/serial/8250.c	2006-07-12 12:16:47.000000000 +0100
+++ linux-2.6.18-rc1/drivers/serial/8250.c	2006-07-12 12:21:10.000000000 +0100
@@ -2240,10 +2240,12 @@
 
 	touch_nmi_watchdog();
 
-	if (oops_in_progress) {
-		locked = spin_trylock_irqsave(&up->port.lock, flags);
-	} else
-		spin_lock_irqsave(&up->port.lock, flags);
+	/*
+	 *	This can occur during an oops, in which case we want to
+	 *	do our best, or sysrq which is hairier and eventually needs
+	 *	a nicer solution
+	 */
+	locked = spin_trylock_irqsave(&up->port.lock, flags);
 
 	/*
 	 *	First save the IER then disable the interrupts

Re: PATCH: Minimal fix for sysrq on serial console hang

[Russell King]

On Wed, Jul 12, 2006 at 03:11:31PM +0100, Alan Cox wrote:
> Ar Mer, 2006-07-12 am 14:47 +0100, ysgrifennodd Russell King:
> > On Wed, Jul 12, 2006 at 02:54:21PM +0100, Alan Cox wrote:
> > > When I originally did this change I used oops_in_progress as a locking
> > > guide. However it turns out there is one other place that turns all the
> > > locking on its head and that is sysrq.
> > 
> > Well, akpm's had a fix in his tree for some time, which he's been
> > pestering me with, so I committed that a few days ago:
> 
> 
> Even better, thanks hadn't seen that go in.

It's not in Linus' tree just yet - I asked Linus to pull stuff last night.
However, since upgrading FC2 to FC5 (which took anaconda some 4 hours
on a 2.6GHz P4) and thereby moving off my ancient git/cogito version,
various scripts broke and didn't produce the nice requests for Linus.
I guess Linus wasn't too happy about pulling my git trees with only
having a diffstat of what was there...

Maybe something will happen tonight though.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core

Re: PATCH: Minimal fix for sysrq on serial console hang

[Alan Cox]

Ar Mer, 2006-07-12 am 14:47 +0100, ysgrifennodd Russell King:
> On Wed, Jul 12, 2006 at 02:54:21PM +0100, Alan Cox wrote:
> > When I originally did this change I used oops_in_progress as a locking
> > guide. However it turns out there is one other place that turns all the
> > locking on its head and that is sysrq.
> 
> Well, akpm's had a fix in his tree for some time, which he's been
> pestering me with, so I committed that a few days ago:


Even better, thanks hadn't seen that go in.