ring-buffer: Fixes for 7.1

ring-buffer fixes for 7.1:

- Fix reporting MISSED EVENTS in trace iterator

When the "trace" file is read with tracing enabled, if the writer were to pass the iterator reader, it resets, sets a "missed_events" flag and continues. The tracing output checks for missed events and if there are some, it prints out "[LOST EVENTS]" to let the user know events were dropped.

But the clearing of the missed_events happened when the tracing system queried the ring buffer iterator about missed events. This was premature as the ring buffer is per CPU, and the tracing code reads all the CPU buffers and checks for missed events when it is read. If the CPU iterator that had missed events isn't printed next, the output for the LOST EVENTS is lost.

Clear the missed_events flag when the iterator moves to the next event and not when the missed_events flag is queried. Also clear it on reset.

- Flush and stop the persistent ring buffer on panic

On panic the persistent ring buffer is used to debug what caused the panic. But on some architectures, it requires flushing the memory from cache, otherwise, the ring buffer persistent memory may not have the last events and this could also cause the ring buffer to be corrupted on the next boot.

- Fix nr_subbufs initialization in simple_ring_buffer_init_mm

The remote simple ring buffer meta data nr_subbufs is initialized too early and gets cleared later on, making it zero and not reflect the actual number of sub-buffers.

- Fix unload_page for simple_ring_buffer init rollback

On error, the pages loaded need to be unloaded. To unload a page it is expected that: page = load_page(va); -> unload_page(page). But the code was doing: unload_page(va) and not unload_page(page).

- Create output file from cmd_check_undefined

The check for undefined symbols checks if the file *.o.checked exists and if so it skips doing the work. But the *.o.checked file never was created making every build do the work even when it was already done previously.

git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace.git
ring-buffer/fixes

Head SHA1: 057caace5214da3b457bbd295e1a2ad34d3685ea

David Carlier (1):
tracing: Fix nr_subbufs initialization in simple_ring_buffer_init_mm()

Masami Hiramatsu (Google) (1):
ring-buffer: Flush and stop persistent ring buffer on panic

Steven Rostedt (1):
ring-buffer: Fix reporting of missed events in iterator

Thomas Weißschuh (1):
tracing: Create output file from cmd_check_undefined

Vincent Donnefort (1):
tracing: Fix unload_page for simple_ring_buffer init rollback

----

 arch/alpha/include/asm/Kbuild        |  1 +
 arch/arc/include/asm/Kbuild          |  1 +
 arch/arm/include/asm/Kbuild          |  1 +
 arch/arm64/include/asm/ring_buffer.h | 10 ++++++++++
 arch/csky/include/asm/Kbuild         |  1 +
 arch/hexagon/include/asm/Kbuild      |  1 +
 arch/loongarch/include/asm/Kbuild    |  1 +
 arch/m68k/include/asm/Kbuild         |  1 +
 arch/microblaze/include/asm/Kbuild   |  1 +
 arch/mips/include/asm/Kbuild         |  1 +
 arch/nios2/include/asm/Kbuild        |  1 +
 arch/openrisc/include/asm/Kbuild     |  1 +
 arch/parisc/include/asm/Kbuild       |  1 +
 arch/powerpc/include/asm/Kbuild      |  1 +
 arch/riscv/include/asm/Kbuild        |  1 +
 arch/s390/include/asm/Kbuild         |  1 +
 arch/sh/include/asm/Kbuild           |  1 +
 arch/sparc/include/asm/Kbuild        |  1 +
 arch/um/include/asm/Kbuild           |  1 +
 arch/x86/include/asm/Kbuild          |  1 +
 arch/xtensa/include/asm/Kbuild       |  1 +
 include/asm-generic/ring_buffer.h    | 13 +++++++++++++
 kernel/trace/Makefile                |  3 ++-
 kernel/trace/ring_buffer.c           | 30 +++++++++++++++++++++++++-----
 kernel/trace/simple_ring_buffer.c    |  4 ++--
 25 files changed, 72 insertions(+), 8 deletions(-)
 create mode 100644 arch/arm64/include/asm/ring_buffer.h
 create mode 100644 include/asm-generic/ring_buffer.h

From: Steven Rostedt <[email protected]>

When tracing is active while reading the trace file, if the iterator reading the buffer detects that the writer has passed the iterator head, it will reset and set a "missed events" flag. This flag is passed to the output processing to show the user that events were missed:

