Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Subject: Re: [PATCH bpf-next] libbpf: also check /sys/kernel/tracing for
 tracefs files
To:     Connor O'Brien <connoro@google.com>,
        Alexei Starovoitov <ast@kernel.org>,
        Andrii Nakryiko <andrii@kernel.org>
Cc:     Martin KaFai Lau <kafai@fb.com>, Song Liu <songliubraving@fb.com>,
        Yonghong Song <yhs@fb.com>,
        John Fastabend <john.fastabend@gmail.com>,
        KP Singh <kpsingh@kernel.org>, netdev@vger.kernel.org,
        bpf@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20220421201125.13907-1-connoro@google.com>
From:   Daniel Borkmann <daniel@iogearbox.net>
Message-ID: <407ab3dd-cee1-a043-585b-1b2886c6f7fd@iogearbox.net>
Date:   Fri, 22 Apr 2022 18:32:32 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.7.2
MIME-Version: 1.0
In-Reply-To: <20220421201125.13907-1-connoro@google.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 4/21/22 10:11 PM, Connor O'Brien wrote:
> libbpf looks for tracefs files only under debugfs, but tracefs may be
> mounted even if debugfs is not. When /sys/kernel/debug/tracing is
> absent, try looking under /sys/kernel/tracing instead.
> 
> Signed-off-by: Connor O'Brien <connoro@google.com>
> ---
>   tools/lib/bpf/libbpf.c | 26 +++++++++++++++++++-------
>   1 file changed, 19 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 68cc134d070d..6ef587329eb2 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -10140,10 +10140,16 @@ static void gen_kprobe_legacy_event_name(char *buf, size_t buf_sz,
>   		 __sync_fetch_and_add(&index, 1));
>   }
>   
> +static bool debugfs_available(void)
> +{
> +	return !access("/sys/kernel/debug/tracing", F_OK);

Should this be a one-time probe, so on subsequent debugfs_available() calls
we return the initially cached result?

> +}
> +
>   static int add_kprobe_event_legacy(const char *probe_name, bool retprobe,
>   				   const char *kfunc_name, size_t offset)
>   {
> -	const char *file = "/sys/kernel/debug/tracing/kprobe_events";
> +	const char *file = debugfs_available() ? "/sys/kernel/debug/tracing/kprobe_events" :
> +		"/sys/kernel/tracing/kprobe_events";
>   
>   	return append_to_file(file, "%c:%s/%s %s+0x%zx",
>   			      retprobe ? 'r' : 'p',
> @@ -10153,7 +10159,8 @@ static int add_kprobe_event_legacy(const char *probe_name, bool retprobe,
>   
>   static int remove_kprobe_event_legacy(const char *probe_name, bool retprobe)
>   {
> -	const char *file = "/sys/kernel/debug/tracing/kprobe_events";
> +	const char *file = debugfs_available() ? "/sys/kernel/debug/tracing/kprobe_events" :
> +		"/sys/kernel/tracing/kprobe_events";
>   
>   	return append_to_file(file, "-:%s/%s", retprobe ? "kretprobes" : "kprobes", probe_name);
>   }
> @@ -10163,7 +10170,8 @@ static int determine_kprobe_perf_type_legacy(const char *probe_name, bool retpro
>   	char file[256];
>   
>   	snprintf(file, sizeof(file),
> -		 "/sys/kernel/debug/tracing/events/%s/%s/id",
> +		 debugfs_available() ? "/sys/kernel/debug/tracing/events/%s/%s/id" :
> +		 "/sys/kernel/tracing/events/%s/%s/id",
>   		 retprobe ? "kretprobes" : "kprobes", probe_name);
>   
>   	return parse_uint_from_file(file, "%d\n");
> @@ -10493,7 +10501,8 @@ static void gen_uprobe_legacy_event_name(char *buf, size_t buf_sz,
>   static inline int add_uprobe_event_legacy(const char *probe_name, bool retprobe,
>   					  const char *binary_path, size_t offset)
>   {
> -	const char *file = "/sys/kernel/debug/tracing/uprobe_events";
> +	const char *file = debugfs_available() ? "/sys/kernel/debug/tracing/uprobe_events" :
> +		"/sys/kernel/tracing/uprobe_events";
>   
>   	return append_to_file(file, "%c:%s/%s %s:0x%zx",
>   			      retprobe ? 'r' : 'p',
> @@ -10503,7 +10512,8 @@ static inline int add_uprobe_event_legacy(const char *probe_name, bool retprobe,
>   
>   static inline int remove_uprobe_event_legacy(const char *probe_name, bool retprobe)
>   {
> -	const char *file = "/sys/kernel/debug/tracing/uprobe_events";
> +	const char *file = debugfs_available() ? "/sys/kernel/debug/tracing/uprobe_events" :
> +		"/sys/kernel/tracing/uprobe_events";
>   
>   	return append_to_file(file, "-:%s/%s", retprobe ? "uretprobes" : "uprobes", probe_name);
>   }
> @@ -10513,7 +10523,8 @@ static int determine_uprobe_perf_type_legacy(const char *probe_name, bool retpro
>   	char file[512];
>   
>   	snprintf(file, sizeof(file),
> -		 "/sys/kernel/debug/tracing/events/%s/%s/id",
> +		 debugfs_available() ? "/sys/kernel/debug/tracing/events/%s/%s/id" :
> +		 "/sys/kernel/tracing/events/%s/%s/id",
>   		 retprobe ? "uretprobes" : "uprobes", probe_name);
>   
>   	return parse_uint_from_file(file, "%d\n");
> @@ -11071,7 +11082,8 @@ static int determine_tracepoint_id(const char *tp_category,
>   	int ret;
>   
>   	ret = snprintf(file, sizeof(file),
> -		       "/sys/kernel/debug/tracing/events/%s/%s/id",
> +		       debugfs_available() ? "/sys/kernel/debug/tracing/events/%s/%s/id" :
> +		       "/sys/kernel/tracing/events/%s/%s/id",
>   		       tp_category, tp_name);
>   	if (ret < 0)
>   		return -errno;
>