[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <1590560759-21453-5-git-send-email-sumit.garg@linaro.org>
Date: Wed, 27 May 2020 11:55:59 +0530
From: Sumit Garg <sumit.garg@...aro.org>
To: daniel.thompson@...aro.org
Cc: kgdb-bugreport@...ts.sourceforge.net, jason.wessel@...driver.com,
dianders@...omium.org, pmladek@...e.com,
sergey.senozhatsky@...il.com, linux-kernel@...r.kernel.org,
Sumit Garg <sumit.garg@...aro.org>
Subject: [PATCH v3 4/4] kdb: Switch kdb_msg_write() to use safer polling I/O
In kgdb NMI context, calling console handlers isn't safe due to locks
used in those handlers which could lead to a deadlock. Although, using
oops_in_progress increases the chance to bypass locks in most console
handlers but it might not be sufficient enough in case a console uses
more locks (VT/TTY is good example).
Currently when a driver provides both polling I/O and a console then kdb
will output using the console. We can increase robustness by using the
currently active polling I/O driver (which should be lockless) instead
of the corresponding console. For several common cases (e.g. an
embedded system with a single serial port that is used both for console
output and debugger I/O) this will result in no console handler being
used.
Suggested-by: Daniel Thompson <daniel.thompson@...aro.org>
Signed-off-by: Sumit Garg <sumit.garg@...aro.org>
---
drivers/tty/serial/kgdboc.c | 17 ++++++++---------
include/linux/kgdb.h | 2 ++
kernel/debug/kdb/kdb_io.c | 46 +++++++++++++++++++++++++++++++--------------
3 files changed, 42 insertions(+), 23 deletions(-)
diff --git a/drivers/tty/serial/kgdboc.c b/drivers/tty/serial/kgdboc.c
index c9f94fa..6199fe1 100644
--- a/drivers/tty/serial/kgdboc.c
+++ b/drivers/tty/serial/kgdboc.c
@@ -35,7 +35,6 @@ static struct kparam_string kps = {
};
static int kgdboc_use_kms; /* 1 if we use kernel mode switching */
-static struct tty_driver *kgdb_tty_driver;
static int kgdb_tty_line;
#ifdef CONFIG_KDB_KEYBOARD
@@ -154,7 +153,7 @@ static int configure_kgdboc(void)
}
kgdboc_io_ops.is_console = 0;
- kgdb_tty_driver = NULL;
+ kgdboc_io_ops.tty_drv = NULL;
kgdboc_use_kms = 0;
if (strncmp(cptr, "kms,", 4) == 0) {
@@ -178,7 +177,7 @@ static int configure_kgdboc(void)
}
}
- kgdb_tty_driver = p;
+ kgdboc_io_ops.tty_drv = p;
kgdb_tty_line = tty_line;
do_register:
@@ -216,18 +215,18 @@ static int __init init_kgdboc(void)
static int kgdboc_get_char(void)
{
- if (!kgdb_tty_driver)
+ if (!kgdboc_io_ops.tty_drv)
return -1;
- return kgdb_tty_driver->ops->poll_get_char(kgdb_tty_driver,
- kgdb_tty_line);
+ return kgdboc_io_ops.tty_drv->ops->poll_get_char(kgdboc_io_ops.tty_drv,
+ kgdb_tty_line);
}
static void kgdboc_put_char(u8 chr)
{
- if (!kgdb_tty_driver)
+ if (!kgdboc_io_ops.tty_drv)
return;
- kgdb_tty_driver->ops->poll_put_char(kgdb_tty_driver,
- kgdb_tty_line, chr);
+ kgdboc_io_ops.tty_drv->ops->poll_put_char(kgdboc_io_ops.tty_drv,
+ kgdb_tty_line, chr);
}
static int param_set_kgdboc_var(const char *kmessage,
diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h
index b072aeb..05d165d 100644
--- a/include/linux/kgdb.h
+++ b/include/linux/kgdb.h
@@ -275,6 +275,7 @@ struct kgdb_arch {
* for the I/O driver.
* @is_console: 1 if the end device is a console 0 if the I/O device is
* not a console
+ * @tty_drv: Pointer to polling tty driver.
*/
struct kgdb_io {
const char *name;
@@ -285,6 +286,7 @@ struct kgdb_io {
void (*pre_exception) (void);
void (*post_exception) (void);
int is_console;
+ struct tty_driver *tty_drv;
};
extern const struct kgdb_arch arch_kgdb_ops;
diff --git a/kernel/debug/kdb/kdb_io.c b/kernel/debug/kdb/kdb_io.c
index f848482..c2efa52 100644
--- a/kernel/debug/kdb/kdb_io.c
+++ b/kernel/debug/kdb/kdb_io.c
@@ -24,6 +24,7 @@
#include <linux/kgdb.h>
#include <linux/kdb.h>
#include <linux/kallsyms.h>
+#include <linux/tty_driver.h>
#include "kdb_private.h"
#define CMD_BUFLEN 256
@@ -542,13 +543,18 @@ static int kdb_search_string(char *searched, char *searchfor)
return 0;
}
-static void kdb_io_write(char *cp, int len, void (*io_put_char)(u8 ch))
+static void kdb_io_write(char *cp, int len, void (*io_put_char)(u8),
+ struct tty_driver *p, int line,
+ void (*poll_put_char)(struct tty_driver *, int, char))
{
if (len <= 0)
return;
while (len--) {
- io_put_char(*cp);
+ if (io_put_char)
+ io_put_char(*cp);
+ if (poll_put_char)
+ poll_put_char(p, line, *cp);
cp++;
}
}
@@ -561,22 +567,34 @@ static void kdb_msg_write(char *msg, int msg_len)
return;
if (dbg_io_ops && !dbg_io_ops->is_console)
- kdb_io_write(msg, msg_len, dbg_io_ops->write_char);
+ kdb_io_write(msg, msg_len, dbg_io_ops->write_char,
+ NULL, 0, NULL);
for_each_console(c) {
+ int line;
+ struct tty_driver *p;
+
if (!(c->flags & CON_ENABLED))
continue;
- /*
- * While rounding up CPUs via NMIs, its possible that
- * a rounded up CPU maybe holding a console port lock
- * leading to kgdb master CPU stuck in a deadlock during
- * invocation of console write operations. So in order
- * to avoid such a deadlock, enable oops_in_progress
- * prior to invocation of console handlers.
- */
- ++oops_in_progress;
- c->write(c, msg, msg_len);
- --oops_in_progress;
+
+ p = c->device ? c->device(c, &line) : NULL;
+ if (p && dbg_io_ops && p == dbg_io_ops->tty_drv && p->ops &&
+ p->ops->poll_put_char) {
+ kdb_io_write(msg, msg_len, NULL, p, line,
+ p->ops->poll_put_char);
+ } else {
+ /*
+ * While rounding up CPUs via NMIs, its possible that
+ * a rounded up CPU maybe holding a console port lock
+ * leading to kgdb master CPU stuck in a deadlock during
+ * invocation of console write operations. So in order
+ * to avoid such a deadlock, enable oops_in_progress
+ * prior to invocation of console handlers.
+ */
+ ++oops_in_progress;
+ c->write(c, msg, msg_len);
+ --oops_in_progress;
+ }
touch_nmi_watchdog();
}
}
--
2.7.4
Powered by blists - more mailing lists