2024-03-13 10:28:00

by Sreenath Vijayan

[permalink] [raw]
Subject: [PATCH v6 1/2] printk: Add function to replay kernel log on consoles

Add a generic function console_replay_all() for replaying
the kernel log on consoles, in any context. It would allow
viewing the logs on an unresponsive terminal via sysrq.

Reuse the existing code from console_flush_on_panic() for
resetting the sequence numbers, by introducing a new helper
function __console_rewind_all(). It is safe to be called
under console_lock().

Try to acquire lock on the console subsystem without waiting.
If successful, reset the sequence number to oldest available
record on all consoles and call console_unlock() which will
automatically flush the messages to the consoles.

Suggested-by: John Ogness <[email protected]>
Suggested-by: Petr Mladek <[email protected]>
Signed-off-by: Shimoyashiki Taichi <[email protected]>
Reviewed-by: John Ogness <[email protected]>
Signed-off-by: Sreenath Vijayan <[email protected]>
---
include/linux/printk.h | 4 +++
kernel/printk/printk.c | 77 +++++++++++++++++++++++++++++-------------
2 files changed, 57 insertions(+), 24 deletions(-)

diff --git a/include/linux/printk.h b/include/linux/printk.h
index 8ef499ab3c1e..8df2ac6c0e1f 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 console_replay_all(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 console_replay_all(void)
+{
+}
#endif

#ifdef CONFIG_SMP
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index f2444b581e16..9e38cf0c1002 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -3134,6 +3134,40 @@ void console_unblank(void)
pr_flush(1000, true);
}

+/*
+ * Rewind all consoles to the oldest available record.
+ *
+ * IMPORTANT: The function is safe only when called under
+ * console_lock(). It is not enforced because
+ * it is used as a best effort in panic().
+ */
+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 {
+ /*
+ * This assignment is safe only when called under
+ * console_lock(). On panic, legacy consoles are
+ * only best effort.
+ */
+ c->seq = seq;
+ }
+ }
+ console_srcu_read_unlock(cookie);
+}
+
/**
* console_flush_on_panic - flush console content on panic
* @mode: flush all messages in buffer or just the pending ones
@@ -3162,30 +3196,8 @@ void console_flush_on_panic(enum con_flush_mode mode)
*/
console_may_schedule = 0;

- if (mode == CONSOLE_REPLAY_ALL) {
- 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 {
- /*
- * This is an unsynchronized assignment. On
- * panic legacy consoles are only best effort.
- */
- c->seq = seq;
- }
- }
- console_srcu_read_unlock(cookie);
- }
+ if (mode == CONSOLE_REPLAY_ALL)
+ __console_rewind_all();

console_flush_all(false, &next_seq, &handover);
}
@@ -4259,6 +4271,23 @@ void kmsg_dump_rewind(struct kmsg_dump_iter *iter)
}
EXPORT_SYMBOL_GPL(kmsg_dump_rewind);

+/**
+ * console_replay_all - replay kernel log on consoles
+ *
+ * Try to obtain lock on console subsystem and replay all
+ * available records in printk buffer on the consoles.
+ * Does nothing if lock is not obtained.
+ *
+ * Context: Any context.
+ */
+void console_replay_all(void)
+{
+ if (console_trylock()) {
+ __console_rewind_all();
+ /* Consoles are flushed as part of console_unlock(). */
+ console_unlock();
+ }
+}
#endif

#ifdef CONFIG_SMP
--
2.25.1



2024-05-22 15:18:31

by Petr Mladek

[permalink] [raw]
Subject: Re: [PATCH v6 1/2] printk: Add function to replay kernel log on consoles


