KDB: Fix dmesg command
cover KDB: Fix dmesg command
Hi all,
The dmesg command appears to be broken after the printk rework. The old logic in the kdb code makes no sense in terms of current printk/logging storage format, and KDB simply hangs forever upon entering 'dmesg' command.
The first patch revives the command by switching to kmsg_dumper iterator. As a side-effect, the code is now much more simpler.
A few changes were needed in the printk.c: we needed unlocked variant of the kmsg_dumper iterator, but these can surely wait for 3.6.
It's probably too late even for the first patch to go to 3.5, but I'll try to convince otherwise. :-) Here we go:
- The current code is broken for sure, and has no hope to work at all. It is a regression;
- The new code works for me, and probably works for everyone else;
- If it compiles (and I urge everyone to compile-test it on your setup), it hardly can make things worse.
Thanks!
--
include/linux/kmsg_dump.h | 16 +++++++ kernel/debug/kdb/kdb_main.c | 91 +++++++++++++++------------------------- kernel/debug/kdb/kdb_private.h | 1 - kernel/printk.c | 83 +++++++++++++++++++++++------------- 4 files changed, 104 insertions(+), 87 deletions(-)
Signature · 2 lines
Anton Vorontsov Email: [email protected]
PATCH 1/4 kdb: Revive dmesg command
dmesg command appears to be broken after the printk rework. The old logic in kdb code makes no sense in terms of current printk/logging storage format, and KDB simply hangs forever.
This patch revives the command by switching to kmsg_dumper iterator.
The code is now much more simpler and shorter.
--- kernel/debug/kdb/kdb_main.c | 91 ++++++++++++++++--------------------------- 1 file changed, 33 insertions(+), 58 deletions(-)
kernel/debug/kdb/kdb_main.c +33 -58
@@ -14,6 +14,7 @@#include <linux/ctype.h> #include <linux/string.h> #include <linux/kernel.h> +#include <linux/kmsg_dump.h> #include <linux/reboot.h> #include <linux/sched.h> #include <linux/sysrq.h>
@@ -2040,8 +2041,15 @@ static int kdb_env(int argc, const char **argv)*/ static int kdb_dmesg (int argc,const char **argv) { -char *syslog_data[4 ], *start, *end, c ='\0' , *p; -int diag, logging, logsize, lines =0 , adjust =0 , n; +int diag; +int logging; +int lines =0 ; +int adjust =0 ; +int n =0 ; +int skip =0 ; +struct kmsg_dumper dumper = { .active =1 }; +size_t len; +char buf[201 ];if (argc >2 )return KDB_ARGCOUNT ;
@@ -2064,22 +2072,10 @@ static int kdb_dmesg(int argc, const char **argv)kdb_set (2 , setargs); } -/* syslog_data[0,1] physical start, end+1. syslog_data[2,3] -* logical start, end+1. */ -kdb_syslog_data (syslog_data); -if (syslog_data[2 ] == syslog_data[3 ]) -return 0 ; - logsize = syslog_data[1 ] - syslog_data[0 ]; - start = syslog_data[2 ]; - end = syslog_data[3 ]; -#define KDB_WRAP (p) (((p - syslog_data[0]) % logsize) + syslog_data[0]) -for (n =0 , p = start; p < end; ++p) { - c = *KDB_WRAP (p); -if (c =='\n' ) - ++n; - } -if (c !='\n' ) - ++n; +kmsg_dump_rewind (&dumper); +while (kmsg_dump_get_line (&dumper,1 ,NULL ,0 ,NULL )) + n++; +if (lines <0 ) {if (adjust >= n) kdb_printf("buffer only contains %d lines, nothing "
@@ -2087,21 +2083,11 @@ static int kdb_dmesg(int argc, const char **argv)else if (adjust - lines >= n)kdb_printf ("buffer only contains %d lines, last %d " "lines printed\n" , n, n - adjust); -if (adjust) { -for (; start < end && adjust; ++start) { -if (*KDB_WRAP (start) =='\n' ) - --adjust; - } -if (start < end) - ++start; - } -for (p = start; p < end && lines; ++p) { -if (*KDB_WRAP (p) =='\n' ) - ++lines; - } - end = p; + skip = adjust; + lines =abs (lines); }else if (lines >0 ) { -int skip = n - (adjust + lines); + skip = n - lines - adjust; + lines =abs (lines);if (adjust >= n) {kdb_printf ("buffer only contains %d lines, " "nothing printed\n" , n);
@@ -2112,35 +2098,24 @@ static int kdb_dmesg(int argc, const char **argv)kdb_printf ("buffer only contains %d lines, first " "%d lines printed\n" , n, lines); } -for (; start < end && skip; ++start) { -if (*KDB_WRAP (start) =='\n' ) - --skip; - } -for (p = start; p < end && lines; ++p) { -if (*KDB_WRAP (p) =='\n' ) - --lines; - } - end = p; + }else { + lines = n; } -/* Do a line at a time (max 200 chars) to reduce protocol overhead */ - c ='\n' ; -while (start != end) { -char buf[201 ]; - p = buf; -if (KDB_FLAG (CMD_INTERRUPT )) -return 0 ; -while (start < end && (c = *KDB_WRAP (start)) && - (p - buf) <sizeof (buf)-1 ) { - ++start; - *p++ = c; -if (c =='\n' ) -break ; + +if (skip >= n || skip <0 ) +return 0 ; + +kmsg_dump_rewind (&dumper); +while (kmsg_dump_get_line (&dumper,1 , buf,sizeof (buf), &len)) { +if (skip) { + skip--; +continue ; } - *p ='\0' ; -kdb_printf ("%s" , buf); +if (!lines--) +break ; + +kdb_printf ("%.*s\n" , (int )len -1 , buf); } -if (c !='\n' ) -kdb_printf ("\n" );return 0 ; }
Signature · 1 line
1.7.10.4
Re: [PATCH for-3.5 1/4] kdb: Revive dmesg command
Quoted 9 lines · On Fri, 20 Jul 2012 17:27:37 -0700 Anton Vorontsov…
dmesg command appears to be broken after the printk rework. The old logic in kdb code makes no sense in terms of current printk/logging storage format, and KDB simply hangs forever.
This patch revives the command by switching to
kmsg_dumperiterator.The code is now much more simpler and shorter.
Signed-off-by Anton Vorontsov <[email protected]>
This one should me merged into 3.5.1 methinks. Note that it has been merged into mainline without a -stable tag.
Re: [PATCH for-3.5 1/4] kdb: Revive dmesg command
Quoted 4 lines · Andrew Morton
Quoted 4 lines
dmesg command appears to be broken after the printk rework. The old logic in kdb code makes no sense in terms of current printk/logging storage format, and KDB simply hangs forever.
Quoted 8 lines
Quoted 5 lines
This patch revives the command by switching to
kmsg_dumperiterator.The code is now much more simpler and shorter.
Signed-off-by Anton Vorontsov <[email protected]>This one should me merged into 3.5.1 methinks. Note that it has been merged into mainline without a -stable tag.
Thanks to Linus, it made it into v3.5 release, so I believe there is no need for -stable.
Cheers,
Signature · 2 lines
Anton Vorontsov Email: [email protected]
PATCH 2/4 printk: Remove kdb_syslog_data
In reply to Anton VorontsovThe function is no longer needed, so remove it.
--- kernel/debug/kdb/kdb_private.h | 1 - kernel/printk.c | 15 --------------- 2 files changed, 16 deletions(-)
kernel/debug/kdb/kdb_private.h +0 -1
@@ -205,7 +205,6 @@ extern char kdb_grep_string[];extern int kdb_grep_leading;extern int kdb_grep_trailing;extern char *kdb_cmds[]; -extern void kdb_syslog_data (char *syslog_data[]);extern unsigned long kdb_task_state_string (const char *);extern char kdb_task_state_char (const struct task_struct *); extern unsigned long kdb_task_state(const struct task_struct *p,
kernel/printk.c +0 -15
@@ -1192,21 +1192,6 @@ SYSCALL_DEFINE3(syslog, int, type, char __user *, buf, int, len)return do_syslog (type, buf, len,SYSLOG_FROM_CALL ); } -#ifdef CONFIG_KGDB_KDB -/* kdb dmesg command needs access to the syslog buffer. do_syslog() -* uses locks so it cannot be used during debugging. Just tell kdb -* where the start and end of the physical and logical logs are. This -* is equivalent to do_syslog(3). -*/ -void kdb_syslog_data (char *syslog_data[4 ]) -{ - syslog_data[0 ] = log_buf; - syslog_data[1 ] = log_buf + log_buf_len; - syslog_data[2 ] = log_buf + log_first_idx; - syslog_data[3 ] = log_buf + log_next_idx; -} -#endif /* CONFIG_KGDB_KDB */ -static bool __read_mostly ignore_loglevel;static int __init ignore_loglevel_setup (char *str)
Signature · 1 line
1.7.10.4
PATCH 3/4 printk: Implement some unlocked kmsg_dump functions
In reply to Anton VorontsovIf used from KDB, the locked variants are prone to deadlocks (suppose we got to the debugger w/ the logbuf lock held).
So, we have to implement a few routines that grab no logbuf lock.
Yet we don't need these functions in modules, so we don't export them.
--- include/linux/kmsg_dump.h | 16 +++++++++++ kernel/printk.c | 68 ++++++++++++++++++++++++++++++++++++--------- 2 files changed, 71 insertions(+), 13 deletions(-)
include/linux/kmsg_dump.h +16 -0
@@ -55,12 +55,17 @@ struct kmsg_dumper {#ifdef CONFIG_PRINTK void kmsg_dump (enum kmsg_dump_reason reason); +bool kmsg_dump_get_line_nolock (struct kmsg_dumper *dumper,bool syslog, +char *line,size_t size,size_t *len); +bool kmsg_dump_get_line (struct kmsg_dumper *dumper,bool syslog,char *line,size_t size,size_t *len);bool kmsg_dump_get_buffer (struct kmsg_dumper *dumper,bool syslog,char *buf,size_t size,size_t *len); +void kmsg_dump_rewind_nolock (struct kmsg_dumper *dumper); +void kmsg_dump_rewind (struct kmsg_dumper *dumper);int kmsg_dump_register (struct kmsg_dumper *dumper);
@@ -71,6 +76,13 @@ static inline void kmsg_dump(enum kmsg_dump_reason reason) { } +static inline bool kmsg_dump_get_line_nolock (struct kmsg_dumper *dumper, +bool syslog,const char *line, +size_t size,size_t *len) +{ +return false; +} +static inline bool kmsg_dump_get_line (struct kmsg_dumper *dumper,bool syslog,const char *line,size_t size,size_t *len) {
@@ -83,6 +95,10 @@ static inline bool kmsg_dump_get_buffer(struct kmsg_dumper *dumper, bool syslog,return false; } +static inline void kmsg_dump_rewind_nolock (struct kmsg_dumper *dumper) +{ +} +static inline void kmsg_dump_rewind (struct kmsg_dumper *dumper) { }
kernel/printk.c +55 -13
118 lines not on this page · raw message
Signature · 1 line
1.7.10.4
PATCH 4/4 kdb: Switch to nolock variants of kmsg_dump functions
In reply to Anton VorontsovThe locked variants are prone to deadlocks (suppose we got to the debugger w/ the logbuf lock held), so let's switch to nolock variants.
--- kernel/debug/kdb/kdb_main.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
kernel/debug/kdb/kdb_main.c +4 -4
@@ -2072,8 +2072,8 @@ static int kdb_dmesg(int argc, const char **argv)kdb_set (2 , setargs); } -kmsg_dump_rewind (&dumper); -while (kmsg_dump_get_line (&dumper,1 ,NULL ,0 ,NULL )) +kmsg_dump_rewind_nolock (&dumper); +while (kmsg_dump_get_line_nolock (&dumper,1 ,NULL ,0 ,NULL )) n++;if (lines <0 ) {
@@ -2105,8 +2105,8 @@ static int kdb_dmesg(int argc, const char **argv)if (skip >= n || skip <0 )return 0 ; -kmsg_dump_rewind (&dumper); -while (kmsg_dump_get_line (&dumper,1 , buf,sizeof (buf), &len)) { +kmsg_dump_rewind_nolock (&dumper); +while (kmsg_dump_get_line_nolock (&dumper,1 , buf,sizeof (buf), &len)) {if (skip) { skip--;continue ;
Signature · 1 line
1.7.10.4