Kexec Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH kexec-tools v3] util_lib/elf_info: stream lockless printk ring records
@ 2026-08-18 14:29 Serapheim Dimitropoulos
  2026-08-24 15:38 ` Simon Horman
  0 siblings, 1 reply; 2+ messages in thread
From: Serapheim Dimitropoulos @ 2026-08-18 14:29 UTC (permalink / raw)
  To: kexec
  Cc: Simon Horman, John Ogness, Petr Mladek, Philipp Rudo, Baoquan He,
	Serapheim Dimitropoulos

From: Serapheim Dimitropoulos <sdimitropoulos@coreweave.com>

We came across this while testing kdump on an arm64 system with a 64 MiB
printk text ring and an 850 MiB capture kernel. vmcore-dmesg was run from
the capture kernel before makedumpfile, but it was killed with SIGKILL
before it could produce any output.

The lockless ringbuffer reader copies the complete descriptor, info, and
text rings before it starts processing records. For the above setup these
copies raised peak RSS to about 345 MiB. This is a significant part of the
memory available to the capture kernel, and most of the copied data is only
visited once.

Keep the ring file offsets and read each active record directly instead.
Allocate space for one descriptor, one info record, and one text record.
Leave the existing ring geometry parsing, types, traversal, and
record-state handling unchanged.

The old and new readers produced byte-identical output for committed and
finalized records, missed and empty records, wrapped text, non-printable
bytes, descriptor ID wraparound, and a dense 100000-record fixture.

For the dense fixture with a 64 MiB text ring, peak RSS dropped from
345276 KiB to 1276 KiB. Runtime increased from 1.98 to 2.09 seconds due to
the per-record reads.

Fixes: 4149df9005f2 ("printk: add support for lockless ringbuffer")
Signed-off-by: Serapheim Dimitropoulos <sdimitropoulos@coreweave.com>
---
Changes in v3:
- Keep the patch focused on changing where ring data is read.
- Remove the new validation and overflow-checking framework, and retain the
  existing types, ring geometry parsing, arithmetic, traversal, and
  record-state handling.
- Use direct pread() calls and the same error handling style as the existing
  reader.
- Re-run make distcheck, byte-for-byte small and dense fixture comparisons,
  peak RSS measurements, and ASan/UBSan tests of the printk-ring path.
- Link to v2: https://lore.kernel.org/r/20260803-vmcore-dmesg-stream-upstream-v2-1-cb7f7449bc1b@coreweave.com
---
 util_lib/elf_info.c | 94 ++++++++++++++++++++++++++++-------------------------
 1 file changed, 49 insertions(+), 45 deletions(-)

diff --git a/util_lib/elf_info.c b/util_lib/elf_info.c
index 36eb895..d6e5ffc 100644
--- a/util_lib/elf_info.c
+++ b/util_lib/elf_info.c
@@ -960,12 +960,15 @@ struct prb_map {
 
 	char		*desc_ring;
 	unsigned long	desc_ring_count;
-	char		*descs;
+	uint64_t	desc_offset;
+	char		*desc;
 
-	char		*infos;
+	uint64_t	info_offset;
+	char		*info;
 
 	char		*text_data_ring;
 	unsigned long	text_data_ring_size;
+	uint64_t	text_data_offset;
 	char		*text_data;
 };
 
@@ -1061,7 +1064,7 @@ static uint64_t sizeof_ulong(void)
 	return (machine_pointer_bits() >> 3);
 }
 
-static void dump_record(struct prb_map *m, unsigned long id,
+static void dump_record(int fd, struct prb_map *m, unsigned long id,
 			void (*handler)(char*, unsigned int))
 {
 #define OUT_BUF_SIZE	4096
@@ -1078,9 +1081,16 @@ static void dump_record(struct prb_map *m, unsigned long id,
 	char *text;
 	char *desc;
 	int i;
+	int ret;
 
-	desc = m->descs + ((id % m->desc_ring_count) * prb_desc_sz);
-	info = m->infos + ((id % m->desc_ring_count) * printk_info_sz);
+	ret = pread(fd, m->desc, prb_desc_sz, m->desc_offset +
+		    ((id % m->desc_ring_count) * prb_desc_sz));
+	if (ret != prb_desc_sz) {
+		fprintf(stderr, "Failed to read desc of size %zu bytes: %s\n",
+			prb_desc_sz, strerror(errno));
+		exit(65);
+	}
+	desc = m->desc;
 
 	/* skip non-committed record */
 	state_var = get_ulong(desc + prb_desc_state_var_offset +
@@ -1088,6 +1098,15 @@ static void dump_record(struct prb_map *m, unsigned long id,
 	if (!record_committed(id, state_var))
 		return;
 
+	ret = pread(fd, m->info, printk_info_sz, m->info_offset +
+		    ((id % m->desc_ring_count) * printk_info_sz));
+	if (ret != printk_info_sz) {
+		fprintf(stderr, "Failed to read info of size %zu bytes: %s\n",
+			printk_info_sz, strerror(errno));
+		exit(65);
+	}
+	info = m->info;
+
 	begin = get_ulong(desc + prb_desc_text_blk_lpos_offset +
 			  prb_data_blk_lpos_begin_offset) %
 		m->text_data_ring_size;
@@ -1120,7 +1139,13 @@ static void dump_record(struct prb_map *m, unsigned long id,
 	if (next - begin < len)
 		len = next - begin;
 
-	text = m->text_data + begin;
+	ret = pread(fd, m->text_data, len, m->text_data_offset + begin);
+	if (ret != len) {
+		fprintf(stderr, "Failed to read text of size %hu bytes: %s\n",
+			len, strerror(errno));
+		exit(65);
+	}
+	text = m->text_data;
 
 	/* escape non-printable characters */
 	for (i = 0; i < len; i++) {
@@ -1176,58 +1201,37 @@ static void dump_dmesg_lockless(int fd, void (*handler)(char*, unsigned int))
 	m.desc_ring_count = 1 << struct_val_u32(m.desc_ring,
 					prb_desc_ring_count_bits_offset);
 	kaddr = get_ulong(m.desc_ring + prb_desc_ring_descs_offset);
-	m.descs = calloc(1, prb_desc_sz * m.desc_ring_count);
-	if (!m.descs) {
-		fprintf(stderr, "Failed to malloc %lu bytes for descs: %s\n",
-			prb_desc_sz * m.desc_ring_count, strerror(errno));
+	m.desc_offset = vaddr_to_offset(kaddr);
+	m.desc = calloc(1, prb_desc_sz);
+	if (!m.desc) {
+		fprintf(stderr, "Failed to malloc %zu bytes for desc: %s\n",
+			prb_desc_sz, strerror(errno));
 		exit(64);
 	}
-	ret = pread(fd, m.descs, prb_desc_sz * m.desc_ring_count,
-		    vaddr_to_offset(kaddr));
-	if (ret != prb_desc_sz * m.desc_ring_count) {
-		fprintf(stderr,
-			"Failed to read descs of size %lu bytes: %s\n",
-			prb_desc_sz * m.desc_ring_count, strerror(errno));
-		exit(65);
-	}
 
 	/* setup info ring */
 	kaddr = get_ulong(m.prb + prb_desc_ring_infos_offset);
-	m.infos = calloc(1, printk_info_sz * m.desc_ring_count);
-	if (!m.infos) {
-		fprintf(stderr, "Failed to malloc %lu bytes for infos: %s\n",
-			printk_info_sz * m.desc_ring_count, strerror(errno));
+	m.info_offset = vaddr_to_offset(kaddr);
+	m.info = calloc(1, printk_info_sz);
+	if (!m.info) {
+		fprintf(stderr, "Failed to malloc %zu bytes for info: %s\n",
+			printk_info_sz, strerror(errno));
 		exit(64);
 	}
-	ret = pread(fd, m.infos, printk_info_sz * m.desc_ring_count,
-		    vaddr_to_offset(kaddr));
-	if (ret != printk_info_sz * m.desc_ring_count) {
-		fprintf(stderr,
-			"Failed to read infos of size %lu bytes: %s\n",
-			printk_info_sz * m.desc_ring_count, strerror(errno));
-		exit(65);
-	}
 
 	/* setup text data ring */
 	m.text_data_ring = m.prb + printk_ringbuffer_text_data_ring_offset;
 	m.text_data_ring_size = 1 << struct_val_u32(m.text_data_ring,
 					prb_data_ring_size_bits_offset);
 	kaddr = get_ulong(m.text_data_ring + prb_data_ring_data_offset);
-	m.text_data = calloc(1, m.text_data_ring_size);
+	m.text_data_offset = vaddr_to_offset(kaddr);
+	m.text_data = calloc(1, UINT16_MAX);
 	if (!m.text_data) {
 		fprintf(stderr,
-			"Failed to malloc %lu bytes for text_data: %s\n",
-			m.text_data_ring_size, strerror(errno));
+			"Failed to malloc %u bytes for text_data: %s\n",
+			UINT16_MAX, strerror(errno));
 		exit(64);
 	}
-	ret = pread(fd, m.text_data, m.text_data_ring_size,
-		    vaddr_to_offset(kaddr));
-	if (ret != m.text_data_ring_size) {
-		fprintf(stderr,
-			"Failed to read text_data of size %lu bytes: %s\n",
-			m.text_data_ring_size, strerror(errno));
-		exit(65);
-	}
 
 	/* ready to go */
 
@@ -1237,14 +1241,14 @@ static void dump_dmesg_lockless(int fd, void (*handler)(char*, unsigned int))
 						atomic_long_t_counter_offset);
 
 	for (id = tail_id; id != head_id; id = id_inc(id))
-		dump_record(&m, id, handler);
+		dump_record(fd, &m, id, handler);
 
 	/* dump head record */
-	dump_record(&m, id, handler);
+	dump_record(fd, &m, id, handler);
 
 	free(m.text_data);
-	free(m.infos);
-	free(m.descs);
+	free(m.info);
+	free(m.desc);
 	free(m.prb);
 }
 

---
base-commit: 29898e149feb5c615b3ab6313260d58315bf1d8d
change-id: 20260728-vmcore-dmesg-stream-upstream-7da695989f95

Best regards,
-- 
Serapheim Dimitropoulos <sdimitropoulos@coreweave.com>



^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH kexec-tools v3] util_lib/elf_info: stream lockless printk ring records
  2026-08-18 14:29 [PATCH kexec-tools v3] util_lib/elf_info: stream lockless printk ring records Serapheim Dimitropoulos
@ 2026-08-24 15:38 ` Simon Horman
  0 siblings, 0 replies; 2+ messages in thread
