public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: John Ogness <john.ogness@linutronix.de>
To: Sreenath Vijayan <sreenath.vijayan@sony.com>,
	sreenath.vijayan@sony.com, rdunlap@infradead.org
Cc: anandakumar.balasubramaniam@sony.com,
	taichi.shimoyashiki@sony.com, pmladek@suse.com, corbet@lwn.net,
	gregkh@linuxfoundation.org, jirislaby@kernel.org,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-serial@vger.kernel.org, senozhatsky@chromium.org,
	rostedt@goodmis.org
Subject: Re: [PATCH v2] tty/sysrq: Dump printk ring buffer messages via sysrq
Date: Wed, 10 Jan 2024 11:26:45 +0106	[thread overview]
Message-ID: <874jfllcaq.fsf@jogness.linutronix.de> (raw)
In-Reply-To: <20240110155634.1685656-1-sreenath.vijayan@sony.com>

On 2024-01-10, Sreenath Vijayan <sreenath.vijayan@sony.com> wrote:
> When terminal is unresponsive, one cannot use dmesg to view printk
> ring buffer messages. Also, syslog services may be disabled,
> to check them after a reboot, especially on embedded systems.
> In this scenario, dump the printk ring buffer messages via sysrq
> by pressing sysrq+D.

Generally speaking, I like this idea. I also like that you do not
re-flood the kernel buffers and instead just print directly to the
consoles. (I wish sysrq+z with ftrace did that as well.)

Relying on a workqueue will really limit the usefulness of this
feature. Safe efforts could be made to print directly from the interrupt
context. But maybe this approach can be accepted for now as being
"better than nothing", which can be improved upon in the future (for
example, when atomic consoles are available).

Some more comments from me below...

> diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
> index 02217e3c916b..62b3911f03b5 100644
> --- a/drivers/tty/sysrq.c
> +++ b/drivers/tty/sysrq.c
> @@ -450,6 +452,51 @@ static const struct sysrq_key_op sysrq_unrt_op = {
>  	.enable_mask	= SYSRQ_ENABLE_RTNICE,
>  };
>  
> +static void dmesg_dump_callback(struct work_struct *work)
> +{
> +	struct kmsg_dump_iter iter;
> +	size_t len;
> +	char *buf;
> +	struct console *con;
> +	int cookie;
> +
> +	/* Size to be updated if PRINTK_MESSAGE_MAX changes */
> +	buf = kzalloc(2048, GFP_KERNEL);
> +	if (!buf)
> +		return;
> +
> +	kmsg_dump_rewind(&iter);
> +	while (kmsg_dump_get_line(&iter, 1, buf, 2048, &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);
> +}

Rather than implementing all this in drivers/tty/sysrq.c it would
probably be better to just call a new function that is implemented in
kernel/printk/printk.c. Then you would have access to printk-private
items (such as the PRINTK_MESSAGE_MAX macro).

For example, sysrq+z just calls ftrace_dump(), which is implemented in
kernel/trace/trace.c.

John Ogness

      reply	other threads:[~2024-01-10 10:20 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-21 13:39 [PATCH] tty/sysrq: Dump kernel ring buffer messages via sysrq Sreenath Vijayan
2023-12-21 16:52 ` Greg KH
2023-12-21 23:12   ` Randy Dunlap
2023-12-22 11:44     ` Sreenath Vijayan
2023-12-22 19:43       ` Randy Dunlap
2023-12-24 11:36 ` kernel test robot
2023-12-24 14:22 ` kernel test robot
2024-01-10 15:54 ` [PATCH v2] tty/sysrq: Dump printk " Sreenath Vijayan
2024-01-10 10:20   ` John Ogness [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=874jfllcaq.fsf@jogness.linutronix.de \
    --to=john.ogness@linutronix.de \
    --cc=anandakumar.balasubramaniam@sony.com \
    --cc=corbet@lwn.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=jirislaby@kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=pmladek@suse.com \
    --cc=rdunlap@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=senozhatsky@chromium.org \
    --cc=sreenath.vijayan@sony.com \
    --cc=taichi.shimoyashiki@sony.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox