Re: [PATCH v6 0/9] Lock annotations

[David Marchand <david.marchand@redhat.com>]

Hello,

On Thu, Feb 9, 2023 at 8:59 AM Xia, Chenbo <chenbo.xia@intel.com> wrote:
> > Subject: [PATCH v6 0/9] Lock annotations
> >
> > vhost internals involves multiple locks to protect data access by
> > multiple threads.
> >
> > This series uses clang thread safety checks [1] to catch issues during
> > compilation: EAL spinlock, seqlock and rwlock are annotated and vhost
> > code is instrumented so that clang can statically check correctness.
> >
> > Those annotations are quite heavy to maintain because the full path of
> > code must be annotated (as can be seen in the vhost datapath code),
> > but I think it is worth using.
> >
> > This has been tested against the whole tree and some fixes are already
> > flying on the mailing list (see [2] for a list).
> >
> > If this first series is merged, I will prepare a followup series for EAL
> > and other libraries.
> >
> >
> > 1: https://clang.llvm.org/docs/ThreadSafetyAnalysis.html
> > 2:
> > https://patchwork.dpdk.org/bundle/dmarchand/lock_fixes/?state=*&archive=bo
> > th
> >
> > --
> > David Marchand
> >
> > Changes since v5:
> > - rebased after lib/vhost updates (patches 5 and 7),
> >
> > Changes since v4:
> > - masked annotations from Doxygen as it seems confused with some
> >   constructs,
> > - fixed typos,
> >

[snip]

> >
> >
> > David Marchand (9):
> >   eal: annotate spinlock, rwlock and seqlock
> >   vhost: simplify need reply handling
> >   vhost: terminate when access lock is not taken
> >   vhost: annotate virtqueue access lock
> >   vhost: annotate async accesses
> >   vhost: always take IOTLB lock
> >   vhost: annotate IOTLB lock
> >   vhost: annotate vDPA device list accesses
> >   vhost: enable lock check
> >
> >  doc/api/doxy-api.conf.in                      |  11 ++
> >  .../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                 |   2 +
> >  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 +
> >  lib/vhost/iotlb.h                             |   4 +
> >  lib/vhost/meson.build                         |   2 +
> >  lib/vhost/vdpa.c                              |  20 +--
> >  lib/vhost/vhost.c                             |  38 ++---
> >  lib/vhost/vhost.h                             |  34 ++++-
> >  lib/vhost/vhost_crypto.c                      |   8 ++
> >  lib/vhost/vhost_user.c                        | 131 ++++++++----------
> >  lib/vhost/virtio_net.c                        | 118 ++++++++++++----
> >  21 files changed, 405 insertions(+), 150 deletions(-)
> >  create mode 100644 lib/eal/include/rte_lock_annotations.h
> >
> > --
> > 2.39.1
>
> Seems one compilation error reported? Not sure it's related or not.

We discovered recently that Intel CI filters out doc/ updates in patches (?!).
https://inbox.dpdk.org/dev/20220328121758.26632-1-david.marchand@redhat.com/T/#mb42fa6342204dd01c923339ec0b1587bc0b5ac0a

So yes, it is "related" to the series, but you can ignore Intel CI
report because the reported issue is fixed since the v4 revision.


Btw, thanks for the review Chenbo!


-- 
David Marchand