On Fri, Jul 26, 2019 at 11:39:56AM -0700, Alexei Starovoitov wrote: [snip] > > > > 1. timeinstate: By hooking 2 programs onto sched_switch and cpu_frequency > > > > tracepoints, we are able to collect CPU power per-UID (specific app). Connor > > > > O'Brien is working on that. > > > > > > > > 2. inode to file path mapping: By hooking onto VFS tracepoints we are adding to > > > > the android kernels, we can collect data when the kernel resolves a file path > > > > to a inode/device number. A BPF map stores the inode/dev number (key) and the > > > > path (value). We have usecases where we need a high speed lookup of this > > > > without having to scan all the files in the filesystem. > > > > > > Can you share the link to vfs tracepoints you're adding? > > > Sounds like you're not going to attempt to upstream them knowing > > > Al's stance towards them? > > > May be there is a way we can do the feature you need, but w/o tracepoints? > > > > Yes, given Al's stance I understand the patch is not upstreamable. The patch > > is here: > > For tracepoint: > > https://android.googlesource.com/kernel/common/+/27d3bfe20558d279041af403a887e7bdbdcc6f24%5E%21/ > > this is way more than tracepoint. True there is some code that calls the tracepoint. I want to optimize it more but lets see I am ready to think more about it before doing it this way, based on your suggestions. > > For bpf program: > > https://android.googlesource.com/platform/system/bpfprogs/+/908f6cd718fab0de7a944f84628c56f292efeb17%5E%21/ > > what is unsafe_bpf_map_update_elem() in there? > The verifier comment sounds odd. > Could you describe the issue you see with the verifier? Will dig out the verifier issue I was seeing. I was just trying to get a prototype working so I did not go into verifier details much. > > I intended to submit the tracepoint only for the Android kernels, however if > > there is an upstream solution to this then that's even better since upstream can > > benefit. Were you thinking of a BPF helper function to get this data? > > I think the best way to evaluate the patches is whether they are upstreamable or not. > If they're not (like this case), it means that there is something wrong with their design > and if android decides to go with such approach it will only create serious issues long term. > Starting with the whole idea of dev+inode -> filepath cache. > dev+inode is not a unique identifier of the file. > In some filesystems two different files may have the same ino integer value. > Have you looked at 'struct file_handle' ? and name_to_handle_at ? > I think fhandle is the only way to get unique identifier of the file. > Could you please share more details why android needs this cache of dev+ino->path? I will follow-up with you on this by email off the list, thanks. thanks, - Joel
