Re: [PATCH v3 21/26] printk: Register information into meminspect

[Lorenzo Stoakes <ljs@kernel.org>]

On Wed, Jul 08, 2026 at 11:02:00AM +0530, Mukesh Ojha wrote:
> Annotate vital static, dynamic information into meminspect for debugging
>
> Static:
>  - prb_descs
>  - prb_infos
>  - prb
>  - prb_data
>  - clear_seq
>  - printk_rb_static
>  - printk_rb_dynamic
>
> Dynamic:
>  - new_descs
>  - new_infos
>  - new_log_buf
>
> meminspect uses a different API to annotate variables for inspection,
> and information about these variables is stored in the inspection table.

This commit message is a mess again. You're writing what the code does, not why,
you mention separate API but not which or why.

It's useless basically, and it's weird to list arbitrary fields like this.

Always think 'what would be useful to reviewers and people coming back to this
patch in debug/development'.

If it just says what the code says it's useless. Put what's NOT there.

>
> Reviewed-by: Petr Mladek <pmladek@suse.com>
> Co-developed-by: Eugen Hristev <eugen.hristev@linaro.org>
> Signed-off-by: Eugen Hristev <eugen.hristev@linaro.org>
> Reviewed-by: John Ogness <john.ogness@linutronix.de>
> Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> ---
>  kernel/printk/printk.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 2fe9a963c823..f7f32c09b9b8 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -49,6 +49,7 @@
>  #include <linux/sched/debug.h>
>  #include <linux/sched/task_stack.h>
>  #include <linux/panic.h>
> +#include <linux/meminspect.h>
>
>  #include <linux/uaccess.h>
>  #include <asm/sections.h>
> @@ -518,10 +519,17 @@ static u32 log_buf_len = __LOG_BUF_LEN;
>  #endif
>  _DEFINE_PRINTKRB(printk_rb_static, CONFIG_LOG_BUF_SHIFT - PRB_AVGBITS,
>  		 PRB_AVGBITS, &__log_buf[0]);
> +MEMINSPECT_NAMED_ENTRY(prb_descs, _printk_rb_static_descs);
> +MEMINSPECT_NAMED_ENTRY(prb_infos, _printk_rb_static_infos);
> +MEMINSPECT_NAMED_ENTRY(prb_data, __log_buf);
> +MEMINSPECT_SIMPLE_ENTRY(printk_rb_static);
>
>  static struct printk_ringbuffer printk_rb_dynamic;
> +MEMINSPECT_SIMPLE_ENTRY(printk_rb_dynamic);
>
>  struct printk_ringbuffer *prb = &printk_rb_static;
> +MEMINSPECT_SIMPLE_ENTRY(prb);
> +MEMINSPECT_SIMPLE_ENTRY(clear_seq);
>
>  /*
>   * We cannot access per-CPU data (e.g. per-CPU flush irq_work) before
> @@ -1238,6 +1246,10 @@ void __init setup_log_buf(int early)
>
>  	local_irq_restore(flags);
>
> +	meminspect_lock_register_va(new_log_buf, new_log_buf_len);
> +	meminspect_lock_register_va(new_descs, new_descs_size);
> +	meminspect_lock_register_va(new_infos, new_infos_size);
> +
>  	/*
>  	 * Copy any remaining messages that might have appeared from
>  	 * NMI context after copying but before switching to the
>
> --
> 2.53.0
>

Overall exposing live dynamic printk information to drivers seems unwise, but
not quite as insane as some of the other stuff thus exposed...

Thanks, Lorenzo