Hi,

The following 3 patches get rid of 'tty' argument in SysRq handlers as
nobody is using it. If noone objects I'd like to get it merged through
my tree.

Thanks!

--
Dmitry

--

[ message continues ]

Since handle_sysrq() does not take tty as argument anymore we can
drop it from usb_serial_handle_sysrq_char() as well.

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 drivers/usb/serial/ftdi_sio.c |    2 +-
 drivers/usb/serial/generic.c  |    5 ++---
 drivers/usb/serial/pl2303.c   |    2 +-
 include/linux/usb/serial.h    |    3 +--
 4 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index e298dc4..8aecfe5 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -1827,7 +1827,7 @@ static int ftdi_process_packet(struct tty_struct *tty,
 
 	if (port->port.console && port->sysrq) {
 		for (i = 0; i < len; i++, ch++) {
-			if (!usb_serial_handle_sysrq_char(tty, port, *ch))
+			if (!usb_serial_handle_sysrq_char(port, *ch))
 				tty_insert_flip_char(tty, *ch, flag);
 		}
 	} else {
diff --git a/drivers/usb/serial/generic.c b/drivers/usb/serial/generic.c
index 9587c22..dee73b9 100644
--- a/drivers/usb/serial/generic.c
+++ b/drivers/usb/serial/generic.c
@@ -338,7 +338,7 @@ void usb_serial_generic_process_read_urb(struct urb *urb)
 		tty_insert_flip_string(tty, ch, urb->actual_length);
 	else {
 		for (i = 0; i < urb->actual_length; i++, ch++) {
-			if (!usb_serial_handle_sysrq_char(tty, port, *ch))
+			if (!usb_serial_handle_sysrq_char(port, *ch))
 				tty_insert_flip_char(tty, *ch, TTY_NORMAL);
 		}
 	}
@@ -443,8 +443,7 @@ void usb_serial_generic_unthrottle(struct tty_struct *tty)
 EXPORT_SYMBOL_GPL(usb_serial_generic_unthrottle);
 
 #ifdef CONFIG_MAGIC_SYSRQ
-int usb_serial_handle_sysrq_char(struct tty_struct *tty,
-			struct usb_serial_port *port, unsigned int ch)
+int usb_serial_handle_sysrq_char(struct usb_serial_port *port, unsigned int ch)
 {
 	if (port->sysrq && port->port.console) {
 		if (ch && time_before(jiffies, port->sysrq)) {
diff --git a/drivers/usb/serial/pl2303.c b/drivers/usb/serial/pl2303.c
index 6b60018..34ad7b3 100644
--- ...
[ message continues ]

Sysrq operations do not accept tty argument anymore so no need to pass
it to us.

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 arch/ia64/hp/sim/simserial.c    |    2 +-
 arch/um/drivers/mconsole_kern.c |    2 +-
 drivers/char/hangcheck-timer.c  |    2 +-
 drivers/char/hvc_console.c      |    2 +-
 drivers/char/hvsi.c             |    2 +-
 drivers/char/sysrq.c            |   11 +++++------
 drivers/s390/char/ctrlchar.c    |    4 +---
 drivers/serial/sn_console.c     |    2 +-
 drivers/usb/serial/generic.c    |    2 +-
 drivers/xen/manage.c            |    2 +-
 include/linux/serial_core.h     |    2 +-
 include/linux/sysrq.h           |   11 ++++-------
 kernel/debug/kdb/kdb_main.c     |    2 +-
 13 files changed, 20 insertions(+), 26 deletions(-)

diff --git a/arch/ia64/hp/sim/simserial.c b/arch/ia64/hp/sim/simserial.c
index 2bef526..1e8d71a 100644
--- a/arch/ia64/hp/sim/simserial.c
+++ b/arch/ia64/hp/sim/simserial.c
@@ -149,7 +149,7 @@ static  void receive_chars(struct tty_struct *tty)
 						ch = ia64_ssc(0, 0, 0, 0,
 							      SSC_GETCHAR);
 					while (!ch);
-					handle_sysrq(ch, NULL);
+					handle_sysrq(ch);
 				}
 #endif
 				seen_esc = 0;
diff --git a/arch/um/drivers/mconsole_kern.c b/arch/um/drivers/mconsole_kern.c
index de317d0..ebc6807 100644
--- a/arch/um/drivers/mconsole_kern.c
+++ b/arch/um/drivers/mconsole_kern.c
@@ -690,7 +690,7 @@ static void with_console(struct mc_request *req, void (*proc)(void *),
 static void sysrq_proc(void *arg)
 {
 	char *op = arg;
-	handle_sysrq(*op, NULL);
+	handle_sysrq(*op);
 }
 
 void mconsole_sysrq(struct mc_request *req)
diff --git a/drivers/char/hangcheck-timer.c b/drivers/char/hangcheck-timer.c
index e024972..f953c96 100644
--- a/drivers/char/hangcheck-timer.c
+++ b/drivers/char/hangcheck-timer.c
@@ -159,7 +159,7 @@ static void hangcheck_fire(unsigned long data)
 		if (hangcheck_dump_tasks) {
 			printk(KERN_CRIT "Hangcheck: Task state:\n");
 #ifdef CONFIG_MAGIC_SYSRQ
-			handle_sysrq('t', ...
[ message continues ]

Acked-by: Jason Wessel <jason.wessel@windriver.com>

--

[ message continues ]

Looks fine to me.  The kernel debugger was passing NULL anyway because
A) it was not going to be easy to figure out the tty at the time, and B)
the sysrq was not using the tty at the lowest level.

Acked-by: Jason Wessel <jason.wessel@windriver.com>

--

[ message continues ]

Noone is using tty argument so let's get rid of it.

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 arch/arm/kernel/etm.c           |    2 +-
 arch/powerpc/xmon/xmon.c        |    5 ++---
 arch/sparc/kernel/process_64.c  |    2 +-
 drivers/char/sysrq.c            |   42 ++++++++++++++++++++-------------------
 drivers/gpu/drm/drm_fb_helper.c |    2 +-
 drivers/net/ibm_newemac/debug.c |    2 +-
 include/linux/sysrq.h           |    6 +++++-
 kernel/power/poweroff.c         |    2 +-
 8 files changed, 33 insertions(+), 30 deletions(-)

diff --git a/arch/arm/kernel/etm.c b/arch/arm/kernel/etm.c
index 8277539..7647593 100644
--- a/arch/arm/kernel/etm.c
+++ b/arch/arm/kernel/etm.c
@@ -230,7 +230,7 @@ static void etm_dump(void)
 	etb_lock(t);
 }
 
-static void sysrq_etm_dump(int key, struct tty_struct *tty)
+static void sysrq_etm_dump(int key)
 {
 	dev_dbg(tracer.dev, "Dumping ETB buffer\n");
 	etm_dump();
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index 8bad7d5..654e155 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -2728,15 +2728,14 @@ static void xmon_init(int enable)
 }
 
 #ifdef CONFIG_MAGIC_SYSRQ
-static void sysrq_handle_xmon(int key, struct tty_struct *tty) 
+static void sysrq_handle_xmon(int key)
 {
 	/* ensure xmon is enabled */
 	xmon_init(1);
 	debugger(get_irq_regs());
 }
 
-static struct sysrq_key_op sysrq_xmon_op = 
-{
+static struct sysrq_key_op sysrq_xmon_op = {
 	.handler =	sysrq_handle_xmon,
 	.help_msg =	"Xmon",
 	.action_msg =	"Entering xmon",
diff --git a/arch/sparc/kernel/process_64.c b/arch/sparc/kernel/process_64.c
index dbe81a3..25b01b4 100644
--- a/arch/sparc/kernel/process_64.c
+++ b/arch/sparc/kernel/process_64.c
@@ -303,7 +303,7 @@ void arch_trigger_all_cpu_backtrace(void)
 
 #ifdef CONFIG_MAGIC_SYSRQ
 
-static void sysrq_handle_globreg(int key, struct tty_struct *tty)
+static void sysrq_handle_globreg(int key)
 {
 	arch_trigger_all_cpu_backtrace();
 }
diff --git ...
[ message continues ]

While talking about sysrq...

I noticed that serial drivers call sysrq with the uart lock held.

I've seen deadlocks caused by that, for example, when entering the
debugger, it waits forever for a CPU which is itself waiting on the UART
lock.

I have a patch to drop the lock in serial_core.h, I'll post that
tomorrow hopefully, just checking if there's any objection there ? The
serial drivers might need to be audited a bit to make sure they cope
with the lock being dropped and re-acquired around the sysrq call.

Cheers,
Ben.
 

--

[ message continues ]

Fundamentally - no. However the impact it has on a lot of the drivers
will be significant and you'll be submitting a huge patch pile to fix up
all the locking assumptions (for one it means port->tty might change

Architecturally I think it would make more sense to add a new sysrq
helper which merely sets a flag, and check that flag at the end of the IRQ
when dropping the lock anyway.

Otherwise it'll be a huge amount of work to even build test all those
consoles.

Alan
--

[ message continues ]

Right. That's nasty. I think we need somewhat to break the loop when
that happens as if we were getting a new interrupt to some extent.


Interesting idea. That does mean that multiple sysrq in one interrupt

Right. Better to have a way where we can fix them one at a time. I'll
look into it. Thanks.

Cheers,
Ben.


--

[ message continues ]

The usb serial has the same kind of problem with all sorts of locks.  I
had resorted to using a tasklet or workqueue to get the sysrq to execute
just outside the usb serial driver context.  This work never made it
upstream because it was part of another series, but the reference is:
http://lkml.org/lkml/2010/3/9/17

For the standard serial you might be able to get away with dropping the
lock for the sysrq.

Did you have a particular way to trigger the problem or was it just
completely random, because I don't know that I have observed this
behavior with the typical 8250 driver.


--

[ message continues ]

Yes, I do actually :-) A 64-way machine and a bunch of processes
flooding the console will do it just fine :-)

Cheers,
Ben.


--

[ message continues ]

On Wed, 04 Aug 2010 00:58:55 -0700

Looks good to me.
--

[ message continues ]

Nice job.

No objection from me for you to take this through your tree:
	Acked-by: Greg Kroah-Hartman <gregkh@suse.de>

thanks,

greg k-h
--

[ message continues ]