From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
	by inbox.dpdk.org (Postfix) with ESMTP id 0BAAF41C2D;
	Tue,  7 Feb 2023 11:45:47 +0100 (CET)
Received: from mails.dpdk.org (localhost [127.0.0.1])
	by mails.dpdk.org (Postfix) with ESMTP id 89F7440EF0;
	Tue,  7 Feb 2023 11:45:46 +0100 (CET)
Received: from us-smtp-delivery-124.mimecast.com
 (us-smtp-delivery-124.mimecast.com [170.10.129.124])
 by mails.dpdk.org (Postfix) with ESMTP id 29DD940042
 for <dev@dpdk.org>; Tue,  7 Feb 2023 11:45:45 +0100 (CET)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;
 s=mimecast20190719; t=1675766744;
 h=from:from:reply-to:subject:subject:date:date:message-id:message-id:
 to:to:cc:cc:mime-version:mime-version:content-type:content-type:
 content-transfer-encoding:content-transfer-encoding:
 in-reply-to:in-reply-to:references:references;
 bh=ApMewxeRT56JaIZAt9b4/azEWQTO+Hn5foXgSecf4GU=;
 b=IKGzqYPzVJsmWxGoPuwM9T9LVZP4VMFOwENZh8VJZGnowlNbleC6YdxZej5JVNFLyzYe7s
 52E7IETbOG2qfqXnKS6I/js+TF4LglhPnjHrvR6QP/JhEMDCqONPvLDsm3AwUN3J9xAgtG
 WVDT5+yG6kw3r0PSjoTHsY8vvpSEU6w=
Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com
 [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS
 (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id
 us-mta-657-Szj0sQVBPQ-90Bu76l1Jog-1; Tue, 07 Feb 2023 05:45:41 -0500
X-MC-Unique: Szj0sQVBPQ-90Bu76l1Jog-1
Received: from smtp.corp.redhat.com (int-mx10.intmail.prod.int.rdu2.redhat.com
 [10.11.54.10])
 (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))
 (No client certificate requested)
 by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 2260B3C0D18B;
 Tue,  7 Feb 2023 10:45:41 +0000 (UTC)
Received: from dmarchan.redhat.com (ovpn-195-46.brq.redhat.com [10.40.195.46])
 by smtp.corp.redhat.com (Postfix) with ESMTP id 3CC84492B21;
 Tue,  7 Feb 2023 10:45:39 +0000 (UTC)
From: David Marchand <david.marchand@redhat.com>
To: dev@dpdk.org
Cc: maxime.coquelin@redhat.com, stephen@networkplumber.org,
 chenbo.xia@intel.com, jiayu.hu@intel.com, yuanx.wang@intel.com,
 xuan.ding@intel.com, mb@smartsharesystems.com
Subject: [PATCH v6 0/9] Lock annotations
Date: Tue,  7 Feb 2023 11:45:23 +0100
Message-Id: <20230207104532.2370869-1-david.marchand@redhat.com>
In-Reply-To: <20220328121758.26632-1-david.marchand@redhat.com>
References: <20220328121758.26632-1-david.marchand@redhat.com>
MIME-Version: 1.0
X-Scanned-By: MIMEDefang 3.1 on 10.11.54.10
X-Mimecast-Spam-Score: 0
X-Mimecast-Originator: redhat.com
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org

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=both

-- 
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,

Changes since RFC v3:
- sorry Maxime, it has been too long since RFC v3 and the code evolved,
  so I dropped all your review tags,
- rebased,
- added documentation,
- dropped/fixed annotations in arch-specific and EAL headers,
- rewrote need reply handling so that we don't have to waive the check
  on the associated functions,
- separated IOTLB lock unconditional acquire from the annotation patch,
- rewrote runtime checks for "unsafe" functions using a panicking assert
  helper,

Changes since RFC v2:
- fixed trylock annotations for rwlock,
- annotated _tm flavors of spinlock and rwlock,
- removed Maxime vhost fix from series (since Mimecast does not like
  me sending Maxime patch...), added a dependency on original fix
  as a hint for reviewers,
- renamed attributes,

Changes since RFC v1:
- Cc'd people who have pending patches for vhost,
- moved annotations to EAL and removed wrappers in vhost,
- as a result of moving to EAL, this series will be tested against
  the main repo, so patch 1 has been kept as part of the series
  even if already applied to next-virtio,
- refined/split patches and annotated all spinlocks in vhost,


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