2020-06-04 10:36:29

by Sumit Garg

[permalink] [raw]
Subject: [PATCH v6 0/4] kdb: Improve console handling

This patch-set is aimed to improve console handling especially when kdb
operates in NMI context.

Brief description of enhancements:
- Add status check for console prior to invoking corresponding handler.
- Fixup to avoid possible deadlock in NMI context due to usage of locks
in the console handlers.
- Prefer usage of polling I/O driver mode (lockless APIs) over invocation
of console handlers.

Changes in v6:
- Replace early_con -> kgdboc_earlycon_io_ops.cons.
- Added Petr's review tag on patch #1, #2 and #3.
- Drop Doug's review tag from patch #4 due to early_con changes which I
think needs a re-review.

Changes in v5:
- Rebased on top of tags/kgdb-5.8-rc1.
- Get rid of kdb_io_write().
- Fixed issue reported by kbuild test robot.
- Remove is_console from kgdboc_earlycon_io_ops as well.
- Misc. comments.
- Added Doug's review tag.

Changes in v4:
- Use dbg_io_ops->write_char() directly instead of passing it as a
function pointer.
- Use "struct console" rather than "struct tty_driver" for comparison.
- Make commit descriptions and comments more informative.
- Add review tag for patch #2.

Changes in v3:
- Split patch to have separate patch for console status check.
- New patch to re-factor kdb message emit code.
- New patch to prefer polling I/O over console mode.
- Add code comments to describe usage of oops_in_progress.

Changes in v2:
- Use oops_in_progress directly instead of bust_spinlocks().

Sumit Garg (4):
kdb: Re-factor kdb_printf() message write code
kdb: Check status of console prior to invoking handlers
kdb: Make kdb_printf() console handling more robust
kdb: Switch to use safer dbg_io_ops over console APIs

drivers/tty/serial/kgdb_nmi.c | 2 +-
drivers/tty/serial/kgdboc.c | 32 +++++++++----------
drivers/usb/early/ehci-dbgp.c | 3 +-
include/linux/kgdb.h | 5 ++-
kernel/debug/kdb/kdb_io.c | 72 ++++++++++++++++++++++++++-----------------
5 files changed, 64 insertions(+), 50 deletions(-)

--
2.7.4


2020-06-04 10:37:25

by Sumit Garg

[permalink] [raw]
Subject: [PATCH v6 4/4] kdb: Switch to use safer dbg_io_ops over console APIs

In kgdb context, calling console handlers aren't safe due to locks used
in those handlers which could in turn 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.

In order to achieve this we need to reverse the order of preference to
use dbg_io_ops (uses polling I/O mode) over console APIs. So we just
store "struct console" that represents debugger I/O in dbg_io_ops and
while emitting kdb messages, skip console that matches dbg_io_ops
console in order to avoid duplicate messages. After this change,
"is_console" param becomes redundant and hence removed.

Suggested-by: Daniel Thompson <[email protected]>
Signed-off-by: Sumit Garg <[email protected]>
---
drivers/tty/serial/kgdb_nmi.c | 2 +-
drivers/tty/serial/kgdboc.c | 32 ++++++++++++++++----------------
drivers/usb/early/ehci-dbgp.c | 3 ++-
include/linux/kgdb.h | 5 ++---
kernel/debug/kdb/kdb_io.c | 4 +++-
5 files changed, 24 insertions(+), 22 deletions(-)

diff --git a/drivers/tty/serial/kgdb_nmi.c b/drivers/tty/serial/kgdb_nmi.c
index 5022447..6004c0c 100644
--- a/drivers/tty/serial/kgdb_nmi.c
+++ b/drivers/tty/serial/kgdb_nmi.c
@@ -50,7 +50,7 @@ static int kgdb_nmi_console_setup(struct console *co, char *options)
* I/O utilities that messages sent to the console will automatically
* be displayed on the dbg_io.
*/
- dbg_io_ops->is_console = true;
+ dbg_io_ops->cons = co;

return 0;
}
diff --git a/drivers/tty/serial/kgdboc.c b/drivers/tty/serial/kgdboc.c
index 4139698..84ffede 100644
--- a/drivers/tty/serial/kgdboc.c
+++ b/drivers/tty/serial/kgdboc.c
@@ -45,7 +45,6 @@ static struct platform_device *kgdboc_pdev;

