On Tue, Sep 16, 2025 at 8:52 AM Steven Rostedt rostedt@goodmis.org wrote:
On Mon, 15 Sep 2025 18:19:53 -0700 Kalesh Singh kaleshsingh@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-kaleshsingh@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