From c67c10a67f6b2edcc7804317947cbfdeab71048c Mon Sep 17 00:00:00 2001 From: Douglas Anderson Date: Sat, 9 Nov 2019 11:16:41 -0800 Subject: kdb: kdb_current_regs should be private As of the patch ("MIPS: kdb: Remove old workaround for backtracing on other CPUs") there is no reason for kdb_current_regs to be in the public "kdb.h". Let's move it next to kdb_current_task. Signed-off-by: Douglas Anderson Link: https://lore.kernel.org/r/20191109111623.2.Iadbfb484e90b557cc4b5ac9890bfca732cd99d77@changeid Signed-off-by: Daniel Thompson --- kernel/debug/kdb/kdb_private.h | 1 + 1 file changed, 1 insertion(+) (limited to 'kernel') diff --git a/kernel/debug/kdb/kdb_private.h b/kernel/debug/kdb/kdb_private.h index 55d052061ef9..e829b22f3946 100644 --- a/kernel/debug/kdb/kdb_private.h +++ b/kernel/debug/kdb/kdb_private.h @@ -242,6 +242,7 @@ extern void debug_kusage(void); extern void kdb_set_current_task(struct task_struct *); extern struct task_struct *kdb_current_task; +extern struct pt_regs *kdb_current_regs; #ifdef CONFIG_KDB_KEYBOARD extern void kdb_kbd_cleanup_state(void); -- cgit v1.2.3 From a8649fb0a8c1de9c842c9c492f189cabbc468272 Mon Sep 17 00:00:00 2001 From: Douglas Anderson Date: Sat, 9 Nov 2019 11:16:42 -0800 Subject: kdb: kdb_current_task shouldn't be exported The kdb_current_task variable has been declared in "kernel/debug/kdb/kdb_private.h" since 2010 when kdb was added to the mainline kernel. This is not a public header. There should be no reason that kdb_current_task should be exported and there are no in-kernel users that need it. Remove the export. Signed-off-by: Douglas Anderson Link: https://lore.kernel.org/r/20191109111623.3.I14b22b5eb15ca8f3812ab33e96621231304dc1f7@changeid Signed-off-by: Daniel Thompson --- kernel/debug/kdb/kdb_main.c | 1 - 1 file changed, 1 deletion(-) (limited to 'kernel') diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c index 4567fe998c30..4d44b3746836 100644 --- a/kernel/debug/kdb/kdb_main.c +++ b/kernel/debug/kdb/kdb_main.c @@ -73,7 +73,6 @@ int kdb_nextline = 1; int kdb_state; /* General KDB state */ struct task_struct *kdb_current_task; -EXPORT_SYMBOL(kdb_current_task); struct pt_regs *kdb_current_regs; const char *kdb_diemsg; -- cgit v1.2.3 From 9441d5f6b77770ee388884f04b14a99b028a15e6 Mon Sep 17 00:00:00 2001 From: Douglas Anderson Date: Sat, 9 Nov 2019 11:16:43 -0800 Subject: kdb: Gid rid of implicit setting of the current task / regs Some (but not all?) of the kdb backtrace paths would cause the kdb_current_task and kdb_current_regs to remain changed. As discussed in a review of a previous patch [1], this doesn't seem intuitive, so let's fix that. ...but, it turns out that there's actually no longer any reason to set the current task / current regs while backtracing anymore anyway. As of commit 2277b492582d ("kdb: Fix stack crawling on 'running' CPUs that aren't the master") if we're backtracing on a task running on a CPU we ask that CPU to do the backtrace itself. Linux can do that without anything fancy. If we're doing backtrace on a sleeping task we can also do that fine without updating globals. So this patch mostly just turns into deleting a bunch of code. [1] https://lore.kernel.org/r/20191010150735.dhrj3pbjgmjrdpwr@holly.lan Signed-off-by: Douglas Anderson Link: https://lore.kernel.org/r/20191109111624.4.Ibc3d982bbeb9e46872d43973ba808cd4c79537c7@changeid Signed-off-by: Daniel Thompson --- kernel/debug/kdb/kdb_bt.c | 8 +------- kernel/debug/kdb/kdb_main.c | 2 +- kernel/debug/kdb/kdb_private.h | 1 - 3 files changed, 2 insertions(+), 9 deletions(-) (limited to 'kernel') diff --git a/kernel/debug/kdb/kdb_bt.c b/kernel/debug/kdb/kdb_bt.c index 4af48ac53625..3de0cc780c16 100644 --- a/kernel/debug/kdb/kdb_bt.c +++ b/kernel/debug/kdb/kdb_bt.c @@ -119,7 +119,6 @@ kdb_bt_cpu(unsigned long cpu) return; } - kdb_set_current_task(kdb_tsk); kdb_bt1(kdb_tsk, ~0UL, false); } @@ -166,10 +165,8 @@ kdb_bt(int argc, const char **argv) if (diag) return diag; p = find_task_by_pid_ns(pid, &init_pid_ns); - if (p) { - kdb_set_current_task(p); + if (p) return kdb_bt1(p, ~0UL, false); - } kdb_printf("No process with pid == %ld found\n", pid); return 0; } else if (strcmp(argv[0], "btt") == 0) { @@ -178,11 +175,9 @@ kdb_bt(int argc, const char **argv) diag = kdbgetularg((char *)argv[1], &addr); if (diag) return diag; - kdb_set_current_task((struct task_struct *)addr); return kdb_bt1((struct task_struct *)addr, ~0UL, false); } else if (strcmp(argv[0], "btc") == 0) { unsigned long cpu = ~0; - struct task_struct *save_current_task = kdb_current_task; if (argc > 1) return KDB_ARGCOUNT; if (argc == 1) { @@ -204,7 +199,6 @@ kdb_bt(int argc, const char **argv) kdb_bt_cpu(cpu); touch_nmi_watchdog(); } - kdb_set_current_task(save_current_task); } return 0; } else { diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c index 4d44b3746836..ba12e9f4661e 100644 --- a/kernel/debug/kdb/kdb_main.c +++ b/kernel/debug/kdb/kdb_main.c @@ -1138,7 +1138,7 @@ static void kdb_dumpregs(struct pt_regs *regs) console_loglevel = old_lvl; } -void kdb_set_current_task(struct task_struct *p) +static void kdb_set_current_task(struct task_struct *p) { kdb_current_task = p; diff --git a/kernel/debug/kdb/kdb_private.h b/kernel/debug/kdb/kdb_private.h index e829b22f3946..2e296e4a234c 100644 --- a/kernel/debug/kdb/kdb_private.h +++ b/kernel/debug/kdb/kdb_private.h @@ -240,7 +240,6 @@ extern void *debug_kmalloc(size_t size, gfp_t flags); extern void debug_kfree(void *); extern void debug_kusage(void); -extern void kdb_set_current_task(struct task_struct *); extern struct task_struct *kdb_current_task; extern struct pt_regs *kdb_current_regs; -- cgit v1.2.3 From bbfceba15f8d1260c328a254efc2b3f2deae4904 Mon Sep 17 00:00:00 2001 From: Douglas Anderson Date: Sat, 9 Nov 2019 11:16:44 -0800 Subject: kdb: Get rid of confusing diag msg from "rd" if current task has no regs If you switch to a sleeping task with the "pid" command and then type "rd", kdb tells you this: No current kdb registers. You may need to select another task diag: -17: Invalid register name The first message makes sense, but not the second. Fix it by just returning 0 after commands accessing the current registers finish if we've already printed the "No current kdb registers" error. While fixing kdb_rd(), change the function to use "if" rather than "ifdef". It cleans the function up a bit and any modern compiler will have no trouble handling still producing good code. Signed-off-by: Douglas Anderson Link: https://lore.kernel.org/r/20191109111624.5.I121f4c6f0c19266200bf6ef003de78841e5bfc3d@changeid Signed-off-by: Daniel Thompson --- kernel/debug/kdb/kdb_main.c | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) (limited to 'kernel') diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c index ba12e9f4661e..b22292b649c4 100644 --- a/kernel/debug/kdb/kdb_main.c +++ b/kernel/debug/kdb/kdb_main.c @@ -543,9 +543,8 @@ int kdbgetaddrarg(int argc, const char **argv, int *nextarg, if (diag) return diag; } else if (symname[0] == '%') { - diag = kdb_check_regs(); - if (diag) - return diag; + if (kdb_check_regs()) + return 0; /* Implement register values with % at a later time as it is * arch optional. */ @@ -1836,8 +1835,7 @@ static int kdb_go(int argc, const char **argv) */ static int kdb_rd(int argc, const char **argv) { - int len = kdb_check_regs(); -#if DBG_MAX_REG_NUM > 0 + int len = 0; int i; char *rname; int rsize; @@ -1846,8 +1844,14 @@ static int kdb_rd(int argc, const char **argv) u16 reg16; u8 reg8; - if (len) - return len; + if (kdb_check_regs()) + return 0; + + /* Fallback to Linux showregs() if we don't have DBG_MAX_REG_NUM */ + if (DBG_MAX_REG_NUM <= 0) { + kdb_dumpregs(kdb_current_regs); + return 0; + } for (i = 0; i < DBG_MAX_REG_NUM; i++) { rsize = dbg_reg_def[i].size * 2; @@ -1889,12 +1893,7 @@ static int kdb_rd(int argc, const char **argv) } } kdb_printf("\n"); -#else - if (len) - return len; - kdb_dumpregs(kdb_current_regs); -#endif return 0; } @@ -1928,9 +1927,8 @@ static int kdb_rm(int argc, const char **argv) if (diag) return diag; - diag = kdb_check_regs(); - if (diag) - return diag; + if (kdb_check_regs()) + return 0; diag = KDB_BADREG; for (i = 0; i < DBG_MAX_REG_NUM; i++) { -- cgit v1.2.3 From a4f8a7fb1963bc02fbd40a0a28e128bb56d2fcc9 Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Thu, 28 Nov 2019 13:07:53 +0000 Subject: kdb: remove redundant assignment to pointer bp The point bp is assigned a value that is never read, it is being re-assigned later to bp = &kdb_breakpoints[lowbp] in a for-loop. Remove the redundant assignment. Addresses-Coverity ("Unused value") Signed-off-by: Colin Ian King Link: https://lore.kernel.org/r/20191128130753.181246-1-colin.king@canonical.com Signed-off-by: Daniel Thompson --- kernel/debug/kdb/kdb_bp.c | 1 - 1 file changed, 1 deletion(-) (limited to 'kernel') diff --git a/kernel/debug/kdb/kdb_bp.c b/kernel/debug/kdb/kdb_bp.c index 62c301ad0773..d7ebb2c79cb8 100644 --- a/kernel/debug/kdb/kdb_bp.c +++ b/kernel/debug/kdb/kdb_bp.c @@ -412,7 +412,6 @@ static int kdb_bc(int argc, const char **argv) * assume that the breakpoint number is desired. */ if (addr < KDB_MAXBPT) { - bp = &kdb_breakpoints[addr]; lowbp = highbp = addr; highbp++; } else { -- cgit v1.2.3 From dc2c733e65848b1df8d55c83eea79fc4a868c800 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Fri, 24 Jan 2020 18:14:40 +0200 Subject: kdb: Use for_each_console() helper Replace open coded single-linked list iteration loop with for_each_console() helper in use. Signed-off-by: Andy Shevchenko Signed-off-by: Daniel Thompson --- kernel/debug/kdb/kdb_io.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) (limited to 'kernel') diff --git a/kernel/debug/kdb/kdb_io.c b/kernel/debug/kdb/kdb_io.c index 8bcdded5d61f..924bc9298a42 100644 --- a/kernel/debug/kdb/kdb_io.c +++ b/kernel/debug/kdb/kdb_io.c @@ -553,7 +553,7 @@ int vkdb_printf(enum kdb_msgsrc src, const char *fmt, va_list ap) int this_cpu, old_cpu; char *cp, *cp2, *cphold = NULL, replaced_byte = ' '; char *moreprompt = "more> "; - struct console *c = console_drivers; + struct console *c; unsigned long uninitialized_var(flags); /* Serialize kdb_printf if multiple cpus try to write at once. @@ -698,10 +698,9 @@ kdb_printit: cp2++; } } - while (c) { + for_each_console(c) { c->write(c, cp, retlen - (cp - kdb_buffer)); touch_nmi_watchdog(); - c = c->next; } } if (logging) { @@ -752,7 +751,6 @@ kdb_printit: moreprompt = "more> "; kdb_input_flush(); - c = console_drivers; if (dbg_io_ops && !dbg_io_ops->is_console) { len = strlen(moreprompt); @@ -762,10 +760,9 @@ kdb_printit: cp++; } } - while (c) { + for_each_console(c) { c->write(c, moreprompt, strlen(moreprompt)); touch_nmi_watchdog(); - c = c->next; } if (logging) -- cgit v1.2.3