| Message ID | 402f0cbc3a573503c7cc794113aa5137ed7f276c.1705331453.git.sreenath.vijayan@sony.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v3,1/2] printk: Add function to dump printk buffer directly to consoles | expand |
On 2024-01-17, Sreenath Vijayan <sreenath.vijayan@sony.com> wrote: > It is useful to be able to dump the printk buffer directly to > consoles in some situations so as to not flood the buffer. > This needs access to private items of printk like PRINTK_MESSAGE_MAX. > Add function in printk.c to accomplish this. > > Suggested-by: John Ogness <john.ogness@linutronix.de> > Signed-off-by: Sreenath Vijayan <sreenath.vijayan@sony.com> > Signed-off-by: Shimoyashiki Taichi <taichi.shimoyashiki@sony.com> > --- > include/linux/printk.h | 4 ++++ > kernel/printk/printk.c | 33 +++++++++++++++++++++++++++++++++ > 2 files changed, 37 insertions(+) > > diff --git a/include/linux/printk.h b/include/linux/printk.h > index 8ef499ab3c1e..0896745f31e2 100644 > --- a/include/linux/printk.h > +++ b/include/linux/printk.h > @@ -192,6 +192,7 @@ void show_regs_print_info(const char *log_lvl); > extern asmlinkage void dump_stack_lvl(const char *log_lvl) __cold; > extern asmlinkage void dump_stack(void) __cold; > void printk_trigger_flush(void); > +void dump_printk_buffer(void); > #else > static inline __printf(1, 0) > int vprintk(const char *s, va_list args) > @@ -271,6 +272,9 @@ static inline void dump_stack(void) > static inline void printk_trigger_flush(void) > { > } > +static inline void dump_printk_buffer(void) > +{ > +} > #endif > > #ifdef CONFIG_SMP > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c > index f2444b581e16..5b11fb377f8f 100644 > --- a/kernel/printk/printk.c > +++ b/kernel/printk/printk.c > @@ -4259,6 +4259,39 @@ void kmsg_dump_rewind(struct kmsg_dump_iter *iter) > } > EXPORT_SYMBOL_GPL(kmsg_dump_rewind); > > +/** > + * Dump the printk ring buffer directly to consoles > + */ > +void dump_printk_buffer(void) > +{ > + struct kmsg_dump_iter iter; > + struct console *con; > + char *buf; > + size_t len; > + int cookie; > + > + buf = kmalloc(PRINTK_MESSAGE_MAX, GFP_KERNEL); > + if (!buf) > + return; > + > + kmsg_dump_rewind(&iter); > + while (kmsg_dump_get_line(&iter, 1, buf, PRINTK_MESSAGE_MAX, &len)) { Although using the kmsg_dump interface will provide you the messages, they will not necessarily be in the correct format. Consoles can be set to use extended format. We probably should respect that console setting. > + /* > + * Since using printk() or pr_*() will append the message to the > + * printk ring buffer, they cannot be used to display the retrieved > + * message. Hence console_write() of serial drivers is used. > + */ > + console_lock(); > + cookie = console_srcu_read_lock(); > + for_each_console_srcu(con) { > + if ((console_srcu_read_flags(con) & CON_ENABLED) && con->write) console_is_usable() should be used instead. It makes the correct checks. > + con->write(con, buf, len); > + } > + console_srcu_read_unlock(cookie); > + console_unlock(); > + } > + kfree(buf); > +} We could do something like this: void dump_printk_buffer(void) { console_lock(); console_flush_on_panic(CONSOLE_REPLAY_ALL); console_unlock(); } This version respects all the console features (formatting, handovers), but console_flush_on_panic() does not to allow cond_resched(), which we would want in this case. We could take the console sequence-resetting code out into its own helper function. Then it would look like this (comments removed to keep things short): static void console_rewind_all(void) { struct console *c; short flags; int cookie; u64 seq; seq = prb_first_valid_seq(prb); cookie = console_srcu_read_lock(); for_each_console_srcu(c) { flags = console_srcu_read_flags(c); if (flags & CON_NBCON) nbcon_seq_force(c, seq); else c->seq = seq; } console_srcu_read_unlock(cookie); } void console_flush_on_panic(enum con_flush_mode mode) { bool handover; u64 next_seq; console_may_schedule = 0; if (mode == CONSOLE_REPLAY_ALL) console_rewind_all(); console_flush_all(false, &next_seq, &handover); } void dump_printk_buffer(void) { bool handover; u64 next_seq; console_lock(); console_rewind_all(); console_flush_all(true, &next_seq, &handover); console_unlock(); } Any thoughts? John
Oops, for the current mainline code it is actually even simpler because
the console_unlock() will perform the flushing:
void dump_printk_buffer(void)
{
console_lock();
console_rewind_all();
console_unlock();
}
John
On Thu, Jan 18, 2024 at 10:55:20AM +0106, John Ogness wrote: > On 2024-01-17, Sreenath Vijayan <sreenath.vijayan@sony.com> wrote: > > It is useful to be able to dump the printk buffer directly to > > consoles in some situations so as to not flood the buffer. > > This needs access to private items of printk like PRINTK_MESSAGE_MAX. > > Add function in printk.c to accomplish this. > > > > Suggested-by: John Ogness <john.ogness@linutronix.de> > > Signed-off-by: Sreenath Vijayan <sreenath.vijayan@sony.com> > > Signed-off-by: Shimoyashiki Taichi <taichi.shimoyashiki@sony.com> > > --- > > include/linux/printk.h | 4 ++++ > > kernel/printk/printk.c | 33 +++++++++++++++++++++++++++++++++ > > 2 files changed, 37 insertions(+) > > > > diff --git a/include/linux/printk.h b/include/linux/printk.h > > index 8ef499ab3c1e..0896745f31e2 100644 > > --- a/include/linux/printk.h > > +++ b/include/linux/printk.h > > @@ -192,6 +192,7 @@ void show_regs_print_info(const char *log_lvl); > > extern asmlinkage void dump_stack_lvl(const char *log_lvl) __cold; > > extern asmlinkage void dump_stack(void) __cold; > > void printk_trigger_flush(void); > > +void dump_printk_buffer(void); > > #else > > static inline __printf(1, 0) > > int vprintk(const char *s, va_list args) > > @@ -271,6 +272,9 @@ static inline void dump_stack(void) > > static inline void printk_trigger_flush(void) > > { > > } > > +static inline void dump_printk_buffer(void) > > +{ > > +} > > #endif > > > > #ifdef CONFIG_SMP > > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c > > index f2444b581e16..5b11fb377f8f 100644 > > --- a/kernel/printk/printk.c > > +++ b/kernel/printk/printk.c > > @@ -4259,6 +4259,39 @@ void kmsg_dump_rewind(struct kmsg_dump_iter *iter) > > } > > EXPORT_SYMBOL_GPL(kmsg_dump_rewind); > > > > +/** > > + * Dump the printk ring buffer directly to consoles > > + */ > > +void dump_printk_buffer(void) > > +{ > > + struct kmsg_dump_iter iter; > > + struct console *con; > > + char *buf; > > + size_t len; > > + int cookie; > > + > > + buf = kmalloc(PRINTK_MESSAGE_MAX, GFP_KERNEL); > > + if (!buf) > > + return; > > + > > + kmsg_dump_rewind(&iter); > > + while (kmsg_dump_get_line(&iter, 1, buf, PRINTK_MESSAGE_MAX, &len)) { > > Although using the kmsg_dump interface will provide you the messages, > they will not necessarily be in the correct format. Consoles can be set > to use extended format. > > We probably should respect that console setting. Thank you for reviewing the patch and pointing out the limitations of ksmg_dump interface. > > > + /* > > + * Since using printk() or pr_*() will append the message to the > > + * printk ring buffer, they cannot be used to display the retrieved > > + * message. Hence console_write() of serial drivers is used. > > + */ > > + console_lock(); > > + cookie = console_srcu_read_lock(); > > + for_each_console_srcu(con) { > > + if ((console_srcu_read_flags(con) & CON_ENABLED) && con->write) > > console_is_usable() should be used instead. It makes the correct checks. > Ok, noted. > > + con->write(con, buf, len); > > + } > > + console_srcu_read_unlock(cookie); > > + console_unlock(); > > + } > > + kfree(buf); > > +} > > We could do something like this: > > void dump_printk_buffer(void) > { > console_lock(); > console_flush_on_panic(CONSOLE_REPLAY_ALL); > console_unlock(); > } > > This version respects all the console features (formatting, handovers), > but console_flush_on_panic() does not to allow cond_resched(), which we > would want in this case. > > We could take the console sequence-resetting code out into its own > helper function. Then it would look like this (comments removed to keep > things short): > > static void console_rewind_all(void) > { > struct console *c; > short flags; > int cookie; > u64 seq; > > seq = prb_first_valid_seq(prb); > > cookie = console_srcu_read_lock(); > for_each_console_srcu(c) { > flags = console_srcu_read_flags(c); > > if (flags & CON_NBCON) > nbcon_seq_force(c, seq); > else > c->seq = seq; > } > console_srcu_read_unlock(cookie); > } > > void console_flush_on_panic(enum con_flush_mode mode) > { > bool handover; > u64 next_seq; > > console_may_schedule = 0; > > if (mode == CONSOLE_REPLAY_ALL) > console_rewind_all(); > > console_flush_all(false, &next_seq, &handover); > } > > void dump_printk_buffer(void) > { > bool handover; > u64 next_seq; > > console_lock(); > console_rewind_all(); > console_flush_all(true, &next_seq, &handover); > console_unlock(); > } > > Any thoughts? > > John Thank you for suggesting this new method. From initial tests, this change looks ok. I will do more testing and send out the next version. Sreenath
diff --git a/include/linux/printk.h b/include/linux/printk.h index 8ef499ab3c1e..0896745f31e2 100644 --- a/include/linux/printk.h +++ b/include/linux/printk.h @@ -192,6 +192,7 @@ void show_regs_print_info(const char *log_lvl); extern asmlinkage void dump_stack_lvl(const char *log_lvl) __cold; extern asmlinkage void dump_stack(void) __cold; void printk_trigger_flush(void); +void dump_printk_buffer(void); #else static inline __printf(1, 0) int vprintk(const char *s, va_list args) @@ -271,6 +272,9 @@ static inline void dump_stack(void) static inline void printk_trigger_flush(void) { } +static inline void dump_printk_buffer(void) +{ +} #endif #ifdef CONFIG_SMP diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index f2444b581e16..5b11fb377f8f 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -4259,6 +4259,39 @@ void kmsg_dump_rewind(struct kmsg_dump_iter *iter) } EXPORT_SYMBOL_GPL(kmsg_dump_rewind); +/** + * Dump the printk ring buffer directly to consoles + */ +void dump_printk_buffer(void) +{ + struct kmsg_dump_iter iter; + struct console *con; + char *buf; + size_t len; + int cookie; + + buf = kmalloc(PRINTK_MESSAGE_MAX, GFP_KERNEL); + if (!buf) + return; + + kmsg_dump_rewind(&iter); + while (kmsg_dump_get_line(&iter, 1, buf, PRINTK_MESSAGE_MAX, &len)) { + /* + * Since using printk() or pr_*() will append the message to the + * printk ring buffer, they cannot be used to display the retrieved + * message. Hence console_write() of serial drivers is used. + */ + console_lock(); + cookie = console_srcu_read_lock(); + for_each_console_srcu(con) { + if ((console_srcu_read_flags(con) & CON_ENABLED) && con->write) + con->write(con, buf, len); + } + console_srcu_read_unlock(cookie); + console_unlock(); + } + kfree(buf); +} #endif #ifdef CONFIG_SMP