All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rik van Riel <riel@redhat.com>
To: linux-kernel@vger.kernel.org
Cc: joern@logfs.org, peterz@infradead.org, akpm@linux-foundation.org,
	cxie@redhat.com, Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Jiri Slaby <jslaby@suse.cz>
Subject: [PATCH RFC] sysrq: rcu-ify __handle_sysrq
Date: Wed, 23 Apr 2014 12:53:52 -0400	[thread overview]
Message-ID: <20140423125352.704f9fb2@annuminas.surriel.com> (raw)

Echoing values into /proc/sysrq-trigger seems to be a popular way to
get information out of the kernel. However, dumping information about
thousands of processes, or hundreds of CPUs to serial console can
result in IRQs being blocked for minutes, resulting in various kinds
of cascade failures.

The most common failure is due to interrupts being blocked for a very
long time. This can lead to things like failed IO requests, and other
things the system cannot easily recover from.

This problem is easily fixable by making __handle_sysrq use RCU
instead of spin_lock_irqsave.

This leaves the warning that RCU grace periods have not elapsed for a
long time, but the system will come back from that automatically.

It also leaves sysrq-from-irq-context when the sysrq keys are pressed,
but that is probably desired since people want that to work in situations
where the system is already hosed.

The callers of register_sysrq_key and unregister_sysrq_key appear to be
capable of sleeping.

Signed-off-by: Rik van Riel <riel@redhat.com>
Reported-by: Madper Xie <cxie@redhat.com>
---
 drivers/tty/sysrq.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
index ce396ec..3c61e9b 100644
--- a/drivers/tty/sysrq.c
+++ b/drivers/tty/sysrq.c
@@ -510,9 +510,8 @@ void __handle_sysrq(int key, bool check_mask)
 	struct sysrq_key_op *op_p;
 	int orig_log_level;
 	int i;
-	unsigned long flags;
 
-	spin_lock_irqsave(&sysrq_key_table_lock, flags);
+	rcu_read_lock();
 	/*
 	 * Raise the apparent loglevel to maximum so that the sysrq header
 	 * is shown to provide the user with positive feedback.  We do not
@@ -554,7 +553,7 @@ void __handle_sysrq(int key, bool check_mask)
 		printk("\n");
 		console_loglevel = orig_log_level;
 	}
-	spin_unlock_irqrestore(&sysrq_key_table_lock, flags);
+	rcu_read_unlock();
 }
 
 void handle_sysrq(int key)
@@ -1043,16 +1042,23 @@ static int __sysrq_swap_key_ops(int key, struct sysrq_key_op *insert_op_p,
                                 struct sysrq_key_op *remove_op_p)
 {
 	int retval;
-	unsigned long flags;
 
-	spin_lock_irqsave(&sysrq_key_table_lock, flags);
+	spin_lock(&sysrq_key_table_lock);
 	if (__sysrq_get_key_op(key) == remove_op_p) {
 		__sysrq_put_key_op(key, insert_op_p);
 		retval = 0;
 	} else {
 		retval = -1;
 	}
-	spin_unlock_irqrestore(&sysrq_key_table_lock, flags);
+	spin_unlock(&sysrq_key_table_lock);
+
+	/*
+	 * A concurrent __handle_sysrq eitehr got the old op or the new op.
+	 * Wait for it to go away before returning, so the code for an old
+	 * op is not freed (eg. on module unload) while it is in use.
+	 */
+	synchronize_rcu();
+
 	return retval;
 }
 


             reply	other threads:[~2014-04-23 16:55 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-23 16:53 Rik van Riel [this message]
2014-04-23 20:04 ` [PATCH RFC] sysrq: rcu-ify __handle_sysrq Andrew Morton
2014-04-23 20:44   ` Rik van Riel
2014-04-23 21:39   ` Jiri Kosina
2014-04-23 21:41     ` Andrew Morton
2014-04-23 21:44       ` Jiri Kosina
2014-04-23 21:49         ` Andrew Morton
2014-04-23 21:37 ` Jiri Kosina
2014-04-23 21:42   ` Rik van Riel
2014-04-23 21:51     ` Jiri Kosina
2014-04-24  1:46       ` Paul E. McKenney
2014-04-24 13:04         ` [PATCH RFC] sysrq,rcu: suppress RCU stall warnings while sysrq runs Rik van Riel
2014-04-24 15:16           ` Paul E. McKenney
2014-04-25  5:35           ` Mike Galbraith
2014-04-24  0:52   ` [PATCH RFC] sysrq: rcu-ify __handle_sysrq Jörn Engel
2014-04-24 19:40     ` [PATCH] printk: Print cpu number along with time Jörn Engel
2014-04-24 19:58       ` Greg Kroah-Hartman
2014-04-24 21:23         ` Jörn Engel
2014-04-24 22:12         ` Jiri Kosina
2014-04-24 22:18           ` David Rientjes
2014-04-24 22:21             ` Jiri Kosina
2014-04-24 23:29               ` Jörn Engel
2014-04-24 22:20           ` Greg Kroah-Hartman
2014-04-28 23:40       ` Jörn Engel
2014-04-29  0:22         ` Andrew Morton
2014-06-04 23:15           ` Jörn Engel
2014-06-04 23:28             ` Andrew Morton
2014-06-04 23:49               ` Jörn Engel
2014-09-09 17:16             ` Jörn Engel
2014-09-10 21:26               ` Andrew Morton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140423125352.704f9fb2@annuminas.surriel.com \
    --to=riel@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=cxie@redhat.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=joern@logfs.org \
    --cc=jslaby@suse.cz \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peterz@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.