Re: [PATCH bpf-next] libbpf: synchronize access to print function pointer

[Stanislav Fomichev <sdf@google.com>]

On 03/24, JP Kobryn wrote:
> This patch prevents races on the print function pointer, allowing the
> libbpf_set_print() function to become thread safe.

Why does it have to be thread-safe? The rest of the APIs aren't, so
why can't use solve it on your side by wrapping those calls with a
mutex?

(is there some context I'm missing?)

> Signed-off-by: JP Kobryn <inwardvessel@gmail.com>
> ---
>   tools/lib/bpf/libbpf.c | 9 ++++++---
>   tools/lib/bpf/libbpf.h | 3 +++
>   2 files changed, 9 insertions(+), 3 deletions(-)

> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index f6a071db5c6e..15737d7b5a28 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -216,9 +216,10 @@ static libbpf_print_fn_t __libbpf_pr = __base_pr;

>   libbpf_print_fn_t libbpf_set_print(libbpf_print_fn_t fn)
>   {
> -	libbpf_print_fn_t old_print_fn = __libbpf_pr;
> +	libbpf_print_fn_t old_print_fn;
> +
> +	old_print_fn = __atomic_exchange_n(&__libbpf_pr, fn, __ATOMIC_RELAXED);

> -	__libbpf_pr = fn;
>   	return old_print_fn;
>   }

> @@ -227,8 +228,10 @@ void libbpf_print(enum libbpf_print_level level,  
> const char *format, ...)
>   {
>   	va_list args;
>   	int old_errno;
> +	libbpf_print_fn_t print_fn;

> -	if (!__libbpf_pr)
> +	print_fn = __atomic_load_n(&__libbpf_pr, __ATOMIC_RELAXED);
> +	if (!print_fn)
>   		return;

>   	old_errno = errno;
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index 1615e55e2e79..4478809ff9ca 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -99,6 +99,9 @@ typedef int (*libbpf_print_fn_t)(enum  
> libbpf_print_level level,
>   /**
>    * @brief **libbpf_set_print()** sets user-provided log callback  
> function to
>    * be used for libbpf warnings and informational messages.
> + *
> + * This function is thread safe.
> + *
>    * @param fn The log print function. If NULL, libbpf won't print  
> anything.
>    * @return Pointer to old print function.
>    */
> --
> 2.39.2