From: Ross Zwisler <[email protected]>
The canonical location for the tracefs filesystem is at /sys/kernel/tracing.
But, from Documentation/trace/ftrace.rst:
Before 4.1, all ftrace tracing control files were within the debugfs
file system, which is typically located at /sys/kernel/debug/tracing.
For backward compatibility, when mounting the debugfs file system,
the tracefs file system will be automatically mounted at:
/sys/kernel/debug/tracing
Many tests in the bpf selftest code still refer to this older debugfs
path, so let's update them to avoid confusion.
Signed-off-by: Ross Zwisler <[email protected]>
Acked-by: Michael S. Tsirkin <[email protected]>
---
tools/testing/selftests/bpf/get_cgroup_id_user.c | 8 ++++++--
.../selftests/bpf/prog_tests/kprobe_multi_test.c | 7 ++++++-
.../selftests/bpf/prog_tests/task_fd_query_tp.c | 8 ++++++--
.../selftests/bpf/prog_tests/tp_attach_query.c | 8 ++++++--
.../testing/selftests/bpf/prog_tests/trace_printk.c | 10 +++++++---
.../selftests/bpf/prog_tests/trace_vprintk.c | 10 +++++++---
.../selftests/bpf/progs/test_stacktrace_map.c | 2 +-
tools/testing/selftests/bpf/progs/test_tracepoint.c | 2 +-
tools/testing/selftests/bpf/test_ftrace.sh | 7 ++++++-
tools/testing/selftests/bpf/test_tunnel.sh | 13 +++++++++----
tools/testing/selftests/bpf/trace_helpers.c | 8 ++++++--
11 files changed, 61 insertions(+), 22 deletions(-)
diff --git a/tools/testing/selftests/bpf/get_cgroup_id_user.c b/tools/testing/selftests/bpf/get_cgroup_id_user.c
index 156743cf5870..4fa61ac8a0ee 100644
--- a/tools/testing/selftests/bpf/get_cgroup_id_user.c
+++ b/tools/testing/selftests/bpf/get_cgroup_id_user.c
@@ -86,8 +86,12 @@ int main(int argc, char **argv)
pid = getpid();
bpf_map_update_elem(pidmap_fd, &key, &pid, 0);
- snprintf(buf, sizeof(buf),
- "/sys/kernel/debug/tracing/events/%s/id", probe_name);
+ if (access("/sys/kernel/tracing/trace", F_OK) == 0)
+ snprintf(buf, sizeof(buf),
+ "/sys/kernel/tracing/events/%s/id", probe_name);
+ else
+ snprintf(buf, sizeof(buf),
+ "/sys/kernel/debug/tracing/events/%s/id", probe_name);
efd = open(buf, O_RDONLY, 0);
if (CHECK(efd < 0, "open", "err %d errno %d\n", efd, errno))
goto close_prog;
diff --git a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
index 113dba349a57..22be0a9a5a0a 100644
--- a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
+++ b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
@@ -338,7 +338,12 @@ static int get_syms(char ***symsp, size_t *cntp, bool kernel)
* Filtering out duplicates by using hashmap__add, which won't
* add existing entry.
*/
- f = fopen("/sys/kernel/debug/tracing/available_filter_functions", "r");
+
+ if (access("/sys/kernel/tracing/trace", F_OK) == 0)
+ f = fopen("/sys/kernel/tracing/available_filter_functions", "r");
+ else
+ f = fopen("/sys/kernel/debug/tracing/available_filter_functions", "r");
+
if (!f)
return -EINVAL;
diff --git a/tools/testing/selftests/bpf/prog_tests/task_fd_query_tp.c b/tools/testing/selftests/bpf/prog_tests/task_fd_query_tp.c
index c717741bf8b6..60f92fd3c37a 100644
--- a/tools/testing/selftests/bpf/prog_tests/task_fd_query_tp.c
+++ b/tools/testing/selftests/bpf/prog_tests/task_fd_query_tp.c
@@ -17,8 +17,12 @@ static void test_task_fd_query_tp_core(const char *probe_name,
if (CHECK(err, "bpf_prog_test_load", "err %d errno %d\n", err, errno))
goto close_prog;
- snprintf(buf, sizeof(buf),
- "/sys/kernel/debug/tracing/events/%s/id", probe_name);
+ if (access("/sys/kernel/tracing/trace", F_OK) == 0)
+ snprintf(buf, sizeof(buf),
+ "/sys/kernel/tracing/events/%s/id", probe_name);
+ else
+ snprintf(buf, sizeof(buf),
+ "/sys/kernel/debug/tracing/events/%s/id", probe_name);
efd = open(buf, O_RDONLY, 0);
if (CHECK(efd < 0, "open", "err %d errno %d\n", efd, errno))
goto close_prog;
diff --git a/tools/testing/selftests/bpf/prog_tests/tp_attach_query.c b/tools/testing/selftests/bpf/prog_tests/tp_attach_query.c
index 770fcc3bb1ba..d3e377fa8e9b 100644
--- a/tools/testing/selftests/bpf/prog_tests/tp_attach_query.c
+++ b/tools/testing/selftests/bpf/prog_tests/tp_attach_query.c
@@ -16,8 +16,12 @@ void serial_test_tp_attach_query(void)
for (i = 0; i < num_progs; i++)
obj[i] = NULL;
- snprintf(buf, sizeof(buf),
- "/sys/kernel/debug/tracing/events/sched/sched_switch/id");
+ if (access("/sys/kernel/tracing/trace", F_OK) == 0)
+ snprintf(buf, sizeof(buf),
+ "/sys/kernel/tracing/events/sched/sched_switch/id");
+ else
+ snprintf(buf, sizeof(buf),
+ "/sys/kernel/debug/tracing/events/sched/sched_switch/id");
efd = open(buf, O_RDONLY, 0);
if (CHECK(efd < 0, "open", "err %d errno %d\n", efd, errno))
return;
diff --git a/tools/testing/selftests/bpf/prog_tests/trace_printk.c b/tools/testing/selftests/bpf/prog_tests/trace_printk.c
index cade7f12315f..7b9124d506a5 100644
--- a/tools/testing/selftests/bpf/prog_tests/trace_printk.c
+++ b/tools/testing/selftests/bpf/prog_tests/trace_printk.c
@@ -5,7 +5,8 @@
#include "trace_printk.lskel.h"
-#define TRACEBUF "/sys/kernel/debug/tracing/trace_pipe"
+#define TRACEFS_PIPE "/sys/kernel/tracing/trace_pipe"
+#define DEBUGFS_PIPE "/sys/kernel/debug/tracing/trace_pipe"
#define SEARCHMSG "testing,testing"
void serial_test_trace_printk(void)
@@ -34,8 +35,11 @@ void serial_test_trace_printk(void)
if (!ASSERT_OK(err, "trace_printk__attach"))
goto cleanup;
- fp = fopen(TRACEBUF, "r");
- if (!ASSERT_OK_PTR(fp, "fopen(TRACEBUF)"))
+ if (access(TRACEFS_PIPE, F_OK) == 0)
+ fp = fopen(TRACEFS_PIPE, "r");
+ else
+ fp = fopen(DEBUGFS_PIPE, "r");
+ if (!ASSERT_OK_PTR(fp, "fopen(TRACE_PIPE)"))
goto cleanup;
/* We do not want to wait forever if this test fails... */
diff --git a/tools/testing/selftests/bpf/prog_tests/trace_vprintk.c b/tools/testing/selftests/bpf/prog_tests/trace_vprintk.c
index 7a4e313e8558..44ea2fd88f4c 100644
--- a/tools/testing/selftests/bpf/prog_tests/trace_vprintk.c
+++ b/tools/testing/selftests/bpf/prog_tests/trace_vprintk.c
@@ -5,7 +5,8 @@
#include "trace_vprintk.lskel.h"
-#define TRACEBUF "/sys/kernel/debug/tracing/trace_pipe"
+#define TRACEFS_PIPE "/sys/kernel/tracing/trace_pipe"
+#define DEBUGFS_PIPE "/sys/kernel/debug/tracing/trace_pipe"
#define SEARCHMSG "1,2,3,4,5,6,7,8,9,10"
void serial_test_trace_vprintk(void)
@@ -27,8 +28,11 @@ void serial_test_trace_vprintk(void)
if (!ASSERT_OK(err, "trace_vprintk__attach"))
goto cleanup;
- fp = fopen(TRACEBUF, "r");
- if (!ASSERT_OK_PTR(fp, "fopen(TRACEBUF)"))
+ if (access(TRACEFS_PIPE, F_OK) == 0)
+ fp = fopen(TRACEFS_PIPE, "r");
+ else
+ fp = fopen(DEBUGFS_PIPE, "r");
+ if (!ASSERT_OK_PTR(fp, "fopen(TRACE_PIPE)"))
goto cleanup;
/* We do not want to wait forever if this test fails... */
diff --git a/tools/testing/selftests/bpf/progs/test_stacktrace_map.c b/tools/testing/selftests/bpf/progs/test_stacktrace_map.c
index 728dbd39eff0..47568007b668 100644
--- a/tools/testing/selftests/bpf/progs/test_stacktrace_map.c
+++ b/tools/testing/selftests/bpf/progs/test_stacktrace_map.c
@@ -38,7 +38,7 @@ struct {
__type(value, stack_trace_t);
} stack_amap SEC(".maps");
-/* taken from /sys/kernel/debug/tracing/events/sched/sched_switch/format */
+/* taken from /sys/kernel/tracing/events/sched/sched_switch/format */
struct sched_switch_args {
unsigned long long pad;
char prev_comm[TASK_COMM_LEN];
diff --git a/tools/testing/selftests/bpf/progs/test_tracepoint.c b/tools/testing/selftests/bpf/progs/test_tracepoint.c
index 43bd7a20cc50..4cb8bbb6a320 100644
--- a/tools/testing/selftests/bpf/progs/test_tracepoint.c
+++ b/tools/testing/selftests/bpf/progs/test_tracepoint.c
@@ -4,7 +4,7 @@
#include <vmlinux.h>
#include <bpf/bpf_helpers.h>
-/* taken from /sys/kernel/debug/tracing/events/sched/sched_switch/format */
+/* taken from /sys/kernel/tracing/events/sched/sched_switch/format */
struct sched_switch_args {
unsigned long long pad;
char prev_comm[TASK_COMM_LEN];
diff --git a/tools/testing/selftests/bpf/test_ftrace.sh b/tools/testing/selftests/bpf/test_ftrace.sh
index 20de7bb873bc..f5109eb0e951 100755
--- a/tools/testing/selftests/bpf/test_ftrace.sh
+++ b/tools/testing/selftests/bpf/test_ftrace.sh
@@ -1,6 +1,11 @@
#!/bin/bash
-TR=/sys/kernel/debug/tracing/
+if [[ -e /sys/kernel/tracing/trace ]]; then
+ TR=/sys/kernel/tracing/
+else
+ TR=/sys/kernel/debug/tracing/
+fi
+
clear_trace() { # reset trace output
echo > $TR/trace
}
diff --git a/tools/testing/selftests/bpf/test_tunnel.sh b/tools/testing/selftests/bpf/test_tunnel.sh
index 06857b689c11..2dec7dbf29a2 100755
--- a/tools/testing/selftests/bpf/test_tunnel.sh
+++ b/tools/testing/selftests/bpf/test_tunnel.sh
@@ -571,8 +571,13 @@ setup_xfrm_tunnel()
test_xfrm_tunnel()
{
+ if [[ -e /sys/kernel/tracing/trace ]]; then
+ TRACE=/sys/kernel/tracing/trace
+ else
+ TRACE=/sys/kernel/debug/tracing/trace
+ fi
config_device
- > /sys/kernel/debug/tracing/trace
+ > ${TRACE}
setup_xfrm_tunnel
mkdir -p ${BPF_PIN_TUNNEL_DIR}
bpftool prog loadall ${BPF_FILE} ${BPF_PIN_TUNNEL_DIR}
@@ -581,11 +586,11 @@ test_xfrm_tunnel()
${BPF_PIN_TUNNEL_DIR}/xfrm_get_state
ip netns exec at_ns0 ping $PING_ARG 10.1.1.200
sleep 1
- grep "reqid 1" /sys/kernel/debug/tracing/trace
+ grep "reqid 1" ${TRACE}
check_err $?
- grep "spi 0x1" /sys/kernel/debug/tracing/trace
+ grep "spi 0x1" ${TRACE}
check_err $?
- grep "remote ip 0xac100164" /sys/kernel/debug/tracing/trace
+ grep "remote ip 0xac100164" ${TRACE}
check_err $?
cleanup
diff --git a/tools/testing/selftests/bpf/trace_helpers.c b/tools/testing/selftests/bpf/trace_helpers.c
index 09a16a77bae4..934bf28fc888 100644
--- a/tools/testing/selftests/bpf/trace_helpers.c
+++ b/tools/testing/selftests/bpf/trace_helpers.c
@@ -12,7 +12,8 @@
#include <sys/mman.h>
#include "trace_helpers.h"
-#define DEBUGFS "/sys/kernel/debug/tracing/"
+#define TRACEFS_PIPE "/sys/kernel/tracing/trace_pipe"
+#define DEBUGFS_PIPE "/sys/kernel/debug/tracing/trace_pipe"
#define MAX_SYMS 300000
static struct ksym syms[MAX_SYMS];
@@ -136,7 +137,10 @@ void read_trace_pipe(void)
{
int trace_fd;
- trace_fd = open(DEBUGFS "trace_pipe", O_RDONLY, 0);
+ if (access(TRACEFS_PIPE, F_OK) == 0)
+ trace_fd = open(TRACEFS_PIPE, O_RDONLY, 0);
+ else
+ trace_fd = open(DEBUGFS_PIPE, O_RDONLY, 0);
if (trace_fd < 0)
return;
--
2.40.0.rc1.284.g88254d51c5-goog
On Fri, 10 Mar 2023 10:52:09 -0700
[email protected] wrote:
> diff --git a/tools/testing/selftests/bpf/get_cgroup_id_user.c b/tools/testing/selftests/bpf/get_cgroup_id_user.c
> index 156743cf5870..4fa61ac8a0ee 100644
> --- a/tools/testing/selftests/bpf/get_cgroup_id_user.c
> +++ b/tools/testing/selftests/bpf/get_cgroup_id_user.c
> @@ -86,8 +86,12 @@ int main(int argc, char **argv)
> pid = getpid();
> bpf_map_update_elem(pidmap_fd, &key, &pid, 0);
>
> - snprintf(buf, sizeof(buf),
> - "/sys/kernel/debug/tracing/events/%s/id", probe_name);
> + if (access("/sys/kernel/tracing/trace", F_OK) == 0)
> + snprintf(buf, sizeof(buf),
> + "/sys/kernel/tracing/events/%s/id", probe_name);
> + else
> + snprintf(buf, sizeof(buf),
> + "/sys/kernel/debug/tracing/events/%s/id", probe_name);
I don't know how the BPF folks feel, but I do know some kernel developers
prefer that if you need to break a single command into multiple lines that
you then need to add brackets around it. As it makes it easier to read.
if (access("/sys/kernel/tracing/trace", F_OK) == 0) {
snprintf(buf, sizeof(buf),
"/sys/kernel/tracing/events/%s/id", probe_name);
} else {
snprintf(buf, sizeof(buf),
"/sys/kernel/debug/tracing/events/%s/id", probe_name);
}
> efd = open(buf, O_RDONLY, 0);
> if (CHECK(efd < 0, "open", "err %d errno %d\n", efd, errno))
> goto close_prog;
> diff --git a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
> index 113dba349a57..22be0a9a5a0a 100644
> --- a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
> +++ b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
> @@ -338,7 +338,12 @@ static int get_syms(char ***symsp, size_t *cntp, bool kernel)
> * Filtering out duplicates by using hashmap__add, which won't
> * add existing entry.
> */
> - f = fopen("/sys/kernel/debug/tracing/available_filter_functions", "r");
> +
> + if (access("/sys/kernel/tracing/trace", F_OK) == 0)
> + f = fopen("/sys/kernel/tracing/available_filter_functions", "r");
> + else
> + f = fopen("/sys/kernel/debug/tracing/available_filter_functions", "r");
> +
> if (!f)
> return -EINVAL;
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/task_fd_query_tp.c b/tools/testing/selftests/bpf/prog_tests/task_fd_query_tp.c
> index c717741bf8b6..60f92fd3c37a 100644
> --- a/tools/testing/selftests/bpf/prog_tests/task_fd_query_tp.c
> +++ b/tools/testing/selftests/bpf/prog_tests/task_fd_query_tp.c
> @@ -17,8 +17,12 @@ static void test_task_fd_query_tp_core(const char *probe_name,
> if (CHECK(err, "bpf_prog_test_load", "err %d errno %d\n", err, errno))
> goto close_prog;
>
> - snprintf(buf, sizeof(buf),
> - "/sys/kernel/debug/tracing/events/%s/id", probe_name);
> + if (access("/sys/kernel/tracing/trace", F_OK) == 0)
> + snprintf(buf, sizeof(buf),
> + "/sys/kernel/tracing/events/%s/id", probe_name);
> + else
> + snprintf(buf, sizeof(buf),
> + "/sys/kernel/debug/tracing/events/%s/id", probe_name);
Same here.
> efd = open(buf, O_RDONLY, 0);
> if (CHECK(efd < 0, "open", "err %d errno %d\n", efd, errno))
> goto close_prog;
> diff --git a/tools/testing/selftests/bpf/prog_tests/tp_attach_query.c b/tools/testing/selftests/bpf/prog_tests/tp_attach_query.c
> index 770fcc3bb1ba..d3e377fa8e9b 100644
> --- a/tools/testing/selftests/bpf/prog_tests/tp_attach_query.c
> +++ b/tools/testing/selftests/bpf/prog_tests/tp_attach_query.c
> @@ -16,8 +16,12 @@ void serial_test_tp_attach_query(void)
> for (i = 0; i < num_progs; i++)
> obj[i] = NULL;
>
> - snprintf(buf, sizeof(buf),
> - "/sys/kernel/debug/tracing/events/sched/sched_switch/id");
> + if (access("/sys/kernel/tracing/trace", F_OK) == 0)
> + snprintf(buf, sizeof(buf),
> + "/sys/kernel/tracing/events/sched/sched_switch/id");
> + else
> + snprintf(buf, sizeof(buf),
> + "/sys/kernel/debug/tracing/events/sched/sched_switch/id");
and here.
But perhaps the BPF folks don't care?
-- Steve
> efd = open(buf, O_RDONLY, 0);
> if (CHECK(efd < 0, "open", "err %d errno %d\n", efd, errno))
> return;
On Fri, Mar 10, 2023 at 06:33:52PM -0500, Steven Rostedt wrote:
> On Fri, 10 Mar 2023 10:52:09 -0700
> [email protected] wrote:
>
> > diff --git a/tools/testing/selftests/bpf/get_cgroup_id_user.c b/tools/testing/selftests/bpf/get_cgroup_id_user.c
> > index 156743cf5870..4fa61ac8a0ee 100644
> > --- a/tools/testing/selftests/bpf/get_cgroup_id_user.c
> > +++ b/tools/testing/selftests/bpf/get_cgroup_id_user.c
> > @@ -86,8 +86,12 @@ int main(int argc, char **argv)
> > pid = getpid();
> > bpf_map_update_elem(pidmap_fd, &key, &pid, 0);
> >
> > - snprintf(buf, sizeof(buf),
> > - "/sys/kernel/debug/tracing/events/%s/id", probe_name);
> > + if (access("/sys/kernel/tracing/trace", F_OK) == 0)
> > + snprintf(buf, sizeof(buf),
> > + "/sys/kernel/tracing/events/%s/id", probe_name);
> > + else
> > + snprintf(buf, sizeof(buf),
> > + "/sys/kernel/debug/tracing/events/%s/id", probe_name);
>
> I don't know how the BPF folks feel, but I do know some kernel developers
> prefer that if you need to break a single command into multiple lines that
> you then need to add brackets around it. As it makes it easier to read.
>
> if (access("/sys/kernel/tracing/trace", F_OK) == 0) {
> snprintf(buf, sizeof(buf),
> "/sys/kernel/tracing/events/%s/id", probe_name);
> } else {
> snprintf(buf, sizeof(buf),
> "/sys/kernel/debug/tracing/events/%s/id", probe_name);
> }
>
>
>
> > efd = open(buf, O_RDONLY, 0);
> > if (CHECK(efd < 0, "open", "err %d errno %d\n", efd, errno))
> > goto close_prog;
> > diff --git a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
> > index 113dba349a57..22be0a9a5a0a 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
> > @@ -338,7 +338,12 @@ static int get_syms(char ***symsp, size_t *cntp, bool kernel)
> > * Filtering out duplicates by using hashmap__add, which won't
> > * add existing entry.
> > */
> > - f = fopen("/sys/kernel/debug/tracing/available_filter_functions", "r");
> > +
> > + if (access("/sys/kernel/tracing/trace", F_OK) == 0)
> > + f = fopen("/sys/kernel/tracing/available_filter_functions", "r");
> > + else
> > + f = fopen("/sys/kernel/debug/tracing/available_filter_functions", "r");
> > +
> > if (!f)
> > return -EINVAL;
> >
> > diff --git a/tools/testing/selftests/bpf/prog_tests/task_fd_query_tp.c b/tools/testing/selftests/bpf/prog_tests/task_fd_query_tp.c
> > index c717741bf8b6..60f92fd3c37a 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/task_fd_query_tp.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/task_fd_query_tp.c
> > @@ -17,8 +17,12 @@ static void test_task_fd_query_tp_core(const char *probe_name,
> > if (CHECK(err, "bpf_prog_test_load", "err %d errno %d\n", err, errno))
> > goto close_prog;
> >
> > - snprintf(buf, sizeof(buf),
> > - "/sys/kernel/debug/tracing/events/%s/id", probe_name);
> > + if (access("/sys/kernel/tracing/trace", F_OK) == 0)
> > + snprintf(buf, sizeof(buf),
> > + "/sys/kernel/tracing/events/%s/id", probe_name);
> > + else
> > + snprintf(buf, sizeof(buf),
> > + "/sys/kernel/debug/tracing/events/%s/id", probe_name);
>
> Same here.
>
> > efd = open(buf, O_RDONLY, 0);
> > if (CHECK(efd < 0, "open", "err %d errno %d\n", efd, errno))
> > goto close_prog;
> > diff --git a/tools/testing/selftests/bpf/prog_tests/tp_attach_query.c b/tools/testing/selftests/bpf/prog_tests/tp_attach_query.c
> > index 770fcc3bb1ba..d3e377fa8e9b 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/tp_attach_query.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/tp_attach_query.c
> > @@ -16,8 +16,12 @@ void serial_test_tp_attach_query(void)
> > for (i = 0; i < num_progs; i++)
> > obj[i] = NULL;
> >
> > - snprintf(buf, sizeof(buf),
> > - "/sys/kernel/debug/tracing/events/sched/sched_switch/id");
> > + if (access("/sys/kernel/tracing/trace", F_OK) == 0)
> > + snprintf(buf, sizeof(buf),
> > + "/sys/kernel/tracing/events/sched/sched_switch/id");
> > + else
> > + snprintf(buf, sizeof(buf),
> > + "/sys/kernel/debug/tracing/events/sched/sched_switch/id");
>
> and here.
>
> But perhaps the BPF folks don't care?
Sure, I agree that this is more readable. I'll gather your Reviewed-by for
patch #1, make this change, rebase to the current bpf/bpf-next and send out
v4.
Ross Zwisler wrote:
> On Fri, Mar 10, 2023 at 06:33:52PM -0500, Steven Rostedt wrote:
> > On Fri, 10 Mar 2023 10:52:09 -0700
> > [email protected] wrote:
> >
> > > diff --git a/tools/testing/selftests/bpf/get_cgroup_id_user.c b/tools/testing/selftests/bpf/get_cgroup_id_user.c
> > > index 156743cf5870..4fa61ac8a0ee 100644
> > > --- a/tools/testing/selftests/bpf/get_cgroup_id_user.c
> > > +++ b/tools/testing/selftests/bpf/get_cgroup_id_user.c
> > > @@ -86,8 +86,12 @@ int main(int argc, char **argv)
> > > pid = getpid();
> > > bpf_map_update_elem(pidmap_fd, &key, &pid, 0);
> > >
> > > - snprintf(buf, sizeof(buf),
> > > - "/sys/kernel/debug/tracing/events/%s/id", probe_name);
> > > + if (access("/sys/kernel/tracing/trace", F_OK) == 0)
> > > + snprintf(buf, sizeof(buf),
> > > + "/sys/kernel/tracing/events/%s/id", probe_name);
> > > + else
> > > + snprintf(buf, sizeof(buf),
> > > + "/sys/kernel/debug/tracing/events/%s/id", probe_name);
> >
> > I don't know how the BPF folks feel, but I do know some kernel developers
> > prefer that if you need to break a single command into multiple lines that
> > you then need to add brackets around it. As it makes it easier to read.
> >
> > if (access("/sys/kernel/tracing/trace", F_OK) == 0) {
> > snprintf(buf, sizeof(buf),
> > "/sys/kernel/tracing/events/%s/id", probe_name);
> > } else {
> > snprintf(buf, sizeof(buf),
> > "/sys/kernel/debug/tracing/events/%s/id", probe_name);
> > }
> >
> >
> >
> > > efd = open(buf, O_RDONLY, 0);
> > > if (CHECK(efd < 0, "open", "err %d errno %d\n", efd, errno))
> > > goto close_prog;
> > > diff --git a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
> > > index 113dba349a57..22be0a9a5a0a 100644
> > > --- a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
> > > +++ b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
> > > @@ -338,7 +338,12 @@ static int get_syms(char ***symsp, size_t *cntp, bool kernel)
> > > * Filtering out duplicates by using hashmap__add, which won't
> > > * add existing entry.
> > > */
> > > - f = fopen("/sys/kernel/debug/tracing/available_filter_functions", "r");
> > > +
> > > + if (access("/sys/kernel/tracing/trace", F_OK) == 0)
> > > + f = fopen("/sys/kernel/tracing/available_filter_functions", "r");
> > > + else
> > > + f = fopen("/sys/kernel/debug/tracing/available_filter_functions", "r");
> > > +
> > > if (!f)
> > > return -EINVAL;
> > >
> > > diff --git a/tools/testing/selftests/bpf/prog_tests/task_fd_query_tp.c b/tools/testing/selftests/bpf/prog_tests/task_fd_query_tp.c
> > > index c717741bf8b6..60f92fd3c37a 100644
> > > --- a/tools/testing/selftests/bpf/prog_tests/task_fd_query_tp.c
> > > +++ b/tools/testing/selftests/bpf/prog_tests/task_fd_query_tp.c
> > > @@ -17,8 +17,12 @@ static void test_task_fd_query_tp_core(const char *probe_name,
> > > if (CHECK(err, "bpf_prog_test_load", "err %d errno %d\n", err, errno))
> > > goto close_prog;
> > >
> > > - snprintf(buf, sizeof(buf),
> > > - "/sys/kernel/debug/tracing/events/%s/id", probe_name);
> > > + if (access("/sys/kernel/tracing/trace", F_OK) == 0)
> > > + snprintf(buf, sizeof(buf),
> > > + "/sys/kernel/tracing/events/%s/id", probe_name);
> > > + else
> > > + snprintf(buf, sizeof(buf),
> > > + "/sys/kernel/debug/tracing/events/%s/id", probe_name);
> >
> > Same here.
> >
> > > efd = open(buf, O_RDONLY, 0);
> > > if (CHECK(efd < 0, "open", "err %d errno %d\n", efd, errno))
> > > goto close_prog;
> > > diff --git a/tools/testing/selftests/bpf/prog_tests/tp_attach_query.c b/tools/testing/selftests/bpf/prog_tests/tp_attach_query.c
> > > index 770fcc3bb1ba..d3e377fa8e9b 100644
> > > --- a/tools/testing/selftests/bpf/prog_tests/tp_attach_query.c
> > > +++ b/tools/testing/selftests/bpf/prog_tests/tp_attach_query.c
> > > @@ -16,8 +16,12 @@ void serial_test_tp_attach_query(void)
> > > for (i = 0; i < num_progs; i++)
> > > obj[i] = NULL;
> > >
> > > - snprintf(buf, sizeof(buf),
> > > - "/sys/kernel/debug/tracing/events/sched/sched_switch/id");
> > > + if (access("/sys/kernel/tracing/trace", F_OK) == 0)
> > > + snprintf(buf, sizeof(buf),
> > > + "/sys/kernel/tracing/events/sched/sched_switch/id");
> > > + else
> > > + snprintf(buf, sizeof(buf),
> > > + "/sys/kernel/debug/tracing/events/sched/sched_switch/id");
> >
> > and here.
> >
> > But perhaps the BPF folks don't care?
>
> Sure, I agree that this is more readable. I'll gather your Reviewed-by for
> patch #1, make this change, rebase to the current bpf/bpf-next and send out
> v4.
Also for the patch. LGTM
Acked-by: John Fastabend <[email protected]>