Re: [PATCH bpf-next] libbpf: fallback to tracefs mount point if debugfs is not mounted

[Yonghong Song <yhs@xxxxxx>]

On 7/14/22 4:21 PM, Andrii Nakryiko wrote:
> Teach libbpf to fallback to tracefs mount point (/sys/kernel/tracing) if
> debugfs (/sys/kernel/debug/tracing) isn't mounted.
>
> Suggested-by: Connor O'Brien <connoro@xxxxxxxxxx>
> Signed-off-by: Andrii Nakryiko <andrii@xxxxxxxxxx>

Ack with a few suggestions below.

Acked-by: Yonghong Song <yhs@xxxxxx>

> ---
>   tools/lib/bpf/libbpf.c | 33 +++++++++++++++++++++++----------
>   1 file changed, 23 insertions(+), 10 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 68da1aca406c..4acdc174cc73 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -9828,6 +9828,19 @@ static int append_to_file(const char *file, const char *fmt, ...)
>   	return err;
>   }
>   
> +#define DEBUGFS "/sys/kernel/debug/tracing"
> +#define TRACEFS "/sys/kernel/tracing"
> +
> +static bool use_debugfs(void)
> +{
> +	static int has_debugfs = -1;
> +
> +	if (has_debugfs < 0)
> +		has_debugfs = access(DEBUGFS, F_OK) == 0;
> +
> +	return has_debugfs == 1;
> +}
> +
>   static void gen_kprobe_legacy_event_name(char *buf, size_t buf_sz,
>   					 const char *kfunc_name, size_t offset)
>   {
> @@ -9840,7 +9853,7 @@ static void gen_kprobe_legacy_event_name(char *buf, size_t buf_sz,
>   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 = use_debugfs() ? DEBUGFS"/kprobe_events" : TRACEFS"/kprobe_events";

I am wondering whether we can have a helper function to return
  use_debugfs() ? DEBUGFS"/kprobe_events" : TRACEFS"/kprobe_events"
so use_debugfs() won't appear in add_kprobe_event_legacy() function.

>   
>   	return append_to_file(file, "%c:%s/%s %s+0x%zx",
>   			      retprobe ? 'r' : 'p',
> @@ -9850,7 +9863,7 @@ 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 = use_debugfs() ? DEBUGFS"/kprobe_events" : TRACEFS"/kprobe_events";
>   
>   	return append_to_file(file, "-:%s/%s", retprobe ? "kretprobes" : "kprobes", probe_name);
>   }
> @@ -9859,8 +9872,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",
> +	snprintf(file, sizeof(file), "%s/events/%s/%s/id",
> +		 use_debugfs() ? DEBUGFS : TRACEFS,

The same here, a helper function can hide the details of use_debugfs().

>   		 retprobe ? "kretprobes" : "kprobes", probe_name);
>   
>   	return parse_uint_from_file(file, "%d\n");
> @@ -10213,7 +10226,7 @@ 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 = use_debugfs() ? DEBUGFS"/uprobe_events" : TRACEFS"/uprobe_events";
>   
>   	return append_to_file(file, "%c:%s/%s %s:0x%zx",
>   			      retprobe ? 'r' : 'p',
[...]