CPU:4 [LOST EVENTS]

The problem is that the flag is reset after it is checked in ring_buffer_iter_dropped(). But the "trace" file iterates over all the CPU ring buffers and it will check if they are dropped when figuring out which buffer to print next. This prematurely clears the missed_events flag if the CPU buffer with the missed events is not the one that is printed next.

On the iteration where the CPU buffer with the missed events is printed, the check if it had missed events would return false and the output does not show that events were missed.

Do not reset the missed_events flag when checking if there were missed events, but instead clear it when moving the iterator head to the next event.

---
 kernel/trace/ring_buffer.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

index 5326924615a4..fcd93d49851e 100644
@@ -5407,6 +5407,7 @@ static void rb_iter_reset(struct ring_buffer_iter *iter)
 	iter->head_page = cpu_buffer->reader_page;
 	iter->head = cpu_buffer->reader_page->read;
 	iter->next_event = iter->head;
+	iter->missed_events = 0;
 
 	iter->cache_reader_page = iter->head_page;
 	iter->cache_read = cpu_buffer->read;

@@ -6086,10 +6087,7 @@ ring_buffer_peek(struct trace_buffer *buffer, int cpu, u64 *ts,
  */
 bool ring_buffer_iter_dropped(struct ring_buffer_iter *iter)
 {
-	bool ret = iter->missed_events != 0;
-
-	iter->missed_events = 0;
-	return ret;
+	return iter->missed_events != 0;
 }
 EXPORT_SYMBOL_GPL(ring_buffer_iter_dropped);
 

@@ -6251,7 +6249,7 @@ void ring_buffer_iter_advance(struct ring_buffer_iter *iter)
 	unsigned long flags;
 
 	raw_spin_lock_irqsave(&cpu_buffer->reader_lock, flags);
-
+	iter->missed_events = 0;
 	rb_advance_iter(iter);
 
 	raw_spin_unlock_irqrestore(&cpu_buffer->reader_lock, flags);

2.53.0

From: "Masami Hiramatsu (Google)" <[email protected]>

On real hardware, panic and machine reboot may not flush hardware cache to memory. This means the persistent ring buffer, which relies on a coherent state of memory, may not have its events written to the buffer and they may be lost. Moreover, there may be inconsistency with the counters which are used for validation of the integrity of the persistent ring buffer which may cause all data to be discarded.

To avoid this issue, stop recording of the ring buffer on panic and flush the cache of the ring buffer's memory.

---
 arch/alpha/include/asm/Kbuild        |  1 +
 arch/arc/include/asm/Kbuild          |  1 +
 arch/arm/include/asm/Kbuild          |  1 +
 arch/arm64/include/asm/ring_buffer.h | 10 ++++++++++
 arch/csky/include/asm/Kbuild         |  1 +
 arch/hexagon/include/asm/Kbuild      |  1 +
 arch/loongarch/include/asm/Kbuild    |  1 +
 arch/m68k/include/asm/Kbuild         |  1 +
 arch/microblaze/include/asm/Kbuild   |  1 +
 arch/mips/include/asm/Kbuild         |  1 +
 arch/nios2/include/asm/Kbuild        |  1 +
 arch/openrisc/include/asm/Kbuild     |  1 +
 arch/parisc/include/asm/Kbuild       |  1 +
 arch/powerpc/include/asm/Kbuild      |  1 +
 arch/riscv/include/asm/Kbuild        |  1 +
 arch/s390/include/asm/Kbuild         |  1 +
 arch/sh/include/asm/Kbuild           |  1 +
 arch/sparc/include/asm/Kbuild        |  1 +
 arch/um/include/asm/Kbuild           |  1 +
 arch/x86/include/asm/Kbuild          |  1 +
 arch/xtensa/include/asm/Kbuild       |  1 +
 include/asm-generic/ring_buffer.h    | 13 +++++++++++++
 kernel/trace/ring_buffer.c           | 22 ++++++++++++++++++++++
 23 files changed, 65 insertions(+)
 create mode 100644 arch/arm64/include/asm/ring_buffer.h
 create mode 100644 include/asm-generic/ring_buffer.h

index 483965c5a4de..b154b4e3dfa8 100644
@@ -5,4 +5,5 @@ generic-y += agp.h
 generic-y += asm-offsets.h
 generic-y += kvm_para.h
 generic-y += mcs_spinlock.h
+generic-y += ring_buffer.h
 generic-y += text-patching.h

index 4c69522e0328..483caacc6988 100644
@@ -5,5 +5,6 @@ generic-y += extable.h
 generic-y += kvm_para.h
 generic-y += mcs_spinlock.h
 generic-y += parport.h
+generic-y += ring_buffer.h
 generic-y += user.h
 generic-y += text-patching.h

index 03657ff8fbe3..decad5f2c826 100644
@@ -3,6 +3,7 @@ generic-y += early_ioremap.h
 generic-y += extable.h
 generic-y += flat.h
 generic-y += parport.h
+generic-y += ring_buffer.h
 
 generated-y += mach-types.h
 generated-y += unistd-nr.h

new file mode 100644
index 000000000000..62316c406888
@@ -0,0 +1,10 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef _ASM_ARM64_RING_BUFFER_H
+#define _ASM_ARM64_RING_BUFFER_H
+
+#include <asm/cacheflush.h>
+
+/* Flush D-cache on persistent ring buffer */
+#define arch_ring_buffer_flush_range(start, end)	dcache_clean_pop(start, end)
+
+#endif /* _ASM_ARM64_RING_BUFFER_H */

index 3a5c7f6e5aac..7dca0c6cdc84 100644
@@ -9,6 +9,7 @@ generic-y += qrwlock.h
 generic-y += qrwlock_types.h
 generic-y += qspinlock.h
 generic-y += parport.h
+generic-y += ring_buffer.h
 generic-y += user.h
 generic-y += vmlinux.lds.h
 generic-y += text-patching.h

index 1efa1e993d4b..0f887d4238ed 100644
@@ -5,4 +5,5 @@ generic-y += extable.h
 generic-y += iomap.h
 generic-y += kvm_para.h
 generic-y += mcs_spinlock.h
+generic-y += ring_buffer.h
 generic-y += text-patching.h

index 9034b583a88a..7e92957baf6a 100644
@@ -10,5 +10,6 @@ generic-y += qrwlock.h
 generic-y += user.h
 generic-y += ioctl.h
 generic-y += mmzone.h
+generic-y += ring_buffer.h
 generic-y += statfs.h
 generic-y += text-patching.h

index b282e0dd8dc1..62543bf305ff 100644
@@ -3,5 +3,6 @@ generated-y += syscall_table.h
 generic-y += extable.h
 generic-y += kvm_para.h
 generic-y += mcs_spinlock.h
+generic-y += ring_buffer.h
 generic-y += spinlock.h
 generic-y += text-patching.h

index 7178f990e8b3..0030309b47ad 100644
@@ -5,6 +5,7 @@ generic-y += extable.h
 generic-y += kvm_para.h
 generic-y += mcs_spinlock.h
 generic-y += parport.h
+generic-y += ring_buffer.h
 generic-y += syscalls.h
 generic-y += tlb.h
 generic-y += user.h

index 684569b2ecd6..9771c3d85074 100644
@@ -12,5 +12,6 @@ generic-y += mcs_spinlock.h
 generic-y += parport.h
 generic-y += qrwlock.h
 generic-y += qspinlock.h
+generic-y += ring_buffer.h
 generic-y += user.h
 generic-y += text-patching.h

index 28004301c236..0a2530964413 100644
@@ -5,6 +5,7 @@ generic-y += cmpxchg.h
 generic-y += extable.h
 generic-y += kvm_para.h
 generic-y += mcs_spinlock.h
+generic-y += ring_buffer.h
 generic-y += spinlock.h
 generic-y += user.h
 generic-y += text-patching.h

index cef49d60d74c..8aa34621702d 100644
@@ -8,4 +8,5 @@ generic-y += spinlock_types.h
 generic-y += spinlock.h
 generic-y += qrwlock_types.h
 generic-y += qrwlock.h
+generic-y += ring_buffer.h
 generic-y += user.h

index 4fb596d94c89..d48d158f7241 100644
@@ -4,4 +4,5 @@ generated-y += syscall_table_64.h
 generic-y += agp.h
 generic-y += kvm_para.h
 generic-y += mcs_spinlock.h
+generic-y += ring_buffer.h
 generic-y += user.h

index 2e23533b67e3..805b5aeebb6f 100644
@@ -5,4 +5,5 @@ generated-y += syscall_table_spu.h
 generic-y += agp.h
 generic-y += mcs_spinlock.h
 generic-y += qrwlock.h
+generic-y += ring_buffer.h
 generic-y += early_ioremap.h

index bd5fc9403295..7721b63642f4 100644
@@ -14,5 +14,6 @@ generic-y += ticket_spinlock.h
 generic-y += qrwlock.h
 generic-y += qrwlock_types.h
 generic-y += qspinlock.h
+generic-y += ring_buffer.h
 generic-y += user.h
 generic-y += vmlinux.lds.h

index 80bad7de7a04..0c1fc47c3ba0 100644
@@ -7,3 +7,4 @@ generated-y += unistd_nr.h
 generic-y += asm-offsets.h
 generic-y += mcs_spinlock.h
 generic-y += mmzone.h
+generic-y += ring_buffer.h

index 4d3f10ed8275..f0403d3ee8ab 100644
@@ -3,4 +3,5 @@ generated-y += syscall_table.h
 generic-y += kvm_para.h
 generic-y += mcs_spinlock.h
 generic-y += parport.h
+generic-y += ring_buffer.h
 generic-y += text-patching.h

index 17ee8a273aa6..49c6bb326b75 100644
@@ -4,4 +4,5 @@ generated-y += syscall_table_64.h
 generic-y += agp.h
 generic-y += kvm_para.h
 generic-y += mcs_spinlock.h
+generic-y += ring_buffer.h
 generic-y += text-patching.h

index 1b9b82bbe322..2a1629ba8140 100644
@@ -17,6 +17,7 @@ generic-y += module.lds.h
 generic-y += parport.h
 generic-y += percpu.h
 generic-y += preempt.h
+generic-y += ring_buffer.h
 generic-y += runtime-const.h
 generic-y += softirq_stack.h
 generic-y += switch_to.h

index 4566000e15c4..078fd2c0d69d 100644
@@ -14,3 +14,4 @@ generic-y += early_ioremap.h
 generic-y += fprobe.h
 generic-y += mcs_spinlock.h
 generic-y += mmzone.h
+generic-y += ring_buffer.h

index 13fe45dea296..e57af619263a 100644
@@ -6,5 +6,6 @@ generic-y += mcs_spinlock.h
 generic-y += parport.h
 generic-y += qrwlock.h
 generic-y += qspinlock.h
+generic-y += ring_buffer.h
 generic-y += user.h
 generic-y += text-patching.h

new file mode 100644
index 000000000000..201d2aee1005
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Generic arch dependent ring_buffer macros.
+ */
+#ifndef __ASM_GENERIC_RING_BUFFER_H__
+#define __ASM_GENERIC_RING_BUFFER_H__
+
+#include <linux/cacheflush.h>
+
+/* Flush cache on ring buffer range if needed. Do nothing by default. */
+#define arch_ring_buffer_flush_range(start, end)	do { } while (0)
+
+#endif /* __ASM_GENERIC_RING_BUFFER_H__ */

index fcd93d49851e..7b07d2004cc6 100644
@@ -7,6 +7,7 @@
 #include <linux/ring_buffer_types.h>
 #include <linux/sched/isolation.h>
 #include <linux/trace_recursion.h>
+#include <linux/panic_notifier.h>
 #include <linux/trace_events.h>
 #include <linux/ring_buffer.h>
 #include <linux/trace_clock.h>

@@ -31,6 +32,7 @@
 #include <linux/oom.h>
 #include <linux/mm.h>
 
+#include <asm/ring_buffer.h>
 #include <asm/local64.h>
 #include <asm/local.h>
 #include <asm/setup.h>

@@ -559,6 +561,7 @@ struct trace_buffer {
 
 	unsigned long			range_addr_start;
 	unsigned long			range_addr_end;
+	struct notifier_block		flush_nb;
 
 	struct ring_buffer_meta		*meta;
 

@@ -2521,6 +2524,16 @@ static void rb_free_cpu_buffer(struct ring_buffer_per_cpu *cpu_buffer)
 	kfree(cpu_buffer);
 }
 
+/* Stop recording on a persistent buffer and flush cache if needed. */
+static int rb_flush_buffer_cb(struct notifier_block *nb, unsigned long event, void *data)
+{
+	struct trace_buffer *buffer = container_of(nb, struct trace_buffer, flush_nb);
+
+	ring_buffer_record_off(buffer);
+	arch_ring_buffer_flush_range(buffer->range_addr_start, buffer->range_addr_end);
+	return NOTIFY_DONE;
+}
+
 static struct trace_buffer *alloc_buffer(unsigned long size, unsigned flags,
 					 int order, unsigned long start,
 					 unsigned long end,

@@ -2651,6 +2664,12 @@ static struct trace_buffer *alloc_buffer(unsigned long size, unsigned flags,
 
 	mutex_init(&buffer->mutex);
 
+	/* Persistent ring buffer needs to flush cache before reboot. */
+	if (start && end) {
+		buffer->flush_nb.notifier_call = rb_flush_buffer_cb;
+		atomic_notifier_chain_register(&panic_notifier_list, &buffer->flush_nb);
+	}
+
 	return_ptr(buffer);
 
  fail_free_buffers:

@@ -2749,6 +2768,9 @@ ring_buffer_free(struct trace_buffer *buffer)
 {
 	int cpu;
 
+	if (buffer->range_addr_start && buffer->range_addr_end)
+		atomic_notifier_chain_unregister(&panic_notifier_list, &buffer->flush_nb);
+
 	cpuhp_state_remove_instance(CPUHP_TRACE_RB_PREPARE, &buffer->node);
 
 	irq_work_sync(&buffer->irq_work.work);

2.53.0

From: David Carlier <[email protected]>

nr_subbufs in the ring buffer metadata is always initialized to zero because it is assigned from cpu_buffer->nr_pages before the page initialization loop has run. While nr_subbufs is not currently read by the kernel, it should reflect the actual buffer geometry in the meta page for correctness.

Move the assignment after the page loop so that cpu_buffer->nr_pages holds the final count.

---
 kernel/trace/simple_ring_buffer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 02af2297ae5a..f731f14d0ff7 100644
@@ -395,7 +395,6 @@ int simple_ring_buffer_init_mm(struct simple_rb_per_cpu *cpu_buffer,
 
 	memset(cpu_buffer->meta, 0, sizeof(*cpu_buffer->meta));
 	cpu_buffer->meta->meta_page_size = PAGE_SIZE;
-	cpu_buffer->meta->nr_subbufs = cpu_buffer->nr_pages;
 
 	/* The reader page is not part of the ring initially */
 	page = load_page(desc->page_va[0]);

@@ -437,6 +436,7 @@ int simple_ring_buffer_init_mm(struct simple_rb_per_cpu *cpu_buffer,
 		return ret;
 	}
 
+	cpu_buffer->meta->nr_subbufs = cpu_buffer->nr_pages;
 	/* Close the ring */
 	bpage->link.next = &cpu_buffer->tail_page->link;
 	cpu_buffer->tail_page->link.prev = &bpage->link;

2.53.0

From: Vincent Donnefort <[email protected]>

The unload_page callback expects the return value of load_page() as its
argument: ret = load_page(va); unload(ret). Fix the rollback code in simple_ring_buffer_init_mm() where the descriptor's VA is used instead of the loaded page address.

---
 kernel/trace/simple_ring_buffer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index f731f14d0ff7..f4642f5adda3 100644
@@ -430,7 +430,7 @@ int simple_ring_buffer_init_mm(struct simple_rb_per_cpu *cpu_buffer,
 
 	if (ret) {
 		for (i--; i >= 0; i--)
-			unload_page((void *)desc->page_va[i]);
+			unload_page(bpages[i].page);
 		unload_page(cpu_buffer->meta);
 
 		return ret;

2.53.0

From: =?UTF-8?q?Thomas=20Wei=C3=9Fschuh?= <[email protected]>

As the output file is currently never created, the check will run every time, even if the inputs have not changed.

Create an empty output file which allows make to skip the execution when it is not necessary.

---
 kernel/trace/Makefile | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

index 9b0834134cae..8d3d96e847d8 100644
@@ -154,7 +154,8 @@ quiet_cmd_check_undefined = NM      $<
               echo "Unexpected symbols in $<:" >&2; \
               echo "$$undefsyms" >&2; \
               false; \
-          fi
+          fi; \
+          touch $@
 
 $(obj)/%.o.checked: $(obj)/%.o $(obj)/undefsyms_base.o FORCE
 	$(call if_changed,check_undefined)

2.53.0