On Tue 2025-04-22 14:31:49, Eugen Hristev wrote:
> Add kmsg_kmemdump_register, which registers prb, log_buf and infos/descs
> to kmemdump.
> This will allow kmemdump to be able to dump specific log buffer areas on
> demand.
>
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -4650,6 +4651,18 @@ int kmsg_dump_register(struct kmsg_dumper *dumper)
> }
> EXPORT_SYMBOL_GPL(kmsg_dump_register);
>
> +void kmsg_kmemdump_register(void)
> +{
> + kmemdump_register("log_buf", (void *)log_buf_addr_get(), log_buf_len_get());
> + kmemdump_register("prb", (void *)&prb, sizeof(prb));
> + kmemdump_register("prb", (void *)prb, sizeof(*prb));
This looks strange. "prb" is a pointer to "struct printk_ringbuffer".
It should be enough to register the memory with the structure.
> + kmemdump_register("prb_descs", (void *)_printk_rb_static_descs,
> + sizeof(_printk_rb_static_descs));
> + kmemdump_register("prb_infos", (void *)_printk_rb_static_infos,
> + sizeof(_printk_rb_static_infos));
Also this looks wrong. These are static buffers which are used during
early boot. They might later be replaced by dynamically allocated
buffers when a bigger buffer is requested by "log_buf_len" command
line parameter.
I think that we need to register the memory of the structure
and 3 more buffers. See how the bigger buffer is allocated in
setup_log_buf().
I would expect something like:
unsigned int descs_count;
unsigned long data_size;
descs_count = 2 << prb->desc_ring.count_bits;
data_size = 2 << prb->data_ring.size_bits;
kmemdump_register("prb", (void *)prb, sizeof(*prb));
kmemdump_register("prb_descs", (void *)prb->desc_ring->descs,
descs_count * sizeof(struct prb_desc));
kmemdump_register("prb_infos", (void *)prb->desc_ring->infos,
descs_count * sizeof(struct printk_info));
kmemdump_register("prb_data", (void *)prb->data_ring->data, data_size);
But I wonder if this is enough. The current crash dump code also needs
to export the format of the used structures, see
log_buf_vmcoreinfo_setup().
Is the CONFIG_VMCORE_INFO code shared with the kmemdump, please?
> +}
> +EXPORT_SYMBOL_GPL(kmsg_kmemdump_register);
> +
Best Regards,
Petr
