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.
Signed-off-by: Eugen Hristev <eugen.hristev@linaro.org>
---
include/linux/kmsg_dump.h | 6 ++++++
kernel/printk/printk.c | 13 +++++++++++++
2 files changed, 19 insertions(+)
diff --git a/include/linux/kmsg_dump.h b/include/linux/kmsg_dump.h
index 6055fc969877..cbe76c94710b 100644
--- a/include/linux/kmsg_dump.h
+++ b/include/linux/kmsg_dump.h
@@ -80,6 +80,8 @@ int kmsg_dump_register(struct kmsg_dumper *dumper);
int kmsg_dump_unregister(struct kmsg_dumper *dumper);
+void kmsg_kmemdump_register(void);
+
const char *kmsg_dump_reason_str(enum kmsg_dump_reason reason);
#else
static inline void kmsg_dump_desc(enum kmsg_dump_reason reason, const char *desc)
@@ -112,6 +114,10 @@ static inline int kmsg_dump_unregister(struct kmsg_dumper *dumper)
return -EINVAL;
}
+static inline void kmsg_kmemdump_register(void)
+{
+}
+
static inline const char *kmsg_dump_reason_str(enum kmsg_dump_reason reason)
{
return "Disabled";
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 057db78876cd..cf4aa86ef7af 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -48,6 +48,7 @@
#include <linux/sched/clock.h>
#include <linux/sched/debug.h>
#include <linux/sched/task_stack.h>
+#include <linux/kmemdump.h>
#include <linux/uaccess.h>
#include <asm/sections.h>
@@ -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));
+ 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));
+}
+EXPORT_SYMBOL_GPL(kmsg_kmemdump_register);
+
/**
* kmsg_dump_unregister - unregister a kmsg dumper.
* @dumper: pointer to the kmsg_dumper structure
--
2.43.0On 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
Hello Petr,
Thank you for your review.
On 5/5/25 18:25, Petr Mladek wrote:
> 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.
Yes, from my perspective this should be also enough. However, when
loading the generated core dump into crash tool , the tool first looks
for the prb pointer itself, and then stops if the pointer is not readable.
After the prb pointer is being found, the crash tool dereferences it ,
and looks at the indicated address for the actual memory.
That is why the pointer is also saved as a kmemdump region in my proof
of concept.
>
>> + 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 will double check whether the crash tool looks for these symbols or
only the memory, and come back with an answer
> 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);
>
>
Thank you. It may be that in my test case, the buffer was not
extended/reallocated with a bigger one.
> 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().
It appears that crash tool looks for the structures into vmlinux
symbols. It can be that this information is not available to some tools,
or vmlinux not available, in which case all the used structures format
and sizes need to be exported. But right now, the crash tool does not
work without vmlinux.
>
> Is the CONFIG_VMCORE_INFO code shared with the kmemdump, please?
I believe CONFIG_KMEMDUMP_COREIMAGE should select CONFIG_VMCORE_INFO
indeed, which is not done in my patches. Or I have not fully understood
your question ?
Eugen
>
>> +}
>> +EXPORT_SYMBOL_GPL(kmsg_kmemdump_register);
>> +
>
> Best Regards,
> Petr
On Mon 2025-05-05 18:51:19, Eugen Hristev wrote:
> Hello Petr,
>
> Thank you for your review.
>
> On 5/5/25 18:25, Petr Mladek wrote:
> > 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.
>
> Yes, from my perspective this should be also enough. However, when
> loading the generated core dump into crash tool , the tool first looks
> for the prb pointer itself, and then stops if the pointer is not readable.
> After the prb pointer is being found, the crash tool dereferences it ,
> and looks at the indicated address for the actual memory.
> That is why the pointer is also saved as a kmemdump region in my proof
> of concept.
I see. It makes perfect sense to store the pointer as well after all.
> >> + 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 will double check whether the crash tool looks for these symbols or
> only the memory, and come back with an answer
>
> > 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);
> >
> >
> Thank you. It may be that in my test case, the buffer was not
> extended/reallocated with a bigger one.
I guess so. A bigger buffer is allocated either when explicitly
requested by "log_buf_len=" command line option. Or when the kernel
is running on a huge system with many CPUs and log_buf_add_cpu()
decides that the default buffer is not big enough for backtraces from
all CPUs.
> > 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().
>
> It appears that crash tool looks for the structures into vmlinux
> symbols. It can be that this information is not available to some tools,
> or vmlinux not available, in which case all the used structures format
> and sizes need to be exported. But right now, the crash tool does not
> work without vmlinux.
> >
> > Is the CONFIG_VMCORE_INFO code shared with the kmemdump, please?
>
> I believe CONFIG_KMEMDUMP_COREIMAGE should select CONFIG_VMCORE_INFO
> indeed, which is not done in my patches. Or I have not fully understood
> your question ?
I do not see CONFIG_VMCORE_INFO selected in drivers/debug/Kconfig.
But maybe the dependency is defined another way.
Honestly, I did not study all these details. I focused primary on
the printk-related interface and commented what came to my mind.
Also I was not sure how the dumped memory can be analyzed. I expected
that it should be readable by the "crash" tool. But I did not see it explained
in Documentation/debug/kmemdump.rst.
Best Regards,
Petr
© 2016 - 2026 Red Hat, Inc.