On Wed 2024-03-13 15:50:52, Sreenath Vijayan wrote:
> Add a generic function console_replay_all() for replaying
> the kernel log on consoles, in any context. It would allow
> viewing the logs on an unresponsive terminal via sysrq.
>
> Reuse the existing code from console_flush_on_panic() for
> resetting the sequence numbers, by introducing a new helper
> function __console_rewind_all(). It is safe to be called
> under console_lock().
>
> Try to acquire lock on the console subsystem without waiting.
> If successful, reset the sequence number to oldest available
> record on all consoles and call console_unlock() which will
> automatically flush the messages to the consoles.
>
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -4259,6 +4271,23 @@ void kmsg_dump_rewind(struct kmsg_dump_iter *iter)
> }
> EXPORT_SYMBOL_GPL(kmsg_dump_rewind);
>
> +/**
> + * console_replay_all - replay kernel log on consoles

It should rather be called "console_try_replay_all()" to make it clear
that it is just the best effort.

> + *
> + * Try to obtain lock on console subsystem and replay all
> + * available records in printk buffer on the consoles.
> + * Does nothing if lock is not obtained.
> + *
> + * Context: Any context.

This should be:

* Context: Any, except for NMI

Basically only lockless code is safe in NMI which is not the case here.

> + */
> +void console_replay_all(void)
> +{
> + if (console_trylock()) {
> + __console_rewind_all();
> + /* Consoles are flushed as part of console_unlock(). */
> + console_unlock();
> + }
> +}
> #endif

Otherwise, it looks good. With the two changes:

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

I am sorry for the late review. I have been snowed under tasks.
Also I had healthy problems.

I have seen a mail that Greg has queued the patch in tty-next.
I am not sure if it still can be fixed. It will be perfectly fine
to change this by a followup patch.

Best Regards,
Petr

2024-05-23 05:53:46

by Sreenath Vijayan

[permalink] [raw]
Subject: Re: [PATCH v6 1/2] printk: Add function to replay kernel log on consoles

On Wed, May 22, 2024 at 05:18:13PM +0200, Petr Mladek wrote:
>
> On Wed 2024-03-13 15:50:52, Sreenath Vijayan wrote:
> > Add a generic function console_replay_all() for replaying
> > the kernel log on consoles, in any context. It would allow
> > viewing the logs on an unresponsive terminal via sysrq.
> >
> > Reuse the existing code from console_flush_on_panic() for
> > resetting the sequence numbers, by introducing a new helper
> > function __console_rewind_all(). It is safe to be called
> > under console_lock().
> >
> > Try to acquire lock on the console subsystem without waiting.
> > If successful, reset the sequence number to oldest available
> > record on all consoles and call console_unlock() which will
> > automatically flush the messages to the consoles.
> >
> > --- a/kernel/printk/printk.c
> > +++ b/kernel/printk/printk.c
> > @@ -4259,6 +4271,23 @@ void kmsg_dump_rewind(struct kmsg_dump_iter *iter)
> > }
> > EXPORT_SYMBOL_GPL(kmsg_dump_rewind);
> >
> > +/**
> > + * console_replay_all - replay kernel log on consoles
>
> It should rather be called "console_try_replay_all()" to make it clear
> that it is just the best effort.
>

Ok, noted.

> > + *
> > + * Try to obtain lock on console subsystem and replay all
> > + * available records in printk buffer on the consoles.
> > + * Does nothing if lock is not obtained.
> > + *
> > + * Context: Any context.
>
> This should be:
>
> * Context: Any, except for NMI
>
> Basically only lockless code is safe in NMI which is not the case here.
>

Understood.

> > + */
> > +void console_replay_all(void)
> > +{
> > + if (console_trylock()) {
> > + __console_rewind_all();
> > + /* Consoles are flushed as part of console_unlock(). */
> > + console_unlock();
> > + }
> > +}
> > #endif
>
> Otherwise, it looks good. With the two changes:
>
> Reviewed-by: Petr Mladek <[email protected]>
>
> I am sorry for the late review. I have been snowed under tasks.
> Also I had healthy problems.
>
> I have seen a mail that Greg has queued the patch in tty-next.
> I am not sure if it still can be fixed. It will be perfectly fine
> to change this by a followup patch.
>
> Best Regards,
> Petr

Thank you for the review comments. As the patch has been merged to
mainline, I will soon send a followup patch with "Fixes:" tag.

Regards,
Sreenath