On 1/19/23 19:46, David Marchand wrote:
clang offers some thread safety checks, statically verifying that locks
are taken and released in the code.
To use those checks, the full code leading to taking or releasing locks
must be annotated with some attributes.
Wrap those attributes into our own set of macros.
rwlock, seqlock and the "normal" spinlock are instrumented.
Those checks might be of interest out of DPDK, but it requires that the
including application locks are annotated.
On the other hand, applications out there might have been using
those same checks.
To be on the safe side, keep this instrumentation under a
RTE_ANNOTATE_LOCKS internal build flag.
A component may en/disable this check by setting
annotate_locks = true/false in its meson.build.
Signed-off-by: David Marchand <david.march...@redhat.com>
---
Changes since RFC v3:
- rebased,
- added some documentation,
- added assert attribute,
- instrumented seqlock,
- cleaned annotations in arch-specific headers,
Changes since RFC v2:
- fixed rwlock trylock,
- instrumented _tm spinlocks,
- aligned attribute names to clang,
---
.../prog_guide/env_abstraction_layer.rst | 24 ++++++
doc/guides/rel_notes/release_23_03.rst | 5 ++
drivers/meson.build | 5 ++
lib/eal/include/generic/rte_rwlock.h | 27 +++++--
lib/eal/include/generic/rte_spinlock.h | 31 +++++---
lib/eal/include/meson.build | 1 +
lib/eal/include/rte_lock_annotations.h | 73 +++++++++++++++++++
lib/eal/include/rte_seqlock.h | 6 ++
lib/eal/ppc/include/rte_spinlock.h | 3 +
lib/eal/x86/include/rte_rwlock.h | 4 +
lib/eal/x86/include/rte_spinlock.h | 9 +++
lib/meson.build | 5 ++
12 files changed, 179 insertions(+), 14 deletions(-)
create mode 100644 lib/eal/include/rte_lock_annotations.h
diff --git a/doc/guides/prog_guide/env_abstraction_layer.rst
b/doc/guides/prog_guide/env_abstraction_layer.rst
index 35fbebe1be..6c1e8ba985 100644
--- a/doc/guides/prog_guide/env_abstraction_layer.rst
+++ b/doc/guides/prog_guide/env_abstraction_layer.rst
@@ -529,6 +529,30 @@ Misc Functions
Locks and atomic operations are per-architecture (i686 and x86_64).
+Lock annotations
+~~~~~~~~~~~~~~~~
+
+R/W locks, seq locks and spinlocks have been instrumented to help developers in
+catching issues in DPDK.
+
+This instrumentation relies on
+`clang Thread Safety checks
<https://clang.llvm.org/docs/ThreadSafetyAnalysis.html>`_.
+All attributes are prefixed with __rte and are fully described in the clang
+documentation.
+
+Some general comments:
+
+- it is important that lock requirements are expressed at the function
+ declaration level in headers so that other code units can be inspected,
+- when some global lock is necessary to some user-exposed API, it is preferred
+ to expose it via an internal helper rather than expose the global variable,
+- there are a list of known limitations with clang instrumentation, but before
+ waiving checks with ``__rte_no_thread_safety_analysis`` in your code, please
+ discuss it on the mailing list,
+
+A DPDK library/driver can enabled/disable the checks by setting
s/enabled/enable/
+``annotate_locks`` accordingly in its ``meson.build`` file.
+
IOVA Mode Detection
~~~~~~~~~~~~~~~~~~~
diff --git a/doc/guides/rel_notes/release_23_03.rst b/doc/guides/rel_notes/release_23_03.rst
index c15f6fbb9f..5425e59c65 100644
--- a/doc/guides/rel_notes/release_23_03.rst
+++ b/doc/guides/rel_notes/release_23_03.rst
@@ -55,6 +55,11 @@ New Features
Also, make sure to start the actual text at the margin.
=======================================================
+* **Introduced lock annotations.**
+
+ Added lock annotations attributes to that clang can statically analyze lock
s/to/so/
+ correctness.
+
With above typos fixed:
Reviewed-by: Maxime Coquelin <maxime.coque...@redhat.com>
Thanks,
Maxime
