Re: [PATCH 3/9] selftests/bpf: use canonical ftrace path

On Mon, Jan 30, 2023 at 06:34:19PM -0500, Steven Rostedt wrote:

On Mon, 30 Jan 2023 12:03:52 -0800
Alexei Starovoitov [off-list ref] wrote:

quoted

quoted

quoted

So this change will break the tests. We cannot do it.  

Could we add a way to try to mount it?

If anything, the tests should not have the path hard coded. It should then
look to see if it is mounted and use the path that is found. Otherwise it
should try mounting it at the correct location.

Feel free to take the code from libtracefs (and modify it):

https://git.kernel.org/pub/scm/libs/libtrace/libtracefs.git/tree/src/tracefs-utils.c#n89

It will make the test code much more robust.  

The point is not about tests. The point is that this change might break
some users that are working today with /sys/kernel/debug/tracing.

quoted

It also might be mounted differently.
For example from another system:
cat /proc/mounts|grep trace
tracefs /sys/kernel/tracing tracefs rw,nosuid,nodev,noexec,relatime 0 0
tracefs /sys/kernel/debug/tracing tracefs rw,relatime 0 0

Yes, and the code works when it's mounted multiple times.

quoted

So I suggest leaving the code as-is.

Why?  I want to make /sys/kernel/debug/tracing deprecated. It's a hack to
not break old code. I've had complaints about that hack, and there's even
systems that disable the auto mounting (that is, /sys/kernel/debug/tracing
would not exist in such configs) This was never expected to be a permanent
solution.

I don't think /sys/kernel/debug/tracing can ever be deprecated.
There are plenty of user space applications (not bpf related at all) that
expect it to be in that location.

Quick search shows:

android profiler:
https://android.googlesource.com/platform/external/perfetto/+/refs/heads/master/src/tools/dump_ftrace_stats/main.cc#60

java profiler:
https://github.com/jvm-profiling-tools/async-profiler/blob/master/src/perfEvents_linux.cpp#L85

If anything, leaving hardcoded calls like that forces the user to mount
debugfs when they may not want to. The entire point of tracefs was to allow
users to have access to the trace events without having to expose debugfs
and all the crud it brings with it. This was requested several times before
it was added.

All makes sense.

What is your technical reason for not modifying the code to look for
tracefs in /sys/kernel/tracing and if it's not there try
/sys/kernel/debug/tracing, and if both are not found, try mounting it.

libbpf already has code to probe both locations.
The point that full deprecation of /sys/kernel/debug/tracing is not possible,
hence no point doing the diff:
48 files changed, 96 insertions(+), 95 deletions(-)
It doesn't move the needle. Just a code churn.