From: Simon Horman @ 2026-08-24 15:38 UTC (permalink / raw)
  To: Serapheim Dimitropoulos
  Cc: kexec, John Ogness, Petr Mladek, Philipp Rudo, Baoquan He,
	Serapheim Dimitropoulos

On Tue, Aug 18, 2026 at 10:29:39AM -0400, Serapheim Dimitropoulos wrote:
> From: Serapheim Dimitropoulos <sdimitropoulos@coreweave.com>
> 
> We came across this while testing kdump on an arm64 system with a 64 MiB
> printk text ring and an 850 MiB capture kernel. vmcore-dmesg was run from
> the capture kernel before makedumpfile, but it was killed with SIGKILL
> before it could produce any output.
> 
> The lockless ringbuffer reader copies the complete descriptor, info, and
> text rings before it starts processing records. For the above setup these
> copies raised peak RSS to about 345 MiB. This is a significant part of the
> memory available to the capture kernel, and most of the copied data is only
> visited once.
> 
> Keep the ring file offsets and read each active record directly instead.
> Allocate space for one descriptor, one info record, and one text record.
> Leave the existing ring geometry parsing, types, traversal, and
> record-state handling unchanged.
> 
> The old and new readers produced byte-identical output for committed and
> finalized records, missed and empty records, wrapped text, non-printable
> bytes, descriptor ID wraparound, and a dense 100000-record fixture.
> 
> For the dense fixture with a 64 MiB text ring, peak RSS dropped from
> 345276 KiB to 1276 KiB. Runtime increased from 1.98 to 2.09 seconds due to
> the per-record reads.
> 
> Fixes: 4149df9005f2 ("printk: add support for lockless ringbuffer")
> Signed-off-by: Serapheim Dimitropoulos <sdimitropoulos@coreweave.com>
> ---
> Changes in v3:
> - Keep the patch focused on changing where ring data is read.
> - Remove the new validation and overflow-checking framework, and retain the
>   existing types, ring geometry parsing, arithmetic, traversal, and
>   record-state handling.
> - Use direct pread() calls and the same error handling style as the existing
>   reader.
> - Re-run make distcheck, byte-for-byte small and dense fixture comparisons,
>   peak RSS measurements, and ASan/UBSan tests of the printk-ring path.
> - Link to v2: https://lore.kernel.org/r/20260803-vmcore-dmesg-stream-upstream-v2-1-cb7f7449bc1b@coreweave.com

Thanks, applied.

- util_lib/elf_info: stream lockless printk ring records
  https://git.kernel.org/pub/scm/utils/kernel/kexec/kexec-tools.git/commit/?id=2bcd7801948d



^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-08-24 15:38 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-18 14:29 [PATCH kexec-tools v3] util_lib/elf_info: stream lockless printk ring records Serapheim Dimitropoulos
2026-08-24 15:38 ` Simon Horman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox