From: "Gaëtan Rivet" <grive@u256.net>
To: "David Marchand" <david.marchand@redhat.com>
Cc: dev@dpdk.org, "Thomas Monjalon" <thomas@monjalon.net>
Subject: Re: [PATCH v2 00/20] Enable lock annotations on most libraries and drivers
Date: Mon, 27 Feb 2023 17:12:49 +0100 [thread overview]
Message-ID: <e17bce10-886c-4d35-8ed2-2ed9ae652b30@app.fastmail.com> (raw)
In-Reply-To: <CAJFAV8w_aPsQr4eg1RNph+1umqa9SKXyvAGU-NP2gLF6=fM1yA@mail.gmail.com>
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 <grive@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
next prev parent reply other threads:[~2023-02-27 16:13 UTC|newest]
Thread overview: 76+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-24 8:16 [PATCH 00/14] " David Marchand
2023-02-24 8:16 ` [PATCH 01/14] malloc: rework heap lock handling David Marchand
2023-02-24 8:16 ` [PATCH 02/14] mem: rework malloc heap init David Marchand
2023-02-24 8:16 ` [PATCH 03/14] mem: annotate shared memory config locks David Marchand
2023-02-24 8:16 ` [PATCH 04/14] hash: annotate cuckoo hash lock David Marchand
2023-02-24 8:16 ` [PATCH 05/14] graph: annotate graph lock David Marchand
2023-02-24 8:16 ` [PATCH 06/14] drivers: inherit lock annotations for Intel drivers David Marchand
2023-02-24 8:16 ` [PATCH 07/14] net/cxgbe: inherit lock annotations David Marchand
2023-02-24 8:16 ` [PATCH 08/14] net/fm10k: annotate mailbox lock David Marchand
2023-02-24 8:16 ` [PATCH 09/14] net/sfc: rework locking in proxy code David Marchand
2023-02-24 8:16 ` [PATCH 10/14] net/sfc: inherit lock annotations David Marchand
2023-02-24 8:16 ` [PATCH 11/14] net/virtio: annotate lock for guest announce David Marchand
2023-02-24 8:16 ` [PATCH 12/14] raw/ifpga: inherit lock annotations David Marchand
2023-02-24 8:16 ` [PATCH 13/14] vdpa/sfc: " David Marchand
2023-02-24 8:16 ` [PATCH 14/14] enable lock check David Marchand
2023-02-24 15:11 ` [PATCH v2 00/20] Enable lock annotations on most libraries and drivers David Marchand
2023-02-24 15:11 ` [PATCH v2 01/20] malloc: rework heap lock handling David Marchand
2023-02-24 15:11 ` [PATCH v2 02/20] mem: rework malloc heap init David Marchand
2023-02-24 15:11 ` [PATCH v2 03/20] mem: annotate shared memory config locks David Marchand
2023-02-24 15:11 ` [PATCH v2 04/20] hash: annotate cuckoo hash lock David Marchand
2023-02-24 15:11 ` [PATCH v2 05/20] graph: annotate graph lock David Marchand
2023-02-24 15:11 ` [PATCH v2 06/20] drivers: inherit lock annotations for Intel drivers David Marchand
2023-02-24 15:11 ` [PATCH v2 07/20] net/cxgbe: inherit lock annotations David Marchand
2023-02-24 15:11 ` [PATCH v2 08/20] net/fm10k: annotate mailbox lock David Marchand
2023-02-24 15:11 ` [PATCH v2 09/20] net/sfc: rework locking in proxy code David Marchand
2023-02-24 15:11 ` [PATCH v2 10/20] net/sfc: inherit lock annotations David Marchand
2023-02-24 15:11 ` [PATCH v2 11/20] net/virtio: annotate lock for guest announce David Marchand
2023-02-27 2:05 ` Xia, Chenbo
2023-02-27 8:24 ` David Marchand
2023-02-27 16:28 ` Maxime Coquelin
2023-02-28 2:45 ` Xia, Chenbo
2023-03-02 9:26 ` David Marchand
2023-03-02 9:28 ` Maxime Coquelin
2023-03-02 12:35 ` David Marchand
2023-02-24 15:11 ` [PATCH v2 12/20] raw/ifpga: inherit lock annotations David Marchand
2023-02-27 6:29 ` Xu, Rosen
2023-02-27 7:15 ` Huang, Wei
2023-02-24 15:11 ` [PATCH v2 13/20] vdpa/sfc: " David Marchand
2023-02-24 15:11 ` [PATCH v2 14/20] ipc: annotate pthread mutex David Marchand
2023-02-24 15:11 ` [PATCH v2 15/20] ethdev: " David Marchand
2023-02-24 15:11 ` [PATCH v2 16/20] net/failsafe: fix mutex locking David Marchand
2023-02-24 15:35 ` Gaëtan Rivet
2023-02-24 15:11 ` [PATCH v2 17/20] net/failsafe: annotate pthread mutex David Marchand
2023-02-24 15:11 ` [PATCH v2 18/20] net/hinic: " David Marchand
2023-02-24 15:11 ` [PATCH v2 19/20] eal/windows: disable lock check on alarm code David Marchand
2023-02-24 15:11 ` [PATCH v2 20/20] enable lock check David Marchand
2023-02-27 2:32 ` Xia, Chenbo
2023-02-24 15:58 ` [PATCH v2 00/20] Enable lock annotations on most libraries and drivers Gaëtan Rivet
2023-02-25 10:16 ` David Marchand
2023-02-27 16:12 ` Gaëtan Rivet [this message]
2023-03-02 8:52 ` David Marchand
2023-04-03 10:52 ` David Marchand
2023-04-03 15:03 ` Tyler Retzlaff
2023-04-03 15:36 ` Tyler Retzlaff
2023-04-04 7:45 ` David Marchand
2023-04-04 12:48 ` [PATCH v3 00/16] " David Marchand
2023-04-04 12:48 ` [PATCH v3 01/16] malloc: rework heap destroy David Marchand
2023-04-04 12:48 ` [PATCH v3 02/16] mem: rework malloc heap init David Marchand
2023-04-04 12:48 ` [PATCH v3 03/16] mem: annotate shared memory config locks David Marchand
2023-04-04 12:48 ` [PATCH v3 04/16] hash: annotate cuckoo hash lock David Marchand
2023-04-04 12:48 ` [PATCH v3 05/16] graph: annotate graph lock David Marchand
2023-04-04 12:48 ` [PATCH v3 06/16] drivers: inherit lock annotations for Intel drivers David Marchand
2023-04-04 12:48 ` [PATCH v3 07/16] net/cxgbe: inherit lock annotations David Marchand
2023-04-04 12:48 ` [PATCH v3 08/16] net/fm10k: annotate mailbox lock David Marchand
2023-04-04 12:48 ` [PATCH v3 09/16] net/sfc: rework locking in proxy code David Marchand
2023-04-04 12:48 ` [PATCH v3 10/16] net/sfc: inherit lock annotations David Marchand
2023-04-04 12:48 ` [PATCH v3 11/16] net/virtio: rework guest announce notify helper David Marchand
2023-04-04 12:48 ` [PATCH v3 12/16] raw/ifpga: inherit lock annotations David Marchand
2023-04-04 12:48 ` [PATCH v3 13/16] vdpa/sfc: " David Marchand
2023-04-04 12:48 ` [PATCH v3 14/16] net/failsafe: fix mutex locking David Marchand
2023-04-04 12:48 ` [PATCH v3 15/16] eal/windows: disable lock check on alarm code David Marchand
2023-04-04 16:08 ` Tyler Retzlaff
2023-04-04 21:02 ` Dmitry Kozlyuk
2023-04-04 12:48 ` [PATCH v3 16/16] enable lock check David Marchand
2023-04-11 3:21 ` Sachin Saxena (OSS)
2023-04-23 20:09 ` [PATCH v3 00/16] Enable lock annotations on most libraries and drivers Thomas Monjalon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e17bce10-886c-4d35-8ed2-2ed9ae652b30@app.fastmail.com \
--to=grive@u256.net \
--cc=david.marchand@redhat.com \
--cc=dev@dpdk.org \
--cc=thomas@monjalon.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).