On Tue, Sep 16, 2025 at 8:52 AM Steven Rostedt <rost...@goodmis.org> wrote: > > On Mon, 15 Sep 2025 18:19:53 -0700 > Kalesh Singh <kaleshsi...@google.com> wrote: > > > > Hi Steve, > > > > Thanks for the comments and suggestion you are right we can use bpf to > > get the comm. There is nothing special about this trace event. I will > > drop comm in the next revision. > > > > The reason I did the task_struct parameter (current): I believe there > > is a limitation that we must specify at least 1 parameter to the > > TRACE_EVENT() PROTO and ARGS macros. > > OK, then this is another issue. We don't want tracepoint "markers". > Each tracepoint can take up to 5K in memory due to the code it > generates and the meta data to control it. > > For something like that, we highly recommend dynamic probes (fprobes, > kprobes, etc). > > The only purpose of a static tracepoint is to get data within a > function that is too difficult to get via a probe. It should never be > used as a trigger where its purpose is "we hit this path".
Hi Steve, I completely agree with the principle that static tracepoints shouldn't be used as markers if a dynamic probe will suffice. The intent here is to avoid introducing overhead in the common case to avoid regressing mmap, munmap, and other syscall latencies; while still providing observability for the max vma_count exceeded failure condition. The original centralized check (before previous review rounds) was indeed in a dedicated function, exceeds_max_map_count(), where a kprobe/fprobe could have been easily attached without impacting the common path. This was changed due to previous review feedback to the capacity based vma_count_remaining() which necessitated the check to be done externally by the callers: https://lore.kernel.org/r/20250903232437.1454293-1-kaleshsi...@google.com/ Would you be ok with something like: trace_max_vma_count_exceeded(mm); TP_STRUCT__entry( __field(unsigned int, mm_id) __field(unsigned int vma_count) ) mm_id would be the hash of the mm_struct ptr similar to rss_stat and the vma_count is the current vma count (some syscalls have different requirements on the capacity remaining: mremap requires 6 available slots, other syscalls require 1). Thanks, Kalesh > > -- Steve