#if IS_BUILTIN(CONFIG_KGDB_SERIAL_CONSOLE)
static struct kgdb_io kgdboc_earlycon_io_ops;
-static struct console *earlycon;
static int (*earlycon_orig_exit)(struct console *con);
#endif /* IS_BUILTIN(CONFIG_KGDB_SERIAL_CONSOLE) */

@@ -145,7 +144,7 @@ static void kgdboc_unregister_kbd(void)
#if IS_BUILTIN(CONFIG_KGDB_SERIAL_CONSOLE)
static void cleanup_earlycon(void)
{
- if (earlycon)
+ if (kgdboc_earlycon_io_ops.cons)
kgdb_unregister_io_module(&kgdboc_earlycon_io_ops);
}
#else /* !IS_BUILTIN(CONFIG_KGDB_SERIAL_CONSOLE) */
@@ -178,7 +177,7 @@ static int configure_kgdboc(void)
goto noconfig;
}

- kgdboc_io_ops.is_console = 0;
+ kgdboc_io_ops.cons = NULL;
kgdb_tty_driver = NULL;

kgdboc_use_kms = 0;
@@ -198,7 +197,7 @@ static int configure_kgdboc(void)
int idx;
if (cons->device && cons->device(cons, &idx) == p &&
idx == tty_line) {
- kgdboc_io_ops.is_console = 1;
+ kgdboc_io_ops.cons = cons;
break;
}
}
@@ -433,7 +432,8 @@ static int kgdboc_earlycon_get_char(void)
{
char c;

- if (!earlycon->read(earlycon, &c, 1))
+ if (!kgdboc_earlycon_io_ops.cons->read(kgdboc_earlycon_io_ops.cons,
+ &c, 1))
return NO_POLL_CHAR;

return c;
@@ -441,7 +441,8 @@ static int kgdboc_earlycon_get_char(void)

static void kgdboc_earlycon_put_char(u8 chr)
{
- earlycon->write(earlycon, &chr, 1);
+ kgdboc_earlycon_io_ops.cons->write(kgdboc_earlycon_io_ops.cons, &chr,
+ 1);
}

static void kgdboc_earlycon_pre_exp_handler(void)
@@ -461,7 +462,7 @@ static void kgdboc_earlycon_pre_exp_handler(void)
* boot if we detect this case.
*/
for_each_console(con)
- if (con == earlycon)
+ if (con == kgdboc_earlycon_io_ops.cons)
return;

already_warned = true;
@@ -484,25 +485,25 @@ static int kgdboc_earlycon_deferred_exit(struct console *con)

static void kgdboc_earlycon_deinit(void)
{
- if (!earlycon)
+ if (!kgdboc_earlycon_io_ops.cons)
return;

- if (earlycon->exit == kgdboc_earlycon_deferred_exit)
+ if (kgdboc_earlycon_io_ops.cons->exit == kgdboc_earlycon_deferred_exit)
/*
* kgdboc_earlycon is exiting but original boot console exit
* was never called (AKA kgdboc_earlycon_deferred_exit()
* didn't ever run). Undo our trap.
*/
- earlycon->exit = earlycon_orig_exit;
- else if (earlycon->exit)
+ kgdboc_earlycon_io_ops.cons->exit = earlycon_orig_exit;
+ else if (kgdboc_earlycon_io_ops.cons->exit)
/*
* We skipped calling the exit() routine so we could try to
* keep using the boot console even after it went away. We're
* finally done so call the function now.
*/
- earlycon->exit(earlycon);
+ kgdboc_earlycon_io_ops.cons->exit(kgdboc_earlycon_io_ops.cons);

- earlycon = NULL;
+ kgdboc_earlycon_io_ops.cons = NULL;
}

static struct kgdb_io kgdboc_earlycon_io_ops = {
@@ -511,7 +512,6 @@ static struct kgdb_io kgdboc_earlycon_io_ops = {
.write_char = kgdboc_earlycon_put_char,
.pre_exception = kgdboc_earlycon_pre_exp_handler,
.deinit = kgdboc_earlycon_deinit,
- .is_console = true,
};

#define MAX_CONSOLE_NAME_LEN (sizeof((struct console *) 0)->name)
@@ -557,10 +557,10 @@ static int __init kgdboc_earlycon_init(char *opt)
goto unlock;
}

- earlycon = con;
+ kgdboc_earlycon_io_ops.cons = con;
pr_info("Going to register kgdb with earlycon '%s'\n", con->name);
if (kgdb_register_io_module(&kgdboc_earlycon_io_ops) != 0) {
- earlycon = NULL;
+ kgdboc_earlycon_io_ops.cons = NULL;
pr_info("Failed to register kgdb with earlycon\n");
} else {
/* Trap exit so we can keep earlycon longer if needed. */
diff --git a/drivers/usb/early/ehci-dbgp.c b/drivers/usb/early/ehci-dbgp.c
index ea0d531..775cf70 100644
--- a/drivers/usb/early/ehci-dbgp.c
+++ b/drivers/usb/early/ehci-dbgp.c
@@ -1058,7 +1058,8 @@ static int __init kgdbdbgp_parse_config(char *str)
kgdbdbgp_wait_time = simple_strtoul(ptr, &ptr, 10);
}
kgdb_register_io_module(&kgdbdbgp_io_ops);
- kgdbdbgp_io_ops.is_console = early_dbgp_console.index != -1;
+ if (early_dbgp_console.index != -1)
+ kgdbdbgp_io_ops.cons = &early_dbgp_console;

return 0;
}
diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h
index c62d764..529116b 100644
--- a/include/linux/kgdb.h
+++ b/include/linux/kgdb.h
@@ -276,8 +276,7 @@ struct kgdb_arch {
* the I/O driver.
* @post_exception: Pointer to a function that will do any cleanup work
* 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
+ * @cons: valid if the I/O device is a console; else NULL.
*/
struct kgdb_io {
const char *name;
@@ -288,7 +287,7 @@ struct kgdb_io {
void (*deinit) (void);
void (*pre_exception) (void);
void (*post_exception) (void);
- int is_console;
+ struct console *cons;
};

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 0e4f2ed..683a799 100644
--- a/kernel/debug/kdb/kdb_io.c
+++ b/kernel/debug/kdb/kdb_io.c
@@ -549,7 +549,7 @@ static void kdb_msg_write(const char *msg, int msg_len)
if (msg_len == 0)
return;

- if (dbg_io_ops && !dbg_io_ops->is_console) {
+ if (dbg_io_ops) {
const char *cp = msg;
int len = msg_len;

@@ -562,6 +562,8 @@ static void kdb_msg_write(const char *msg, int msg_len)
for_each_console(c) {
if (!(c->flags & CON_ENABLED))
continue;
+ if (c == dbg_io_ops->cons)
+ continue;
/*
* Set oops_in_progress to encourage the console drivers to
* disregard their internal spin locks: in the current calling
--
2.7.4

2020-06-04 10:38:31

by Sumit Garg

[permalink] [raw]
Subject: [PATCH v6 2/4] kdb: Check status of console prior to invoking handlers

Check if a console is enabled prior to invoking corresponding write
handler.

Suggested-by: Sergey Senozhatsky <[email protected]>
Signed-off-by: Sumit Garg <[email protected]>
Reviewed-by: Daniel Thompson <[email protected]>
Reviewed-by: Douglas Anderson <[email protected]>
Revieved-by: Petr Mladek <[email protected]>
---
kernel/debug/kdb/kdb_io.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/kernel/debug/kdb/kdb_io.c b/kernel/debug/kdb/kdb_io.c
index 2d42a02..58b7d25 100644
--- a/kernel/debug/kdb/kdb_io.c
+++ b/kernel/debug/kdb/kdb_io.c
@@ -560,6 +560,8 @@ static void kdb_msg_write(const char *msg, int msg_len)
}

for_each_console(c) {
+ if (!(c->flags & CON_ENABLED))
+ continue;
c->write(c, msg, msg_len);
touch_nmi_watchdog();
}
--
2.7.4

2020-06-04 21:15:41

by Doug Anderson

[permalink] [raw]
Subject: Re: [PATCH v6 4/4] kdb: Switch to use safer dbg_io_ops over console APIs

Hi,

On Thu, Jun 4, 2020 at 3:02 AM Sumit Garg <[email protected]> wrote:
>
> @@ -433,7 +432,8 @@ static int kgdboc_earlycon_get_char(void)
> {
> char c;
>
> - if (!earlycon->read(earlycon, &c, 1))
> + if (!kgdboc_earlycon_io_ops.cons->read(kgdboc_earlycon_io_ops.cons,
> + &c, 1))
> return NO_POLL_CHAR;
>
> return c;
> @@ -441,7 +441,8 @@ static int kgdboc_earlycon_get_char(void)
>
> static void kgdboc_earlycon_put_char(u8 chr)
> {
> - earlycon->write(earlycon, &chr, 1);
> + kgdboc_earlycon_io_ops.cons->write(kgdboc_earlycon_io_ops.cons, &chr,
> + 1);
> }

The get_char / put_char functions are pretty unwieldy now. If it were
me I would have done:

struct console *con = kgdboc_earlycon_io_ops.cons;

...and then used it so the lines didn't wrap in such a terrible way. ;-)

I'm not sure if I'd spin just for that, though.

Reviewed-by: Douglas Anderson <[email protected]>

-Doug

2020-06-05 04:02:40

by Sergey Senozhatsky

[permalink] [raw]
Subject: Re: [PATCH v6 0/4] kdb: Improve console handling

On (20/06/04 15:31), Sumit Garg wrote:
>
> drivers/tty/serial/kgdb_nmi.c | 2 +-
> drivers/tty/serial/kgdboc.c | 32 +++++++++----------
> drivers/usb/early/ehci-dbgp.c | 3 +-
> include/linux/kgdb.h | 5 ++-
> kernel/debug/kdb/kdb_io.c | 72 ++++++++++++++++++++++++++-----------------
> 5 files changed, 64 insertions(+), 50 deletions(-)

Reviewed-by: Sergey Senozhatsky <[email protected]>

-ss

2020-06-05 09:56:47

by Petr Mladek

[permalink] [raw]
Subject: Re: [PATCH v6 4/4] kdb: Switch to use safer dbg_io_ops over console APIs

On Thu 2020-06-04 15:31:19, Sumit Garg wrote:
> In kgdb context, calling console handlers aren't safe due to locks used
> in those handlers which could in turn 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.
>
> In order to achieve this we need to reverse the order of preference to
> use dbg_io_ops (uses polling I/O mode) over console APIs. So we just
> store "struct console" that represents debugger I/O in dbg_io_ops and
> while emitting kdb messages, skip console that matches dbg_io_ops
> console in order to avoid duplicate messages. After this change,
> "is_console" param becomes redundant and hence removed.
>
> Suggested-by: Daniel Thompson <[email protected]>
> Signed-off-by: Sumit Garg <[email protected]>

Looks good to me now:

Reviewed-by: Petr Mladek <[email protected]>

Best Regards,
Petr

2020-06-26 12:42:49

by Daniel Thompson

[permalink] [raw]
Subject: Re: [PATCH v6 4/4] kdb: Switch to use safer dbg_io_ops over console APIs

Hi Greg

This patch touches some kgdb related code in both serial and usb trees.

Any objections to my queuing this via the kgdb tree?

After testing I've concluded that this, and its friends (which only
touch kgdb), fix enough problems that I plan to queue it for v5.8.


Daniel.


On Thu, Jun 04, 2020 at 03:31:19PM +0530, Sumit Garg wrote:
> In kgdb context, calling console handlers aren't safe due to locks used
> in those handlers which could in turn 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.
>
> In order to achieve this we need to reverse the order of preference to
> use dbg_io_ops (uses polling I/O mode) over console APIs. So we just
> store "struct console" that represents debugger I/O in dbg_io_ops and
> while emitting kdb messages, skip console that matches dbg_io_ops
> console in order to avoid duplicate messages. After this change,
> "is_console" param becomes redundant and hence removed.
>
> Suggested-by: Daniel Thompson <[email protected]>
> Signed-off-by: Sumit Garg <[email protected]>
> ---
> drivers/tty/serial/kgdb_nmi.c | 2 +-
> drivers/tty/serial/kgdboc.c | 32 ++++++++++++++++----------------
> drivers/usb/early/ehci-dbgp.c | 3 ++-
> include/linux/kgdb.h | 5 ++---
> kernel/debug/kdb/kdb_io.c | 4 +++-
> 5 files changed, 24 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/tty/serial/kgdb_nmi.c b/drivers/tty/serial/kgdb_nmi.c
> index 5022447..6004c0c 100644
> --- a/drivers/tty/serial/kgdb_nmi.c
> +++ b/drivers/tty/serial/kgdb_nmi.c
> @@ -50,7 +50,7 @@ static int kgdb_nmi_console_setup(struct console *co, char *options)
> * I/O utilities that messages sent to the console will automatically
> * be displayed on the dbg_io.
> */
> - dbg_io_ops->is_console = true;
> + dbg_io_ops->cons = co;
>
> return 0;
> }
> diff --git a/drivers/tty/serial/kgdboc.c b/drivers/tty/serial/kgdboc.c
> index 4139698..84ffede 100644
> --- a/drivers/tty/serial/kgdboc.c
> +++ b/drivers/tty/serial/kgdboc.c
> @@ -45,7 +45,6 @@ static struct platform_device *kgdboc_pdev;
>
> #if IS_BUILTIN(CONFIG_KGDB_SERIAL_CONSOLE)
> static struct kgdb_io kgdboc_earlycon_io_ops;
> -static struct console *earlycon;
> static int (*earlycon_orig_exit)(struct console *con);
> #endif /* IS_BUILTIN(CONFIG_KGDB_SERIAL_CONSOLE) */
>
> @@ -145,7 +144,7 @@ static void kgdboc_unregister_kbd(void)
> #if IS_BUILTIN(CONFIG_KGDB_SERIAL_CONSOLE)
> static void cleanup_earlycon(void)
> {
> - if (earlycon)
> + if (kgdboc_earlycon_io_ops.cons)
> kgdb_unregister_io_module(&kgdboc_earlycon_io_ops);
> }
> #else /* !IS_BUILTIN(CONFIG_KGDB_SERIAL_CONSOLE) */
> @@ -178,7 +177,7 @@ static int configure_kgdboc(void)
> goto noconfig;
> }
>
> - kgdboc_io_ops.is_console = 0;
> + kgdboc_io_ops.cons = NULL;
> kgdb_tty_driver = NULL;
>
> kgdboc_use_kms = 0;
> @@ -198,7 +197,7 @@ static int configure_kgdboc(void)
> int idx;
> if (cons->device && cons->device(cons, &idx) == p &&
> idx == tty_line) {
> - kgdboc_io_ops.is_console = 1;
> + kgdboc_io_ops.cons = cons;
> break;
> }
> }
> @@ -433,7 +432,8 @@ static int kgdboc_earlycon_get_char(void)
> {
> char c;
>
> - if (!earlycon->read(earlycon, &c, 1))
> + if (!kgdboc_earlycon_io_ops.cons->read(kgdboc_earlycon_io_ops.cons,
> + &c, 1))
> return NO_POLL_CHAR;
>
> return c;
> @@ -441,7 +441,8 @@ static int kgdboc_earlycon_get_char(void)
>
> static void kgdboc_earlycon_put_char(u8 chr)
> {
> - earlycon->write(earlycon, &chr, 1);
> + kgdboc_earlycon_io_ops.cons->write(kgdboc_earlycon_io_ops.cons, &chr,
> + 1);
> }
>
> static void kgdboc_earlycon_pre_exp_handler(void)
> @@ -461,7 +462,7 @@ static void kgdboc_earlycon_pre_exp_handler(void)
> * boot if we detect this case.
> */
> for_each_console(con)
> - if (con == earlycon)
> + if (con == kgdboc_earlycon_io_ops.cons)
> return;
>
> already_warned = true;
> @@ -484,25 +485,25 @@ static int kgdboc_earlycon_deferred_exit(struct console *con)
>
> static void kgdboc_earlycon_deinit(void)
> {
> - if (!earlycon)
> + if (!kgdboc_earlycon_io_ops.cons)
> return;
>
> - if (earlycon->exit == kgdboc_earlycon_deferred_exit)
> + if (kgdboc_earlycon_io_ops.cons->exit == kgdboc_earlycon_deferred_exit)
> /*
> * kgdboc_earlycon is exiting but original boot console exit
> * was never called (AKA kgdboc_earlycon_deferred_exit()
> * didn't ever run). Undo our trap.
> */
> - earlycon->exit = earlycon_orig_exit;
> - else if (earlycon->exit)
> + kgdboc_earlycon_io_ops.cons->exit = earlycon_orig_exit;
> + else if (kgdboc_earlycon_io_ops.cons->exit)
> /*
> * We skipped calling the exit() routine so we could try to
> * keep using the boot console even after it went away. We're
> * finally done so call the function now.
> */
> - earlycon->exit(earlycon);
> + kgdboc_earlycon_io_ops.cons->exit(kgdboc_earlycon_io_ops.cons);
>
> - earlycon = NULL;
> + kgdboc_earlycon_io_ops.cons = NULL;
> }
>
> static struct kgdb_io kgdboc_earlycon_io_ops = {
> @@ -511,7 +512,6 @@ static struct kgdb_io kgdboc_earlycon_io_ops = {
> .write_char = kgdboc_earlycon_put_char,
> .pre_exception = kgdboc_earlycon_pre_exp_handler,
> .deinit = kgdboc_earlycon_deinit,
> - .is_console = true,
> };
>
> #define MAX_CONSOLE_NAME_LEN (sizeof((struct console *) 0)->name)
> @@ -557,10 +557,10 @@ static int __init kgdboc_earlycon_init(char *opt)
> goto unlock;
> }
>
> - earlycon = con;
> + kgdboc_earlycon_io_ops.cons = con;
> pr_info("Going to register kgdb with earlycon '%s'\n", con->name);
> if (kgdb_register_io_module(&kgdboc_earlycon_io_ops) != 0) {
> - earlycon = NULL;
> + kgdboc_earlycon_io_ops.cons = NULL;
> pr_info("Failed to register kgdb with earlycon\n");
> } else {
> /* Trap exit so we can keep earlycon longer if needed. */
> diff --git a/drivers/usb/early/ehci-dbgp.c b/drivers/usb/early/ehci-dbgp.c
> index ea0d531..775cf70 100644
> --- a/drivers/usb/early/ehci-dbgp.c
> +++ b/drivers/usb/early/ehci-dbgp.c
> @@ -1058,7 +1058,8 @@ static int __init kgdbdbgp_parse_config(char *str)
> kgdbdbgp_wait_time = simple_strtoul(ptr, &ptr, 10);
> }
> kgdb_register_io_module(&kgdbdbgp_io_ops);
> - kgdbdbgp_io_ops.is_console = early_dbgp_console.index != -1;
> + if (early_dbgp_console.index != -1)
> + kgdbdbgp_io_ops.cons = &early_dbgp_console;
>
> return 0;
> }
> diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h
> index c62d764..529116b 100644
> --- a/include/linux/kgdb.h
> +++ b/include/linux/kgdb.h
> @@ -276,8 +276,7 @@ struct kgdb_arch {
> * the I/O driver.
> * @post_exception: Pointer to a function that will do any cleanup work
> * 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
> + * @cons: valid if the I/O device is a console; else NULL.
> */
> struct kgdb_io {
> const char *name;
> @@ -288,7 +287,7 @@ struct kgdb_io {
> void (*deinit) (void);
> void (*pre_exception) (void);
> void (*post_exception) (void);
> - int is_console;
> + struct console *cons;
> };
>
> 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 0e4f2ed..683a799 100644
> --- a/kernel/debug/kdb/kdb_io.c
> +++ b/kernel/debug/kdb/kdb_io.c
> @@ -549,7 +549,7 @@ static void kdb_msg_write(const char *msg, int msg_len)
> if (msg_len == 0)
> return;
>
> - if (dbg_io_ops && !dbg_io_ops->is_console) {
> + if (dbg_io_ops) {
> const char *cp = msg;
> int len = msg_len;
>
> @@ -562,6 +562,8 @@ static void kdb_msg_write(const char *msg, int msg_len)
> for_each_console(c) {
> if (!(c->flags & CON_ENABLED))
> continue;
> + if (c == dbg_io_ops->cons)
> + continue;
> /*
> * Set oops_in_progress to encourage the console drivers to
> * disregard their internal spin locks: in the current calling
> --
> 2.7.4

2020-06-26 14:20:26

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v6 4/4] kdb: Switch to use safer dbg_io_ops over console APIs

On Fri, Jun 26, 2020 at 01:13:48PM +0100, Daniel Thompson wrote:
> Hi Greg
>
> This patch touches some kgdb related code in both serial and usb trees.
>
> Any objections to my queuing this via the kgdb tree?
>
> After testing I've concluded that this, and its friends (which only
> touch kgdb), fix enough problems that I plan to queue it for v5.8.

No objections at all:

Acked-by: Greg Kroah-Hartman <[email protected]>