On August 28, 2025 5:17:18 PM GMT-03:00, Steven Rostedt <rostedt@xxxxxxxxxx> wrote: >On Thu, 28 Aug 2025 12:18:39 -0700 >Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote: > >> On Thu, 28 Aug 2025 at 11:58, Arnaldo Carvalho de Melo >> <arnaldo.melo@xxxxxxxxx> wrote: >> > > >> > >Give the damn thing an actual filename or something *useful*, not a >> > >number that user space can't even necessarily match up to anything. >> > >> > A build ID? >> >> I think that's a better thing than the disgusting inode number, yes. > >I don't care what it is. I picked inode/device just because it was the >only thing I saw available. I'm not sure build ID is appropriate either. > >> >> That said, I think they are problematic too, in that I don't think >> they are universally available, so if you want to trace some >> executable without build ids - and there are good reasons to do that - >> you might hate being limited that way. >> >> So I think you'd be much better off with just actual pathnames. > >As you mentioned below, the reason I avoided path names is that they >take up too much of the ring buffer, and would be duplicated all over >the place. I've run this for a while, and it only picked up a couple of >hundred paths while the trace had several thousand stack traces. > >> >> Are there no trace events for "mmap this path"? Create a good u64 hash >> from the contents of a 'struct path' (which is just two pointers: the >> dentry and the mnt) when mmap'ing the file, and then you can just >> associate the stack trace entry with that hash. > >I would love to have a hash to use. The next patch does the mapping of >the inode numbers to their path name. It can The path name is a nice to have detail, but a content based hash is what we want, no? Tracing/profiling has to be about contents of files later used for analysis, and filenames provide no guarantee about that. - Arnaldo easily be switched over to >do the same with a hash number. > >> >> That should be simple and straightforward, and hashing two pointers >> should be simple and straightforward. > >Would a hash of these pointers have any collisions? That would be bad. > >Hmm, I just tried using the pointer to vma->vm_file->f_inode, and that >gives me a unique number. Then I just need to map that back to the path name: > > trace-cmd-1016 [002] ...1. 34.675646: inode_cache: inode=ffff8881007ed428 dev=[254:3] path=/usr/lib/x86_64-linux-gnu/libc.so.6 > trace-cmd-1016 [002] ...1. 34.675893: inode_cache: inode=ffff88811970e648 dev=[254:3] path=/usr/local/lib64/libtracefs.so.1.8.2 > trace-cmd-1016 [002] ...1. 34.675933: inode_cache: inode=ffff88811970b8f8 dev=[254:3] path=/usr/local/lib64/libtraceevent.so.1.8.4 > trace-cmd-1016 [002] ...1. 34.675981: inode_cache: inode=ffff888110b78ba8 dev=[254:3] path=/usr/lib/x86_64-linux-gnu/libzstd.so.1.5.7 > bash-1007 [003] ...1. 34.677316: inode_cache: inode=ffff888103f05d38 dev=[254:3] path=/usr/bin/bash > bash-1007 [003] ...1. 35.432951: inode_cache: inode=ffff888116be94b8 dev=[254:3] path=/usr/lib/x86_64-linux-gnu/libtinfo.so.6.5 > bash-1018 [005] ...1. 36.104543: inode_cache: inode=ffff8881007e9dc8 dev=[254:3] path=/usr/lib/x86_64-linux-gnu/ld-linux-x86-64.so.2 > bash-1018 [005] ...1. 36.110407: inode_cache: inode=ffff888110b78298 dev=[254:3] path=/usr/lib/x86_64-linux-gnu/libz.so.1.3.1 > bash-1018 [005] ...1. 36.110536: inode_cache: inode=ffff888103d09dc8 dev=[254:3] path=/usr/local/bin/trace-cmd > >I just swapped out the inode with the above (unsigned long)vma->vm_file->f_inode, >and it appears to be unique. > >Thus, I could use that as the "hash" value and then the above could be turned into: > > trace-cmd-1016 [002] ...1. 34.675646: inode_cache: hash=ffff8881007ed428 path=/usr/lib/x86_64-linux-gnu/libc.so.6 > trace-cmd-1016 [002] ...1. 34.675893: inode_cache: hash=ffff88811970e648 path=/usr/local/lib64/libtracefs.so.1.8.2 > trace-cmd-1016 [002] ...1. 34.675933: inode_cache: hash=ffff88811970b8f8 path=/usr/local/lib64/libtraceevent.so.1.8.4 > trace-cmd-1016 [002] ...1. 34.675981: inode_cache: hash=ffff888110b78ba8 path=/usr/lib/x86_64-linux-gnu/libzstd.so.1.5.7 > bash-1007 [003] ...1. 34.677316: inode_cache: hash=ffff888103f05d38 path=/usr/bin/bash > bash-1007 [003] ...1. 35.432951: inode_cache: hash=ffff888116be94b8 path=/usr/lib/x86_64-linux-gnu/libtinfo.so.6.5 > bash-1018 [005] ...1. 36.104543: inode_cache: hash=ffff8881007e9dc8 path=/usr/lib/x86_64-linux-gnu/ld-linux-x86-64.so.2 > bash-1018 [005] ...1. 36.110407: inode_cache: hash=ffff888110b78298 path=/usr/lib/x86_64-linux-gnu/libz.so.1.3.1 > bash-1018 [005] ...1. 36.110536: inode_cache: hash=ffff888103d09dc8 path=/usr/local/bin/trace-cmd > >This would mean the readers of the userstacktrace_delay need to also >have this event enabled to do the mappings. But that shouldn't be an >issue. > >-- Steve > - Arnaldo
