On Sat, Feb 25, 2023, at 11:16, David Marchand wrote: > Salut Gaëtan, > > On Fri, Feb 24, 2023 at 4:59 PM Gaëtan Rivet <gr...@u256.net> wrote: >> > FreeBSD libc pthread API has lock annotations while Linux glibc has >> > none. >> > We could simply disable the check on FreeBSD, but having this check, >> > a few issues got raised in drivers that are built with FreeBSD. >> > For now, I went with a simple #ifdef FreeBSD for pthread mutex related >> > annotations in our code. >> > >> >> Hi David, >> >> This is a great change, thanks for doing it. >> However I am not sure I understand the logic regarding the '#ifdef FREEBSD'. >> >> These annotations provide static hints to clang's thread safety analysis. >> What is the dependency on FreeBSD glibc? > > FreeBSD libc added clang annotations, while glibc did not. > > FreeBSD 13.1 libc: > int pthread_mutex_lock(pthread_mutex_t * __mutex) > __locks_exclusive(*__mutex); > > With: > > #if __has_extension(c_thread_safety_attributes) > #define __lock_annotate(x) __attribute__((x)) > #else > #define __lock_annotate(x) > #endif > > /* Function acquires an exclusive or shared lock. */ > #define __locks_exclusive(...) \ > __lock_annotate(exclusive_lock_function(__VA_ARGS__)) > > > glibc: > extern int pthread_mutex_lock (pthread_mutex_t *__mutex) > __THROWNL __nonnull ((1)); > > > > Since glibc does not declare that pthread_mutex_t is lockable, adding > an annotation triggers an error for Linux (taking eal_common_proc.c > patch 14 as an example): > > ../lib/eal/common/eal_common_proc.c:911:2: error: > 'exclusive_locks_required' attribute requires arguments whose type is > annotated with 'capability' attribute; type here is 'pthread_mutex_t' > [-Werror,-Wthread-safety-attributes] > __rte_exclusive_locks_required(pending_requests.lock) > ^ > ../lib/eal/include/rte_lock_annotations.h:23:17: note: expanded from > macro '__rte_exclusive_locks_required' > __attribute__((exclusive_locks_required(__VA_ARGS__))) > ^ > > We could wrap this annotation in a new construct (which would require > some build time check wrt pthread API state). > Not sure it is worth the effort though, for now. > > > -- > David Marchand
Ah ok, so if I understand correctly, DPDK would need to declare some '__rte_lockable rte_mutex' and associated functions for transparent support, to wrap above the pthread API. Unless it happens, would it be possible to condition the thread-safety annotations to FREEBSD as well? Maybe have two versions of the annotations: __rte_exclusive_locks_required() /* Conditioned on clang */ __pthread_exclusive_locks_required() /* Conditioned on glibc/pthread support */ the first one to use on RTE types that can be declared with __rte_lockable, the second for pthread objects that we don't want to replace. I know the namespace is not great so maybe named another way. The '#ifdef FREEBSD' ossifies the annotation support at the function level, when this is a matter of types. This impedance mismatch would mean large changes if someone was to make this support evolve at some point. -- Gaetan Rivet
