On Thu, 3 Sep 2026 21:24:07 +0800 Pengfei Li <[email protected]> wrote:
> Add a lock-free hash map (ftrace_stackmap) that deduplicates kernel > stack traces for the ftrace ring buffer. Instead of storing full > stack traces (80-160 bytes each) in the ring buffer for every event, > ftrace can store a 4-byte stack_id when the stackmap option is enabled. Thanks, I have some comments below. [...] > > Kernel command line parameter: > - ftrace_stackmap.bits=N: set map capacity (2^N unique stacks, > range 10-18, default 14) Ah, this kernel cmdline parameter is also be a separated patch, because this is not fundamentary needed. > > Signed-off-by: Pengfei Li <[email protected]> It is OK to use an official address for SoB, but to make sure this address work, please at least Cc to this address. > --- > kernel/trace/Kconfig | 22 + > kernel/trace/Makefile | 1 + > kernel/trace/trace_stackmap.c | 871 ++++++++++++++++++++++++++++++++++ > kernel/trace/trace_stackmap.h | 55 +++ > 4 files changed, 949 insertions(+) > create mode 100644 kernel/trace/trace_stackmap.c > create mode 100644 kernel/trace/trace_stackmap.h [...] > +/* --- Stats --- */ > + > +static int stackmap_stat_show(struct seq_file *m, void *v) > +{ > + struct ftrace_stackmap *smap = m->private; > + u64 successes = 0, drops = 0; > + u32 entries; > + int cpu; > + > + if (!smap) { > + seq_puts(m, "stackmap not initialized\n"); > + return 0; > + } > + You also need down_read(&smap->reader_sem) here for serializing. > + entries = atomic_read(&smap->next_elt); > + for_each_possible_cpu(cpu) { > + successes += local_read(per_cpu_ptr(smap->successes, cpu)); > + drops += local_read(per_cpu_ptr(smap->drops, cpu)); > + } > + > + seq_printf(m, "entries: %u / %u\n", entries, smap->max_elts); > + seq_printf(m, "table_size: %u\n", smap->map_size); > + seq_printf(m, "successes: %llu\n", successes); > + seq_printf(m, "drops: %llu\n", drops); > + if (successes + drops > 0) { > + /* > + * mul_u64_u64_div_u64() uses a 128-bit intermediate, so > + * (successes * 100) cannot overflow even when successes > + * approaches U64_MAX on a very long trace. > + */ > + successes = mul_u64_u64_div_u64(successes, 100, successes + > drops); > + } else { > + successes = 0; > + } > + seq_printf(m, "success_rate: %llu%%\n", successes); and up_read(&smap->reader_sem) too. > + return 0; > +} > + > +static int stackmap_stat_open(struct inode *inode, struct file *file) > +{ > + return single_open(file, stackmap_stat_show, inode->i_private); > +} > + > +const struct file_operations ftrace_stackmap_stat_fops = { > + .open = stackmap_stat_open, > + .read = seq_read, > + .llseek = seq_lseek, > + .release = single_release, > +}; > + > +/* --- Binary export --- */ > + > +struct stackmap_bin_snapshot { > + /* > + * Use u64 (not size_t) so data[] is 8-byte aligned on both > + * 32-bit and 64-bit architectures. The IP array within data[] > + * is accessed as u64*, which would alignment-fault on strict > + * architectures (e.g. older ARM, SPARC) if data[] started at > + * a 4-byte boundary. > + */ > + u64 size; > + char data[]; > +}; > + > +static int stackmap_bin_open(struct inode *inode, struct file *file) > +{ > + struct ftrace_stackmap *smap = inode->i_private; > + struct stackmap_bin_snapshot *snap; > + struct ftrace_stackmap_bin_header *hdr; > + struct ftrace_stackmap_bin_entry *e; > + size_t alloc_size, off; > + u32 nr_entries, i, nr_stacks; > + > + if (!smap) > + return -ENODEV; > + > + /* > + * Serialize opens: only one snapshot may be pinned at a time > + * (see @bin_open). Released in stackmap_bin_release(). > + */ > + if (atomic_cmpxchg(&smap->bin_open, 0, 1) != 0) > + return -EBUSY; > + > + /* > + * Worst-case allocation size: every populated entry uses a > + * full-depth stack. The (+1) gives one slack slot in case a > + * concurrent insert lands between this snapshot and iteration. > + * The loop below performs an explicit bounds check anyway. > + * > + * At bits=18 this caps at ~135 MB. The file is mode 0440 > + * (TRACE_MODE_READ), so only privileged users can open it. Hmm, this is too huge to make a copy inside the kernel. If the stackmap is only increasing, and can avoid resetting by reader_sem, what about rewriting this as a raw-output mode of seq_file? You can use seq_write() to seq_file buffer. Thank you, -- Masami Hiramatsu (Google) <[email protected]>
