From patchwork Thu Jan 19 18:46:12 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Marchand X-Patchwork-Id: 122367 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 544034241A; Thu, 19 Jan 2023 19:46:38 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 1DEB841101; Thu, 19 Jan 2023 19:46:38 +0100 (CET) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mails.dpdk.org (Postfix) with ESMTP id 47160400D5 for ; Thu, 19 Jan 2023 19:46:36 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1674153995; 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=Fx1a4dGq2UmUqZiAxhR8IqW7Y1Ky7ar6ELZflX1c3OA=; b=VQdcQMy3RQR+UdLVTmcfZdOe02D1Vq98JzH4tIxmwFgI48TFHouivAp8betxeJRIfFKRNJ UqfDAR64NeCq25fQ0lJM85eiO8ASPsNlYCjPtjT4RCfKZnQq2r9aWdWhQPVFdSw6BRASoq ydTkr11H/EiJ41xpwFJ5vZWiLv/JecA= 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-508-QUWrLtQnNquZcst17ns4gg-1; Thu, 19 Jan 2023 13:46:32 -0500 X-MC-Unique: QUWrLtQnNquZcst17ns4gg-1 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.rdu2.redhat.com [10.11.54.7]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id E71F21C05AEE; Thu, 19 Jan 2023 18:46:31 +0000 (UTC) Received: from dmarchan.redhat.com (ovpn-193-92.brq.redhat.com [10.40.193.92]) by smtp.corp.redhat.com (Postfix) with ESMTP id 559E7140EBF4; Thu, 19 Jan 2023 18:46:29 +0000 (UTC) From: David Marchand 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, Anatoly Burakov , =?utf-8?q?Mattias_R=C3=B6nnblom?= , David Christensen , Bruce Richardson , Konstantin Ananyev Subject: [PATCH v4 1/9] eal: annotate spinlock, rwlock and seqlock Date: Thu, 19 Jan 2023 19:46:12 +0100 Message-Id: <20230119184620.3195267-2-david.marchand@redhat.com> In-Reply-To: <20230119184620.3195267-1-david.marchand@redhat.com> References: <20220328121758.26632-1-david.marchand@redhat.com> <20230119184620.3195267-1-david.marchand@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.7 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org 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 Reviewed-by: Maxime Coquelin --- 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 `_. +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 +``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 + correctness. + * **Updated Intel QuickAssist Technology (QAT) crypto driver.** * Added support for SHA3 224/256/384/512 plain hash in QAT GEN 3. diff --git a/drivers/meson.build b/drivers/meson.build index c6d619200f..bddc4a6cc4 100644 --- a/drivers/meson.build +++ b/drivers/meson.build @@ -91,6 +91,7 @@ foreach subpath:subdirs build = true # set to false to disable, e.g. missing deps reason = '' # set if build == false to explain name = drv + annotate_locks = false sources = [] headers = [] driver_sdk_headers = [] # public headers included by drivers @@ -167,6 +168,10 @@ foreach subpath:subdirs enabled_drivers += name lib_name = '_'.join(['rte', class, name]) cflags += '-DRTE_LOG_DEFAULT_LOGTYPE=' + '.'.join([log_prefix, name]) + if annotate_locks and cc.has_argument('-Wthread-safety') + cflags += '-DRTE_ANNOTATE_LOCKS' + cflags += '-Wthread-safety' + endif dpdk_conf.set(lib_name.to_upper(), 1) dpdk_extra_ldflags += pkgconfig_extra_libs diff --git a/lib/eal/include/generic/rte_rwlock.h b/lib/eal/include/generic/rte_rwlock.h index 233d4262be..d45c22c189 100644 --- a/lib/eal/include/generic/rte_rwlock.h +++ b/lib/eal/include/generic/rte_rwlock.h @@ -30,6 +30,7 @@ extern "C" { #include #include +#include #include /** @@ -55,7 +56,7 @@ extern "C" { /* Writer is waiting or has lock */ #define RTE_RWLOCK_READ 0x4 /* Reader increment */ -typedef struct { +typedef struct __rte_lockable { int32_t cnt; } rte_rwlock_t; @@ -84,6 +85,8 @@ rte_rwlock_init(rte_rwlock_t *rwl) */ static inline void rte_rwlock_read_lock(rte_rwlock_t *rwl) + __rte_shared_lock_function(rwl) + __rte_no_thread_safety_analysis { int32_t x; @@ -119,6 +122,8 @@ rte_rwlock_read_lock(rte_rwlock_t *rwl) */ static inline int rte_rwlock_read_trylock(rte_rwlock_t *rwl) + __rte_shared_trylock_function(0, rwl) + __rte_no_thread_safety_analysis { int32_t x; @@ -150,6 +155,8 @@ rte_rwlock_read_trylock(rte_rwlock_t *rwl) */ static inline void rte_rwlock_read_unlock(rte_rwlock_t *rwl) + __rte_unlock_function(rwl) + __rte_no_thread_safety_analysis { __atomic_fetch_sub(&rwl->cnt, RTE_RWLOCK_READ, __ATOMIC_RELEASE); } @@ -166,6 +173,8 @@ rte_rwlock_read_unlock(rte_rwlock_t *rwl) */ static inline int rte_rwlock_write_trylock(rte_rwlock_t *rwl) + __rte_exclusive_trylock_function(0, rwl) + __rte_no_thread_safety_analysis { int32_t x; @@ -186,6 +195,8 @@ rte_rwlock_write_trylock(rte_rwlock_t *rwl) */ static inline void rte_rwlock_write_lock(rte_rwlock_t *rwl) + __rte_exclusive_lock_function(rwl) + __rte_no_thread_safety_analysis { int32_t x; @@ -219,6 +230,8 @@ rte_rwlock_write_lock(rte_rwlock_t *rwl) */ static inline void rte_rwlock_write_unlock(rte_rwlock_t *rwl) + __rte_unlock_function(rwl) + __rte_no_thread_safety_analysis { __atomic_fetch_sub(&rwl->cnt, RTE_RWLOCK_WRITE, __ATOMIC_RELEASE); } @@ -237,7 +250,8 @@ rte_rwlock_write_unlock(rte_rwlock_t *rwl) * A pointer to a rwlock structure. */ static inline void -rte_rwlock_read_lock_tm(rte_rwlock_t *rwl); +rte_rwlock_read_lock_tm(rte_rwlock_t *rwl) + __rte_shared_lock_function(rwl); /** * Commit hardware memory transaction or release the read lock if the lock is used as a fall-back @@ -246,7 +260,8 @@ rte_rwlock_read_lock_tm(rte_rwlock_t *rwl); * A pointer to the rwlock structure. */ static inline void -rte_rwlock_read_unlock_tm(rte_rwlock_t *rwl); +rte_rwlock_read_unlock_tm(rte_rwlock_t *rwl) + __rte_unlock_function(rwl); /** * Try to execute critical section in a hardware memory transaction, if it @@ -262,7 +277,8 @@ rte_rwlock_read_unlock_tm(rte_rwlock_t *rwl); * A pointer to a rwlock structure. */ static inline void -rte_rwlock_write_lock_tm(rte_rwlock_t *rwl); +rte_rwlock_write_lock_tm(rte_rwlock_t *rwl) + __rte_exclusive_lock_function(rwl); /** * Commit hardware memory transaction or release the write lock if the lock is used as a fall-back @@ -271,7 +287,8 @@ rte_rwlock_write_lock_tm(rte_rwlock_t *rwl); * A pointer to a rwlock structure. */ static inline void -rte_rwlock_write_unlock_tm(rte_rwlock_t *rwl); +rte_rwlock_write_unlock_tm(rte_rwlock_t *rwl) + __rte_unlock_function(rwl); #ifdef __cplusplus } diff --git a/lib/eal/include/generic/rte_spinlock.h b/lib/eal/include/generic/rte_spinlock.h index 73ed4bfbdc..8ca47bbfaa 100644 --- a/lib/eal/include/generic/rte_spinlock.h +++ b/lib/eal/include/generic/rte_spinlock.h @@ -22,12 +22,13 @@ #ifdef RTE_FORCE_INTRINSICS #include #endif +#include #include /** * The rte_spinlock_t type. */ -typedef struct { +typedef struct __rte_lockable { volatile int locked; /**< lock status 0 = unlocked, 1 = locked */ } rte_spinlock_t; @@ -55,11 +56,13 @@ rte_spinlock_init(rte_spinlock_t *sl) * A pointer to the spinlock. */ static inline void -rte_spinlock_lock(rte_spinlock_t *sl); +rte_spinlock_lock(rte_spinlock_t *sl) + __rte_exclusive_lock_function(sl); #ifdef RTE_FORCE_INTRINSICS static inline void rte_spinlock_lock(rte_spinlock_t *sl) + __rte_no_thread_safety_analysis { int exp = 0; @@ -79,11 +82,13 @@ rte_spinlock_lock(rte_spinlock_t *sl) * A pointer to the spinlock. */ static inline void -rte_spinlock_unlock (rte_spinlock_t *sl); +rte_spinlock_unlock(rte_spinlock_t *sl) + __rte_unlock_function(sl); #ifdef RTE_FORCE_INTRINSICS static inline void -rte_spinlock_unlock (rte_spinlock_t *sl) +rte_spinlock_unlock(rte_spinlock_t *sl) + __rte_no_thread_safety_analysis { __atomic_store_n(&sl->locked, 0, __ATOMIC_RELEASE); } @@ -99,11 +104,13 @@ rte_spinlock_unlock (rte_spinlock_t *sl) */ __rte_warn_unused_result static inline int -rte_spinlock_trylock (rte_spinlock_t *sl); +rte_spinlock_trylock(rte_spinlock_t *sl) + __rte_exclusive_trylock_function(1, sl); #ifdef RTE_FORCE_INTRINSICS static inline int -rte_spinlock_trylock (rte_spinlock_t *sl) +rte_spinlock_trylock(rte_spinlock_t *sl) + __rte_no_thread_safety_analysis { int exp = 0; return __atomic_compare_exchange_n(&sl->locked, &exp, 1, @@ -147,7 +154,8 @@ static inline int rte_tm_supported(void); * A pointer to the spinlock. */ static inline void -rte_spinlock_lock_tm(rte_spinlock_t *sl); +rte_spinlock_lock_tm(rte_spinlock_t *sl) + __rte_exclusive_lock_function(sl); /** * Commit hardware memory transaction or release the spinlock if @@ -157,7 +165,8 @@ rte_spinlock_lock_tm(rte_spinlock_t *sl); * A pointer to the spinlock. */ static inline void -rte_spinlock_unlock_tm(rte_spinlock_t *sl); +rte_spinlock_unlock_tm(rte_spinlock_t *sl) + __rte_unlock_function(sl); /** * Try to execute critical section in a hardware memory transaction, @@ -177,7 +186,8 @@ rte_spinlock_unlock_tm(rte_spinlock_t *sl); */ __rte_warn_unused_result static inline int -rte_spinlock_trylock_tm(rte_spinlock_t *sl); +rte_spinlock_trylock_tm(rte_spinlock_t *sl) + __rte_exclusive_trylock_function(1, sl); /** * The rte_spinlock_recursive_t type. @@ -213,6 +223,7 @@ static inline void rte_spinlock_recursive_init(rte_spinlock_recursive_t *slr) * A pointer to the recursive spinlock. */ static inline void rte_spinlock_recursive_lock(rte_spinlock_recursive_t *slr) + __rte_no_thread_safety_analysis { int id = rte_gettid(); @@ -229,6 +240,7 @@ static inline void rte_spinlock_recursive_lock(rte_spinlock_recursive_t *slr) * A pointer to the recursive spinlock. */ static inline void rte_spinlock_recursive_unlock(rte_spinlock_recursive_t *slr) + __rte_no_thread_safety_analysis { if (--(slr->count) == 0) { slr->user = -1; @@ -247,6 +259,7 @@ static inline void rte_spinlock_recursive_unlock(rte_spinlock_recursive_t *slr) */ __rte_warn_unused_result static inline int rte_spinlock_recursive_trylock(rte_spinlock_recursive_t *slr) + __rte_no_thread_safety_analysis { int id = rte_gettid(); diff --git a/lib/eal/include/meson.build b/lib/eal/include/meson.build index cfcd40aaed..b0db9b3b3a 100644 --- a/lib/eal/include/meson.build +++ b/lib/eal/include/meson.build @@ -27,6 +27,7 @@ headers += files( 'rte_keepalive.h', 'rte_launch.h', 'rte_lcore.h', + 'rte_lock_annotations.h', 'rte_log.h', 'rte_malloc.h', 'rte_mcslock.h', diff --git a/lib/eal/include/rte_lock_annotations.h b/lib/eal/include/rte_lock_annotations.h new file mode 100644 index 0000000000..9fc50082d6 --- /dev/null +++ b/lib/eal/include/rte_lock_annotations.h @@ -0,0 +1,73 @@ +/* SPDX-License-Identifier: BSD-3-Clause + * Copyright(c) 2022 Red Hat, Inc. + */ + +#ifndef RTE_LOCK_ANNOTATIONS_H +#define RTE_LOCK_ANNOTATIONS_H + +#ifdef __cplusplus +extern "C" { +#endif + +#ifdef RTE_ANNOTATE_LOCKS + +#define __rte_lockable \ + __attribute__((lockable)) + +#define __rte_guarded_by(...) \ + __attribute__((guarded_by(__VA_ARGS__))) +#define __rte_guarded_var \ + __attribute__((guarded_var)) + +#define __rte_exclusive_locks_required(...) \ + __attribute__((exclusive_locks_required(__VA_ARGS__))) +#define __rte_exclusive_lock_function(...) \ + __attribute__((exclusive_lock_function(__VA_ARGS__))) +#define __rte_exclusive_trylock_function(ret, ...) \ + __attribute__((exclusive_trylock_function(ret, __VA_ARGS__))) +#define __rte_assert_exclusive_lock(...) \ + __attribute__((assert_exclusive_lock(__VA_ARGS__))) + +#define __rte_shared_locks_required(...) \ + __attribute__((shared_locks_required(__VA_ARGS__))) +#define __rte_shared_lock_function(...) \ + __attribute__((shared_lock_function(__VA_ARGS__))) +#define __rte_shared_trylock_function(ret, ...) \ + __attribute__((shared_trylock_function(ret, __VA_ARGS__))) +#define __rte_assert_shared_lock(...) \ + __attribute__((assert_shared_lock(__VA_ARGS__))) + +#define __rte_unlock_function(...) \ + __attribute__((unlock_function(__VA_ARGS__))) + +#define __rte_no_thread_safety_analysis \ + __attribute__((no_thread_safety_analysis)) + +#else /* ! RTE_ANNOTATE_LOCKS */ + +#define __rte_lockable + +#define __rte_guarded_by(...) +#define __rte_guarded_var + +#define __rte_exclusive_locks_required(...) +#define __rte_exclusive_lock_function(...) +#define __rte_exclusive_trylock_function(...) +#define __rte_assert_exclusive_lock(...) + +#define __rte_shared_locks_required(...) +#define __rte_shared_lock_function(...) +#define __rte_shared_trylock_function(...) +#define __rte_assert_shared_lock(...) + +#define __rte_unlock_function(...) + +#define __rte_no_thread_safety_analysis + +#endif /* RTE_ANNOTATE_LOCKS */ + +#ifdef __cplusplus +} +#endif + +#endif /* RTE_LOCK_ANNOTATIONS_H */ diff --git a/lib/eal/include/rte_seqlock.h b/lib/eal/include/rte_seqlock.h index 1663af62e8..f34fa2005d 100644 --- a/lib/eal/include/rte_seqlock.h +++ b/lib/eal/include/rte_seqlock.h @@ -215,6 +215,9 @@ rte_seqlock_read_retry(const rte_seqlock_t *seqlock, uint32_t begin_sn) __rte_experimental static inline void rte_seqlock_write_lock(rte_seqlock_t *seqlock) +#ifndef __DOXYGEN__ + __rte_exclusive_lock_function(&seqlock->lock) +#endif { /* To synchronize with other writers. */ rte_spinlock_lock(&seqlock->lock); @@ -240,6 +243,9 @@ rte_seqlock_write_lock(rte_seqlock_t *seqlock) __rte_experimental static inline void rte_seqlock_write_unlock(rte_seqlock_t *seqlock) +#ifndef __DOXYGEN__ + __rte_unlock_function(&seqlock->lock) +#endif { rte_seqcount_write_end(&seqlock->count); diff --git a/lib/eal/ppc/include/rte_spinlock.h b/lib/eal/ppc/include/rte_spinlock.h index 149ec245c7..3a4c905b22 100644 --- a/lib/eal/ppc/include/rte_spinlock.h +++ b/lib/eal/ppc/include/rte_spinlock.h @@ -20,6 +20,7 @@ extern "C" { static inline void rte_spinlock_lock(rte_spinlock_t *sl) + __rte_no_thread_safety_analysis { while (__sync_lock_test_and_set(&sl->locked, 1)) while (sl->locked) @@ -28,12 +29,14 @@ rte_spinlock_lock(rte_spinlock_t *sl) static inline void rte_spinlock_unlock(rte_spinlock_t *sl) + __rte_no_thread_safety_analysis { __sync_lock_release(&sl->locked); } static inline int rte_spinlock_trylock(rte_spinlock_t *sl) + __rte_no_thread_safety_analysis { return __sync_lock_test_and_set(&sl->locked, 1) == 0; } diff --git a/lib/eal/x86/include/rte_rwlock.h b/lib/eal/x86/include/rte_rwlock.h index eec4c7123c..1796b69265 100644 --- a/lib/eal/x86/include/rte_rwlock.h +++ b/lib/eal/x86/include/rte_rwlock.h @@ -14,6 +14,7 @@ extern "C" { static inline void rte_rwlock_read_lock_tm(rte_rwlock_t *rwl) + __rte_no_thread_safety_analysis { if (likely(rte_try_tm(&rwl->cnt))) return; @@ -22,6 +23,7 @@ rte_rwlock_read_lock_tm(rte_rwlock_t *rwl) static inline void rte_rwlock_read_unlock_tm(rte_rwlock_t *rwl) + __rte_no_thread_safety_analysis { if (unlikely(rwl->cnt)) rte_rwlock_read_unlock(rwl); @@ -31,6 +33,7 @@ rte_rwlock_read_unlock_tm(rte_rwlock_t *rwl) static inline void rte_rwlock_write_lock_tm(rte_rwlock_t *rwl) + __rte_no_thread_safety_analysis { if (likely(rte_try_tm(&rwl->cnt))) return; @@ -39,6 +42,7 @@ rte_rwlock_write_lock_tm(rte_rwlock_t *rwl) static inline void rte_rwlock_write_unlock_tm(rte_rwlock_t *rwl) + __rte_no_thread_safety_analysis { if (unlikely(rwl->cnt)) rte_rwlock_write_unlock(rwl); diff --git a/lib/eal/x86/include/rte_spinlock.h b/lib/eal/x86/include/rte_spinlock.h index e2e2b2643c..0b20ddfd73 100644 --- a/lib/eal/x86/include/rte_spinlock.h +++ b/lib/eal/x86/include/rte_spinlock.h @@ -23,6 +23,7 @@ extern "C" { #ifndef RTE_FORCE_INTRINSICS static inline void rte_spinlock_lock(rte_spinlock_t *sl) + __rte_no_thread_safety_analysis { int lock_val = 1; asm volatile ( @@ -43,6 +44,7 @@ rte_spinlock_lock(rte_spinlock_t *sl) static inline void rte_spinlock_unlock (rte_spinlock_t *sl) + __rte_no_thread_safety_analysis { int unlock_val = 0; asm volatile ( @@ -54,6 +56,7 @@ rte_spinlock_unlock (rte_spinlock_t *sl) static inline int rte_spinlock_trylock (rte_spinlock_t *sl) + __rte_no_thread_safety_analysis { int lockval = 1; @@ -121,6 +124,7 @@ rte_try_tm(volatile int *lock) static inline void rte_spinlock_lock_tm(rte_spinlock_t *sl) + __rte_no_thread_safety_analysis { if (likely(rte_try_tm(&sl->locked))) return; @@ -130,6 +134,7 @@ rte_spinlock_lock_tm(rte_spinlock_t *sl) static inline int rte_spinlock_trylock_tm(rte_spinlock_t *sl) + __rte_no_thread_safety_analysis { if (likely(rte_try_tm(&sl->locked))) return 1; @@ -139,6 +144,7 @@ rte_spinlock_trylock_tm(rte_spinlock_t *sl) static inline void rte_spinlock_unlock_tm(rte_spinlock_t *sl) + __rte_no_thread_safety_analysis { if (unlikely(sl->locked)) rte_spinlock_unlock(sl); @@ -148,6 +154,7 @@ rte_spinlock_unlock_tm(rte_spinlock_t *sl) static inline void rte_spinlock_recursive_lock_tm(rte_spinlock_recursive_t *slr) + __rte_no_thread_safety_analysis { if (likely(rte_try_tm(&slr->sl.locked))) return; @@ -157,6 +164,7 @@ rte_spinlock_recursive_lock_tm(rte_spinlock_recursive_t *slr) static inline void rte_spinlock_recursive_unlock_tm(rte_spinlock_recursive_t *slr) + __rte_no_thread_safety_analysis { if (unlikely(slr->sl.locked)) rte_spinlock_recursive_unlock(slr); @@ -166,6 +174,7 @@ rte_spinlock_recursive_unlock_tm(rte_spinlock_recursive_t *slr) static inline int rte_spinlock_recursive_trylock_tm(rte_spinlock_recursive_t *slr) + __rte_no_thread_safety_analysis { if (likely(rte_try_tm(&slr->sl.locked))) return 1; diff --git a/lib/meson.build b/lib/meson.build index a90fee31b7..450c061d2b 100644 --- a/lib/meson.build +++ b/lib/meson.build @@ -120,6 +120,7 @@ foreach l:libraries reason = '' # set if build == false to explain why name = l use_function_versioning = false + annotate_locks = false sources = [] headers = [] indirect_headers = [] # public headers not directly included by apps @@ -202,6 +203,10 @@ foreach l:libraries cflags += '-DRTE_USE_FUNCTION_VERSIONING' endif cflags += '-DRTE_LOG_DEFAULT_LOGTYPE=lib.' + l + if annotate_locks and cc.has_argument('-Wthread-safety') + cflags += '-DRTE_ANNOTATE_LOCKS' + cflags += '-Wthread-safety' + endif # first build static lib static_lib = static_library(libname, From patchwork Thu Jan 19 18:46:13 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Marchand X-Patchwork-Id: 122368 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id D07E94241A; Thu, 19 Jan 2023 19:46:46 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 4225642D9F; Thu, 19 Jan 2023 19:46:39 +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 EB708400D5 for ; Thu, 19 Jan 2023 19:46:37 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1674153997; 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=CCTZ9nHm4rcb6npUCUqwk9cezwwdfcpcr64v+2kBX6w=; b=UnmqYuhxDW+1qnWFF6uXszyz9804jDt3OJlDxjrkxG0Y1j76dd2xraaO3X1z0ydnLz2jPx TnktFKkV+rA+HK1PNBGmYibIip4KGmOHJg0HXABXT9bZU13NfEK0AHU02WBBvCsmu2kY+D 7lgcjKrHJykk2NZgPWwo2rBfoHWa+bo= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-611-L8Z9tJXMMgKWkpZitqOnNw-1; Thu, 19 Jan 2023 13:46:36 -0500 X-MC-Unique: L8Z9tJXMMgKWkpZitqOnNw-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 92DF6885620; Thu, 19 Jan 2023 18:46:35 +0000 (UTC) Received: from dmarchan.redhat.com (ovpn-193-92.brq.redhat.com [10.40.193.92]) by smtp.corp.redhat.com (Postfix) with ESMTP id 1BAAE492C3C; Thu, 19 Jan 2023 18:46:33 +0000 (UTC) From: David Marchand 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 Subject: [PATCH v4 2/9] vhost: simplify need reply handling Date: Thu, 19 Jan 2023 19:46:13 +0100 Message-Id: <20230119184620.3195267-3-david.marchand@redhat.com> In-Reply-To: <20230119184620.3195267-1-david.marchand@redhat.com> References: <20220328121758.26632-1-david.marchand@redhat.com> <20230119184620.3195267-1-david.marchand@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.9 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Dedicate send_vhost_slave_message() helper to the case when no reply is needed. Add a send_vhost_slave_message_process_reply() helper for the opposite. This new helper merges both send_vhost_slave_message() and the code previously in process_slave_message_reply(). The slave_req_lock lock is then only handled in this helper which will make lock checks trivial. Signed-off-by: David Marchand Reviewed-by: Maxime Coquelin --- lib/vhost/vhost_user.c | 119 ++++++++++++++++++----------------------- 1 file changed, 51 insertions(+), 68 deletions(-) diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c index 9902ae9944..3f6c5df900 100644 --- a/lib/vhost/vhost_user.c +++ b/lib/vhost/vhost_user.c @@ -2868,18 +2868,46 @@ send_vhost_reply(struct virtio_net *dev, int sockfd, struct vhu_msg_context *ctx } static int -send_vhost_slave_message(struct virtio_net *dev, - struct vhu_msg_context *ctx) +send_vhost_slave_message(struct virtio_net *dev, struct vhu_msg_context *ctx) +{ + return send_vhost_message(dev, dev->slave_req_fd, ctx); +} + +static int +send_vhost_slave_message_process_reply(struct virtio_net *dev, struct vhu_msg_context *ctx) { + struct vhu_msg_context msg_reply; int ret; - if (ctx->msg.flags & VHOST_USER_NEED_REPLY) - rte_spinlock_lock(&dev->slave_req_lock); + rte_spinlock_lock(&dev->slave_req_lock); + ret = send_vhost_slave_message(dev, ctx); + if (ret < 0) { + VHOST_LOG_CONFIG(dev->ifname, ERR, "failed to send config change (%d)\n", ret); + goto out; + } - ret = send_vhost_message(dev, dev->slave_req_fd, ctx); - if (ret < 0 && (ctx->msg.flags & VHOST_USER_NEED_REPLY)) - rte_spinlock_unlock(&dev->slave_req_lock); + ret = read_vhost_message(dev, dev->slave_req_fd, &msg_reply); + if (ret <= 0) { + if (ret < 0) + VHOST_LOG_CONFIG(dev->ifname, ERR, + "vhost read slave message reply failed\n"); + else + VHOST_LOG_CONFIG(dev->ifname, INFO, "vhost peer closed\n"); + ret = -1; + goto out; + } + + if (msg_reply.msg.request.slave != ctx->msg.request.slave) { + VHOST_LOG_CONFIG(dev->ifname, ERR, + "received unexpected msg type (%u), expected %u\n", + msg_reply.msg.request.slave, ctx->msg.request.slave); + ret = -1; + goto out; + } + ret = msg_reply.msg.payload.u64 ? -1 : 0; +out: + rte_spinlock_unlock(&dev->slave_req_lock); return ret; } @@ -3203,42 +3231,6 @@ vhost_user_msg_handler(int vid, int fd) return ret; } -static int process_slave_message_reply(struct virtio_net *dev, - const struct vhu_msg_context *ctx) -{ - struct vhu_msg_context msg_reply; - int ret; - - if ((ctx->msg.flags & VHOST_USER_NEED_REPLY) == 0) - return 0; - - ret = read_vhost_message(dev, dev->slave_req_fd, &msg_reply); - if (ret <= 0) { - if (ret < 0) - VHOST_LOG_CONFIG(dev->ifname, ERR, - "vhost read slave message reply failed\n"); - else - VHOST_LOG_CONFIG(dev->ifname, INFO, "vhost peer closed\n"); - ret = -1; - goto out; - } - - ret = 0; - if (msg_reply.msg.request.slave != ctx->msg.request.slave) { - VHOST_LOG_CONFIG(dev->ifname, ERR, - "received unexpected msg type (%u), expected %u\n", - msg_reply.msg.request.slave, ctx->msg.request.slave); - ret = -1; - goto out; - } - - ret = msg_reply.msg.payload.u64 ? -1 : 0; - -out: - rte_spinlock_unlock(&dev->slave_req_lock); - return ret; -} - int vhost_user_iotlb_miss(struct virtio_net *dev, uint64_t iova, uint8_t perm) { @@ -3267,10 +3259,9 @@ vhost_user_iotlb_miss(struct virtio_net *dev, uint64_t iova, uint8_t perm) return 0; } -static int -vhost_user_slave_config_change(struct virtio_net *dev, bool need_reply) +int +rte_vhost_slave_config_change(int vid, bool need_reply) { - int ret; struct vhu_msg_context ctx = { .msg = { .request.slave = VHOST_USER_SLAVE_CONFIG_CHANGE_MSG, @@ -3278,29 +3269,23 @@ vhost_user_slave_config_change(struct virtio_net *dev, bool need_reply) .size = 0, } }; - - if (need_reply) - ctx.msg.flags |= VHOST_USER_NEED_REPLY; - - ret = send_vhost_slave_message(dev, &ctx); - if (ret < 0) { - VHOST_LOG_CONFIG(dev->ifname, ERR, "failed to send config change (%d)\n", ret); - return ret; - } - - return process_slave_message_reply(dev, &ctx); -} - -int -rte_vhost_slave_config_change(int vid, bool need_reply) -{ struct virtio_net *dev; + int ret; dev = get_device(vid); if (!dev) return -ENODEV; - return vhost_user_slave_config_change(dev, need_reply); + if (!need_reply) { + ret = send_vhost_slave_message(dev, &ctx); + } else { + ctx.msg.flags |= VHOST_USER_NEED_REPLY; + ret = send_vhost_slave_message_process_reply(dev, &ctx); + } + + if (ret < 0) + VHOST_LOG_CONFIG(dev->ifname, ERR, "failed to send config change (%d)\n", ret); + return ret; } static int vhost_user_slave_set_vring_host_notifier(struct virtio_net *dev, @@ -3329,13 +3314,11 @@ static int vhost_user_slave_set_vring_host_notifier(struct virtio_net *dev, ctx.fd_num = 1; } - ret = send_vhost_slave_message(dev, &ctx); - if (ret < 0) { + ret = send_vhost_slave_message_process_reply(dev, &ctx); + if (ret < 0) VHOST_LOG_CONFIG(dev->ifname, ERR, "failed to set host notifier (%d)\n", ret); - return ret; - } - return process_slave_message_reply(dev, &ctx); + return ret; } int rte_vhost_host_notifier_ctrl(int vid, uint16_t qid, bool enable) From patchwork Thu Jan 19 18:46:14 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Marchand X-Patchwork-Id: 122369 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 944944241A; Thu, 19 Jan 2023 19:46:52 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 1D22142D65; Thu, 19 Jan 2023 19:46:44 +0100 (CET) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mails.dpdk.org (Postfix) with ESMTP id 0EFA0410EE for ; Thu, 19 Jan 2023 19:46:42 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1674154002; 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=qjmf8/SgLr6aiuJbT50SPdvOWRIBHbgNGxs78yuBinE=; b=Ji+auArfpkk9KhAR+3UlQ3rhcr1D+rjk/09FEFcAjCQsxpUmHRI+eFzEvCLiyPaHImpEHd yKsRrvxsx1f6+Tu3FYI6jNOG2HHd8zfOgX2J5gX6L/YIuEZMLGq0YrFtJpn8t6u4EEzh83 XQFNgOctPY62NnlKZ7yX2bMzwe5hsmM= 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-616-M9hnp6dQPDus5bBFf3kIeA-1; Thu, 19 Jan 2023 13:46:39 -0500 X-MC-Unique: M9hnp6dQPDus5bBFf3kIeA-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.rdu2.redhat.com [10.11.54.2]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 0BB3F29ABA19; Thu, 19 Jan 2023 18:46:39 +0000 (UTC) Received: from dmarchan.redhat.com (ovpn-193-92.brq.redhat.com [10.40.193.92]) by smtp.corp.redhat.com (Postfix) with ESMTP id 9329B40C6EC4; Thu, 19 Jan 2023 18:46:37 +0000 (UTC) From: David Marchand 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 Subject: [PATCH v4 3/9] vhost: terminate when access lock is not taken Date: Thu, 19 Jan 2023 19:46:14 +0100 Message-Id: <20230119184620.3195267-4-david.marchand@redhat.com> In-Reply-To: <20230119184620.3195267-1-david.marchand@redhat.com> References: <20220328121758.26632-1-david.marchand@redhat.com> <20230119184620.3195267-1-david.marchand@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.2 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Be a bit more strict when a programmatic error is detected wrt to the access_lock not being taken. Mark the new helper with __rte_assert_exclusive_lock so that clang understands where locks are expected to be taken. Signed-off-by: David Marchand Reviewed-by: Maxime Coquelin --- lib/vhost/vhost.c | 18 +++--------------- lib/vhost/vhost.h | 10 ++++++++++ lib/vhost/virtio_net.c | 6 +----- 3 files changed, 14 insertions(+), 20 deletions(-) diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c index 19c7b92c32..8cd727ca2f 100644 --- a/lib/vhost/vhost.c +++ b/lib/vhost/vhost.c @@ -1781,11 +1781,7 @@ rte_vhost_async_channel_register_thread_unsafe(int vid, uint16_t queue_id) if (unlikely(vq == NULL || !dev->async_copy)) return -1; - if (unlikely(!rte_spinlock_is_locked(&vq->access_lock))) { - VHOST_LOG_CONFIG(dev->ifname, ERR, "%s() called without access lock taken.\n", - __func__); - return -1; - } + vq_assert_lock(dev, vq); return async_channel_register(dev, vq); } @@ -1847,11 +1843,7 @@ rte_vhost_async_channel_unregister_thread_unsafe(int vid, uint16_t queue_id) if (vq == NULL) return -1; - if (unlikely(!rte_spinlock_is_locked(&vq->access_lock))) { - VHOST_LOG_CONFIG(dev->ifname, ERR, "%s() called without access lock taken.\n", - __func__); - return -1; - } + vq_assert_lock(dev, vq); if (!vq->async) return 0; @@ -1994,11 +1986,7 @@ rte_vhost_async_get_inflight_thread_unsafe(int vid, uint16_t queue_id) if (vq == NULL) return ret; - if (unlikely(!rte_spinlock_is_locked(&vq->access_lock))) { - VHOST_LOG_CONFIG(dev->ifname, ERR, "%s() called without access lock taken.\n", - __func__); - return -1; - } + vq_assert_lock(dev, vq); if (!vq->async) return ret; diff --git a/lib/vhost/vhost.h b/lib/vhost/vhost.h index ef211ed519..f6b2930efd 100644 --- a/lib/vhost/vhost.h +++ b/lib/vhost/vhost.h @@ -512,6 +512,16 @@ struct virtio_net { struct rte_vhost_user_extern_ops extern_ops; } __rte_cache_aligned; +static inline void +vq_assert_lock__(struct virtio_net *dev, struct vhost_virtqueue *vq, const char *func) + __rte_assert_exclusive_lock(&vq->access_lock) +{ + if (unlikely(!rte_spinlock_is_locked(&vq->access_lock))) + rte_panic("VHOST_CONFIG: (%s) %s() called without access lock taken.\n", + dev->ifname, func); +} +#define vq_assert_lock(dev, vq) vq_assert_lock__(dev, vq, __func__) + static __rte_always_inline bool vq_is_packed(struct virtio_net *dev) { diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c index 9abf752f30..2a75cda7b6 100644 --- a/lib/vhost/virtio_net.c +++ b/lib/vhost/virtio_net.c @@ -2185,11 +2185,7 @@ rte_vhost_clear_queue_thread_unsafe(int vid, uint16_t queue_id, vq = dev->virtqueue[queue_id]; - if (unlikely(!rte_spinlock_is_locked(&vq->access_lock))) { - VHOST_LOG_DATA(dev->ifname, ERR, "%s() called without access lock taken.\n", - __func__); - return -1; - } + vq_assert_lock(dev, vq); if (unlikely(!vq->async)) { VHOST_LOG_DATA(dev->ifname, ERR, From patchwork Thu Jan 19 18:46:15 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Marchand X-Patchwork-Id: 122370 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id CBAA84241A; Thu, 19 Jan 2023 19:46:58 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 31AA542D97; Thu, 19 Jan 2023 19:46:50 +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 D7F0242D87 for ; Thu, 19 Jan 2023 19:46:48 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1674154008; 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=42TuvWZmHlzbV9ZXXWwPKIwhfit1vPI3gBALnnMQrhc=; b=N9PKeD1YUwnT67z37ikr7FbGwXx8Hq1LWOXBfFyfh0eP/MHTpJLUhrhZaga5S37AZUvwy2 W6m4xzJ9cfYKIwUKip1KtM4Ktp0eP5TFfpSkNHmuVAutnUdt6DaIJLM63yFxsi2Shz1Xmg /nwAA0w3L+PFwwdJljWGEsK6+QyM4Vs= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-421-GORFXYyVNFql2kaALGN2Nw-1; Thu, 19 Jan 2023 13:46:43 -0500 X-MC-Unique: GORFXYyVNFql2kaALGN2Nw-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.rdu2.redhat.com [10.11.54.8]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 9EF8D802BF3; Thu, 19 Jan 2023 18:46:42 +0000 (UTC) Received: from dmarchan.redhat.com (ovpn-193-92.brq.redhat.com [10.40.193.92]) by smtp.corp.redhat.com (Postfix) with ESMTP id 337F1C15BAD; Thu, 19 Jan 2023 18:46:41 +0000 (UTC) From: David Marchand 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 Subject: [PATCH v4 4/9] vhost: annotate virtqueue access lock Date: Thu, 19 Jan 2023 19:46:15 +0100 Message-Id: <20230119184620.3195267-5-david.marchand@redhat.com> In-Reply-To: <20230119184620.3195267-1-david.marchand@redhat.com> References: <20220328121758.26632-1-david.marchand@redhat.com> <20230119184620.3195267-1-david.marchand@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.8 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org vhost_user_lock/unlock_all_queue_pairs must be waived since clang annotations can't express taking a runtime number of locks. vhost_queue_stats_update() requirement can be expressed with a required tag. Signed-off-by: David Marchand Reviewed-by: Maxime Coquelin --- Changes since RFC v3: - removed annotations needed for vhost async which went to the next patch, --- lib/vhost/vhost_user.c | 2 ++ lib/vhost/virtio_net.c | 4 +--- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c index 3f6c5df900..c57f092975 100644 --- a/lib/vhost/vhost_user.c +++ b/lib/vhost/vhost_user.c @@ -2955,6 +2955,7 @@ vhost_user_check_and_alloc_queue_pair(struct virtio_net *dev, static void vhost_user_lock_all_queue_pairs(struct virtio_net *dev) + __rte_no_thread_safety_analysis { unsigned int i = 0; unsigned int vq_num = 0; @@ -2972,6 +2973,7 @@ vhost_user_lock_all_queue_pairs(struct virtio_net *dev) static void vhost_user_unlock_all_queue_pairs(struct virtio_net *dev) + __rte_no_thread_safety_analysis { unsigned int i = 0; unsigned int vq_num = 0; diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c index 2a75cda7b6..f05e379316 100644 --- a/lib/vhost/virtio_net.c +++ b/lib/vhost/virtio_net.c @@ -52,12 +52,10 @@ is_valid_virt_queue_idx(uint32_t idx, int is_tx, uint32_t nr_vring) return (is_tx ^ (idx & 1)) == 0 && idx < nr_vring; } -/* - * This function must be called with virtqueue's access_lock taken. - */ static inline void vhost_queue_stats_update(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mbuf **pkts, uint16_t count) + __rte_exclusive_locks_required(&vq->access_lock) { struct virtqueue_stats *stats = &vq->stats; int i; From patchwork Thu Jan 19 18:46:16 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Marchand X-Patchwork-Id: 122371 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 6DFD44241A; Thu, 19 Jan 2023 19:47:04 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 2292F42DB3; Thu, 19 Jan 2023 19:46:51 +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 5C1E7410EE for ; Thu, 19 Jan 2023 19:46:50 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1674154009; 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=57mru6FlMzhDeyDftjQauM0d7Tjd/e54C9swcDY7LG8=; b=Q4cv/Hw1w9kSb2LHavqbeby6xPNiQEHFCFOw3cDq9RpxY2kkitDhmHOVBYxLpBMXw2IGZg RQKx3I6+EoRkpfatgMW2DDlPVb8LL/X20HGin/wioOBigEcJ/1LW8wfVeVAiiGJ0TGCm9O SfrjMlu5Wn7NnfVI5S3Qr50fVHGTbRY= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-339-rIzPmlacN_a0iayYIqdjCQ-1; Thu, 19 Jan 2023 13:46:46 -0500 X-MC-Unique: rIzPmlacN_a0iayYIqdjCQ-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.rdu2.redhat.com [10.11.54.2]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 4AC79857D0D; Thu, 19 Jan 2023 18:46:46 +0000 (UTC) Received: from dmarchan.redhat.com (ovpn-193-92.brq.redhat.com [10.40.193.92]) by smtp.corp.redhat.com (Postfix) with ESMTP id A1EDC40C6EC4; Thu, 19 Jan 2023 18:46:44 +0000 (UTC) From: David Marchand 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 Subject: [PATCH v4 5/9] vhost: annotate async accesses Date: Thu, 19 Jan 2023 19:46:16 +0100 Message-Id: <20230119184620.3195267-6-david.marchand@redhat.com> In-Reply-To: <20230119184620.3195267-1-david.marchand@redhat.com> References: <20220328121758.26632-1-david.marchand@redhat.com> <20230119184620.3195267-1-david.marchand@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.2 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org vq->async is initialised and must be accessed under vq->access_lock. Signed-off-by: David Marchand Reviewed-by: Maxime Coquelin --- Changes since RFC v3: - rebased, - fixed annotations vq->access_lock -> &vq->access_lock, - reworked free_vq, --- lib/vhost/vhost.c | 4 ++++ lib/vhost/vhost.h | 2 +- lib/vhost/vhost_user.c | 10 +++++++--- lib/vhost/virtio_net.c | 35 +++++++++++++++++++++++++++++++++++ 4 files changed, 47 insertions(+), 4 deletions(-) diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c index 8cd727ca2f..8bccdd8584 100644 --- a/lib/vhost/vhost.c +++ b/lib/vhost/vhost.c @@ -369,6 +369,7 @@ cleanup_device(struct virtio_net *dev, int destroy) static void vhost_free_async_mem(struct vhost_virtqueue *vq) + __rte_exclusive_locks_required(&vq->access_lock) { if (!vq->async) return; @@ -393,7 +394,9 @@ free_vq(struct virtio_net *dev, struct vhost_virtqueue *vq) else rte_free(vq->shadow_used_split); + rte_spinlock_lock(&vq->access_lock); vhost_free_async_mem(vq); + rte_spinlock_unlock(&vq->access_lock); rte_free(vq->batch_copy_elems); vhost_user_iotlb_destroy(vq); rte_free(vq->log_cache); @@ -1669,6 +1672,7 @@ rte_vhost_extern_callback_register(int vid, static __rte_always_inline int async_channel_register(struct virtio_net *dev, struct vhost_virtqueue *vq) + __rte_exclusive_locks_required(&vq->access_lock) { struct vhost_async *async; int node = vq->numa_node; diff --git a/lib/vhost/vhost.h b/lib/vhost/vhost.h index f6b2930efd..239ed02bd4 100644 --- a/lib/vhost/vhost.h +++ b/lib/vhost/vhost.h @@ -325,7 +325,7 @@ struct vhost_virtqueue { struct rte_vhost_resubmit_info *resubmit_inflight; uint64_t global_counter; - struct vhost_async *async; + struct vhost_async *async __rte_guarded_var; int notif_enable; #define VIRTIO_UNINITIALIZED_NOTIF (-1) diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c index c57f092975..75c7fc851e 100644 --- a/lib/vhost/vhost_user.c +++ b/lib/vhost/vhost_user.c @@ -2159,6 +2159,7 @@ vhost_user_set_vring_enable(struct virtio_net **pdev, int main_fd __rte_unused) { struct virtio_net *dev = *pdev; + struct vhost_virtqueue *vq; bool enable = !!ctx->msg.payload.state.num; int index = (int)ctx->msg.payload.state.index; @@ -2166,15 +2167,18 @@ vhost_user_set_vring_enable(struct virtio_net **pdev, "set queue enable: %d to qp idx: %d\n", enable, index); - if (enable && dev->virtqueue[index]->async) { - if (dev->virtqueue[index]->async->pkts_inflight_n) { + vq = dev->virtqueue[index]; + /* vhost_user_lock_all_queue_pairs locked all qps */ + vq_assert_lock(dev, vq); + if (enable && vq->async) { + if (vq->async->pkts_inflight_n) { VHOST_LOG_CONFIG(dev->ifname, ERR, "failed to enable vring. Inflight packets must be completed first\n"); return RTE_VHOST_MSG_RESULT_ERR; } } - dev->virtqueue[index]->enabled = enable; + vq->enabled = enable; return RTE_VHOST_MSG_RESULT_OK; } diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c index f05e379316..eedaf0fbf3 100644 --- a/lib/vhost/virtio_net.c +++ b/lib/vhost/virtio_net.c @@ -102,6 +102,7 @@ static __rte_always_inline int64_t vhost_async_dma_transfer_one(struct virtio_net *dev, struct vhost_virtqueue *vq, int16_t dma_id, uint16_t vchan_id, uint16_t flag_idx, struct vhost_iov_iter *pkt) + __rte_exclusive_locks_required(&vq->access_lock) { struct async_dma_vchan_info *dma_info = &dma_copy_track[dma_id].vchans[vchan_id]; uint16_t ring_mask = dma_info->ring_mask; @@ -151,6 +152,7 @@ static __rte_always_inline uint16_t vhost_async_dma_transfer(struct virtio_net *dev, struct vhost_virtqueue *vq, int16_t dma_id, uint16_t vchan_id, uint16_t head_idx, struct vhost_iov_iter *pkts, uint16_t nr_pkts) + __rte_exclusive_locks_required(&vq->access_lock) { struct async_dma_vchan_info *dma_info = &dma_copy_track[dma_id].vchans[vchan_id]; int64_t ret, nr_copies = 0; @@ -1063,6 +1065,7 @@ static __rte_always_inline int async_fill_seg(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mbuf *m, uint32_t mbuf_offset, uint64_t buf_iova, uint32_t cpy_len, bool to_desc) + __rte_exclusive_locks_required(&vq->access_lock) { struct vhost_async *async = vq->async; uint64_t mapped_len; @@ -1140,6 +1143,7 @@ static __rte_always_inline int mbuf_to_desc(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mbuf *m, struct buf_vector *buf_vec, uint16_t nr_vec, uint16_t num_buffers, bool is_async) + __rte_exclusive_locks_required(&vq->access_lock) { uint32_t vec_idx = 0; uint32_t mbuf_offset, mbuf_avail; @@ -1268,6 +1272,7 @@ vhost_enqueue_single_packed(struct virtio_net *dev, struct rte_mbuf *pkt, struct buf_vector *buf_vec, uint16_t *nr_descs) + __rte_exclusive_locks_required(&vq->access_lock) { uint16_t nr_vec = 0; uint16_t avail_idx = vq->last_avail_idx; @@ -1328,6 +1333,7 @@ vhost_enqueue_single_packed(struct virtio_net *dev, static __rte_noinline uint32_t virtio_dev_rx_split(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mbuf **pkts, uint32_t count) + __rte_exclusive_locks_required(&vq->access_lock) { uint32_t pkt_idx = 0; uint16_t num_buffers; @@ -1497,6 +1503,7 @@ static __rte_always_inline int16_t virtio_dev_rx_single_packed(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mbuf *pkt) + __rte_exclusive_locks_required(&vq->access_lock) { struct buf_vector buf_vec[BUF_VECTOR_MAX]; uint16_t nr_descs = 0; @@ -1521,6 +1528,7 @@ virtio_dev_rx_packed(struct virtio_net *dev, struct vhost_virtqueue *__rte_restrict vq, struct rte_mbuf **__rte_restrict pkts, uint32_t count) + __rte_exclusive_locks_required(&vq->access_lock) { uint32_t pkt_idx = 0; @@ -1620,6 +1628,7 @@ rte_vhost_enqueue_burst(int vid, uint16_t queue_id, static __rte_always_inline uint16_t async_get_first_inflight_pkt_idx(struct vhost_virtqueue *vq) + __rte_exclusive_locks_required(&vq->access_lock) { struct vhost_async *async = vq->async; @@ -1665,6 +1674,7 @@ store_dma_desc_info_packed(struct vring_used_elem_packed *s_ring, static __rte_noinline uint32_t virtio_dev_rx_async_submit_split(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mbuf **pkts, uint32_t count, int16_t dma_id, uint16_t vchan_id) + __rte_exclusive_locks_required(&vq->access_lock) { struct buf_vector buf_vec[BUF_VECTOR_MAX]; uint32_t pkt_idx = 0; @@ -1771,6 +1781,7 @@ vhost_enqueue_async_packed(struct virtio_net *dev, struct buf_vector *buf_vec, uint16_t *nr_descs, uint16_t *nr_buffers) + __rte_exclusive_locks_required(&vq->access_lock) { uint16_t nr_vec = 0; uint16_t avail_idx = vq->last_avail_idx; @@ -1828,6 +1839,7 @@ vhost_enqueue_async_packed(struct virtio_net *dev, static __rte_always_inline int16_t virtio_dev_rx_async_packed(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mbuf *pkt, uint16_t *nr_descs, uint16_t *nr_buffers) + __rte_exclusive_locks_required(&vq->access_lock) { struct buf_vector buf_vec[BUF_VECTOR_MAX]; @@ -1847,6 +1859,7 @@ virtio_dev_rx_async_packed(struct virtio_net *dev, struct vhost_virtqueue *vq, static __rte_always_inline void dma_error_handler_packed(struct vhost_virtqueue *vq, uint16_t slot_idx, uint32_t nr_err, uint32_t *pkt_idx) + __rte_exclusive_locks_required(&vq->access_lock) { uint16_t descs_err = 0; uint16_t buffers_err = 0; @@ -1873,6 +1886,7 @@ dma_error_handler_packed(struct vhost_virtqueue *vq, uint16_t slot_idx, static __rte_noinline uint32_t virtio_dev_rx_async_submit_packed(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mbuf **pkts, uint32_t count, int16_t dma_id, uint16_t vchan_id) + __rte_exclusive_locks_required(&vq->access_lock) { uint32_t pkt_idx = 0; uint16_t n_xfer; @@ -1942,6 +1956,7 @@ virtio_dev_rx_async_submit_packed(struct virtio_net *dev, struct vhost_virtqueue static __rte_always_inline void write_back_completed_descs_split(struct vhost_virtqueue *vq, uint16_t n_descs) + __rte_exclusive_locks_required(&vq->access_lock) { struct vhost_async *async = vq->async; uint16_t nr_left = n_descs; @@ -1974,6 +1989,7 @@ write_back_completed_descs_split(struct vhost_virtqueue *vq, uint16_t n_descs) static __rte_always_inline void write_back_completed_descs_packed(struct vhost_virtqueue *vq, uint16_t n_buffers) + __rte_exclusive_locks_required(&vq->access_lock) { struct vhost_async *async = vq->async; uint16_t from = async->last_buffer_idx_packed; @@ -2038,6 +2054,7 @@ write_back_completed_descs_packed(struct vhost_virtqueue *vq, static __rte_always_inline uint16_t vhost_poll_enqueue_completed(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mbuf **pkts, uint16_t count, int16_t dma_id, uint16_t vchan_id) + __rte_exclusive_locks_required(&vq->access_lock) { struct vhost_async *async = vq->async; struct async_inflight_info *pkts_info = async->pkts_info; @@ -2642,6 +2659,7 @@ desc_to_mbuf(struct virtio_net *dev, struct vhost_virtqueue *vq, struct buf_vector *buf_vec, uint16_t nr_vec, struct rte_mbuf *m, struct rte_mempool *mbuf_pool, bool legacy_ol_flags, uint16_t slot_idx, bool is_async) + __rte_exclusive_locks_required(&vq->access_lock) { uint32_t buf_avail, buf_offset, buf_len; uint64_t buf_addr, buf_iova; @@ -2847,6 +2865,7 @@ static uint16_t virtio_dev_tx_split(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mempool *mbuf_pool, struct rte_mbuf **pkts, uint16_t count, bool legacy_ol_flags) + __rte_exclusive_locks_required(&vq->access_lock) { uint16_t i; uint16_t avail_entries; @@ -2950,6 +2969,7 @@ static uint16_t virtio_dev_tx_split_legacy(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mempool *mbuf_pool, struct rte_mbuf **pkts, uint16_t count) + __rte_exclusive_locks_required(&vq->access_lock) { return virtio_dev_tx_split(dev, vq, mbuf_pool, pkts, count, true); } @@ -2959,6 +2979,7 @@ static uint16_t virtio_dev_tx_split_compliant(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mempool *mbuf_pool, struct rte_mbuf **pkts, uint16_t count) + __rte_exclusive_locks_required(&vq->access_lock) { return virtio_dev_tx_split(dev, vq, mbuf_pool, pkts, count, false); } @@ -3085,6 +3106,7 @@ vhost_dequeue_single_packed(struct virtio_net *dev, uint16_t *buf_id, uint16_t *desc_count, bool legacy_ol_flags) + __rte_exclusive_locks_required(&vq->access_lock) { struct buf_vector buf_vec[BUF_VECTOR_MAX]; uint32_t buf_len; @@ -3133,6 +3155,7 @@ virtio_dev_tx_single_packed(struct virtio_net *dev, struct rte_mempool *mbuf_pool, struct rte_mbuf *pkts, bool legacy_ol_flags) + __rte_exclusive_locks_required(&vq->access_lock) { uint16_t buf_id, desc_count = 0; @@ -3163,6 +3186,7 @@ virtio_dev_tx_packed(struct virtio_net *dev, struct rte_mbuf **__rte_restrict pkts, uint32_t count, bool legacy_ol_flags) + __rte_exclusive_locks_required(&vq->access_lock) { uint32_t pkt_idx = 0; @@ -3206,6 +3230,7 @@ static uint16_t virtio_dev_tx_packed_legacy(struct virtio_net *dev, struct vhost_virtqueue *__rte_restrict vq, struct rte_mempool *mbuf_pool, struct rte_mbuf **__rte_restrict pkts, uint32_t count) + __rte_exclusive_locks_required(&vq->access_lock) { return virtio_dev_tx_packed(dev, vq, mbuf_pool, pkts, count, true); } @@ -3215,6 +3240,7 @@ static uint16_t virtio_dev_tx_packed_compliant(struct virtio_net *dev, struct vhost_virtqueue *__rte_restrict vq, struct rte_mempool *mbuf_pool, struct rte_mbuf **__rte_restrict pkts, uint32_t count) + __rte_exclusive_locks_required(&vq->access_lock) { return virtio_dev_tx_packed(dev, vq, mbuf_pool, pkts, count, false); } @@ -3332,6 +3358,7 @@ static __rte_always_inline uint16_t async_poll_dequeue_completed(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mbuf **pkts, uint16_t count, int16_t dma_id, uint16_t vchan_id, bool legacy_ol_flags) + __rte_exclusive_locks_required(&vq->access_lock) { uint16_t start_idx, from, i; uint16_t nr_cpl_pkts = 0; @@ -3378,6 +3405,7 @@ static __rte_always_inline uint16_t virtio_dev_tx_async_split(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mempool *mbuf_pool, struct rte_mbuf **pkts, uint16_t count, int16_t dma_id, uint16_t vchan_id, bool legacy_ol_flags) + __rte_exclusive_locks_required(&vq->access_lock) { static bool allocerr_warned; bool dropped = false; @@ -3524,6 +3552,7 @@ virtio_dev_tx_async_split_legacy(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mempool *mbuf_pool, struct rte_mbuf **pkts, uint16_t count, int16_t dma_id, uint16_t vchan_id) + __rte_exclusive_locks_required(&vq->access_lock) { return virtio_dev_tx_async_split(dev, vq, mbuf_pool, pkts, count, dma_id, vchan_id, true); @@ -3535,6 +3564,7 @@ virtio_dev_tx_async_split_compliant(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mempool *mbuf_pool, struct rte_mbuf **pkts, uint16_t count, int16_t dma_id, uint16_t vchan_id) + __rte_exclusive_locks_required(&vq->access_lock) { return virtio_dev_tx_async_split(dev, vq, mbuf_pool, pkts, count, dma_id, vchan_id, false); @@ -3543,6 +3573,7 @@ virtio_dev_tx_async_split_compliant(struct virtio_net *dev, static __rte_always_inline void vhost_async_shadow_dequeue_single_packed(struct vhost_virtqueue *vq, uint16_t buf_id, uint16_t count) + __rte_exclusive_locks_required(&vq->access_lock) { struct vhost_async *async = vq->async; uint16_t idx = async->buffer_idx_packed; @@ -3564,6 +3595,7 @@ virtio_dev_tx_async_single_packed(struct virtio_net *dev, struct rte_mbuf *pkts, uint16_t slot_idx, bool legacy_ol_flags) + __rte_exclusive_locks_required(&vq->access_lock) { int err; uint16_t buf_id, desc_count = 0; @@ -3614,6 +3646,7 @@ static __rte_always_inline uint16_t virtio_dev_tx_async_packed(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mempool *mbuf_pool, struct rte_mbuf **pkts, uint16_t count, uint16_t dma_id, uint16_t vchan_id, bool legacy_ol_flags) + __rte_exclusive_locks_required(&vq->access_lock) { uint16_t pkt_idx; uint16_t slot_idx = 0; @@ -3707,6 +3740,7 @@ static uint16_t virtio_dev_tx_async_packed_legacy(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mempool *mbuf_pool, struct rte_mbuf **pkts, uint16_t count, uint16_t dma_id, uint16_t vchan_id) + __rte_exclusive_locks_required(&vq->access_lock) { return virtio_dev_tx_async_packed(dev, vq, mbuf_pool, pkts, count, dma_id, vchan_id, true); @@ -3717,6 +3751,7 @@ static uint16_t virtio_dev_tx_async_packed_compliant(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mempool *mbuf_pool, struct rte_mbuf **pkts, uint16_t count, uint16_t dma_id, uint16_t vchan_id) + __rte_exclusive_locks_required(&vq->access_lock) { return virtio_dev_tx_async_packed(dev, vq, mbuf_pool, pkts, count, dma_id, vchan_id, false); From patchwork Thu Jan 19 18:46:17 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Marchand X-Patchwork-Id: 122372 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 071004241A; Thu, 19 Jan 2023 19:47:12 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 41C4142DA9; Thu, 19 Jan 2023 19:46:55 +0100 (CET) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mails.dpdk.org (Postfix) with ESMTP id E400742D87 for ; Thu, 19 Jan 2023 19:46:53 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1674154013; 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=bGppIxGUF/TurwE2u9dr2KMyw7NZqKoKaJnwQK6PQEI=; b=Aqp5dqZIC2bjblZTFhFDW/EIOCuF2pE+6+gnW/mGBQsDlFfGzyCJi3BcrpgY9uFgGgaem9 KizhH3cqDdJ4XnWnqizu8embB/yqfbWaVG9pbYEmUkW5x2M5qCflMNi3pFjHz4RG6DQxh1 G4r1ej7eKVZDzM2oovSVK4fdPPUDedw= 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-624-urmeOC_EOiWunb3HMLuHEw-1; Thu, 19 Jan 2023 13:46:50 -0500 X-MC-Unique: urmeOC_EOiWunb3HMLuHEw-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id E80CA29ABA21; Thu, 19 Jan 2023 18:46:49 +0000 (UTC) Received: from dmarchan.redhat.com (ovpn-193-92.brq.redhat.com [10.40.193.92]) by smtp.corp.redhat.com (Postfix) with ESMTP id 735721121315; Thu, 19 Jan 2023 18:46:48 +0000 (UTC) From: David Marchand 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 Subject: [PATCH v4 6/9] vhost: always take IOTLB lock Date: Thu, 19 Jan 2023 19:46:17 +0100 Message-Id: <20230119184620.3195267-7-david.marchand@redhat.com> In-Reply-To: <20230119184620.3195267-1-david.marchand@redhat.com> References: <20220328121758.26632-1-david.marchand@redhat.com> <20230119184620.3195267-1-david.marchand@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.3 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org clang does not support conditionally held locks when statically analysing taken locks with thread safety checks. Always take iotlb locks regardless of VIRTIO_F_IOMMU_PLATFORM feature. Signed-off-by: David Marchand Reviewed-by: Maxime Coquelin --- lib/vhost/vhost.c | 8 +++----- lib/vhost/virtio_net.c | 24 ++++++++---------------- 2 files changed, 11 insertions(+), 21 deletions(-) diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c index 8bccdd8584..1e0c30791e 100644 --- a/lib/vhost/vhost.c +++ b/lib/vhost/vhost.c @@ -563,10 +563,9 @@ vring_translate(struct virtio_net *dev, struct vhost_virtqueue *vq) } void -vring_invalidate(struct virtio_net *dev, struct vhost_virtqueue *vq) +vring_invalidate(struct virtio_net *dev __rte_unused, struct vhost_virtqueue *vq) { - if (dev->features & (1ULL << VIRTIO_F_IOMMU_PLATFORM)) - vhost_user_iotlb_wr_lock(vq); + vhost_user_iotlb_wr_lock(vq); vq->access_ok = false; vq->desc = NULL; @@ -574,8 +573,7 @@ vring_invalidate(struct virtio_net *dev, struct vhost_virtqueue *vq) vq->used = NULL; vq->log_guest_addr = 0; - if (dev->features & (1ULL << VIRTIO_F_IOMMU_PLATFORM)) - vhost_user_iotlb_wr_unlock(vq); + vhost_user_iotlb_wr_unlock(vq); } static void diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c index eedaf0fbf3..ed92a855f8 100644 --- a/lib/vhost/virtio_net.c +++ b/lib/vhost/virtio_net.c @@ -1572,8 +1572,7 @@ virtio_dev_rx(struct virtio_net *dev, struct vhost_virtqueue *vq, if (unlikely(!vq->enabled)) goto out_access_unlock; - if (dev->features & (1ULL << VIRTIO_F_IOMMU_PLATFORM)) - vhost_user_iotlb_rd_lock(vq); + vhost_user_iotlb_rd_lock(vq); if (unlikely(!vq->access_ok)) if (unlikely(vring_translate(dev, vq) < 0)) @@ -1591,8 +1590,7 @@ virtio_dev_rx(struct virtio_net *dev, struct vhost_virtqueue *vq, vhost_queue_stats_update(dev, vq, pkts, nb_tx); out: - if (dev->features & (1ULL << VIRTIO_F_IOMMU_PLATFORM)) - vhost_user_iotlb_rd_unlock(vq); + vhost_user_iotlb_rd_unlock(vq); out_access_unlock: rte_spinlock_unlock(&vq->access_lock); @@ -2312,8 +2310,7 @@ virtio_dev_rx_async_submit(struct virtio_net *dev, struct vhost_virtqueue *vq, if (unlikely(!vq->enabled || !vq->async)) goto out_access_unlock; - if (dev->features & (1ULL << VIRTIO_F_IOMMU_PLATFORM)) - vhost_user_iotlb_rd_lock(vq); + vhost_user_iotlb_rd_lock(vq); if (unlikely(!vq->access_ok)) if (unlikely(vring_translate(dev, vq) < 0)) @@ -2333,8 +2330,7 @@ virtio_dev_rx_async_submit(struct virtio_net *dev, struct vhost_virtqueue *vq, vq->stats.inflight_submitted += nb_tx; out: - if (dev->features & (1ULL << VIRTIO_F_IOMMU_PLATFORM)) - vhost_user_iotlb_rd_unlock(vq); + vhost_user_iotlb_rd_unlock(vq); out_access_unlock: rte_spinlock_unlock(&vq->access_lock); @@ -3282,8 +3278,7 @@ rte_vhost_dequeue_burst(int vid, uint16_t queue_id, goto out_access_unlock; } - if (dev->features & (1ULL << VIRTIO_F_IOMMU_PLATFORM)) - vhost_user_iotlb_rd_lock(vq); + vhost_user_iotlb_rd_lock(vq); if (unlikely(!vq->access_ok)) if (unlikely(vring_translate(dev, vq) < 0)) { @@ -3342,8 +3337,7 @@ rte_vhost_dequeue_burst(int vid, uint16_t queue_id, vhost_queue_stats_update(dev, vq, pkts, count); out: - if (dev->features & (1ULL << VIRTIO_F_IOMMU_PLATFORM)) - vhost_user_iotlb_rd_unlock(vq); + vhost_user_iotlb_rd_unlock(vq); out_access_unlock: rte_spinlock_unlock(&vq->access_lock); @@ -3815,8 +3809,7 @@ rte_vhost_async_try_dequeue_burst(int vid, uint16_t queue_id, goto out_access_unlock; } - if (dev->features & (1ULL << VIRTIO_F_IOMMU_PLATFORM)) - vhost_user_iotlb_rd_lock(vq); + vhost_user_iotlb_rd_lock(vq); if (unlikely(vq->access_ok == 0)) if (unlikely(vring_translate(dev, vq) < 0)) { @@ -3880,8 +3873,7 @@ rte_vhost_async_try_dequeue_burst(int vid, uint16_t queue_id, vhost_queue_stats_update(dev, vq, pkts, count); out: - if (dev->features & (1ULL << VIRTIO_F_IOMMU_PLATFORM)) - vhost_user_iotlb_rd_unlock(vq); + vhost_user_iotlb_rd_unlock(vq); out_access_unlock: rte_spinlock_unlock(&vq->access_lock); From patchwork Thu Jan 19 18:46:18 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Marchand X-Patchwork-Id: 122373 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id D5D464241A; Thu, 19 Jan 2023 19:47:17 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 65DAD42DB7; Thu, 19 Jan 2023 19:47:01 +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 2CC5242DAF for ; Thu, 19 Jan 2023 19:46:59 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1674154018; 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=1AA/Ou/CmN7riopZGqjSkrV5jOUx9Hf65BtS57GnMk8=; b=XPfGp0xJJvWl6I3hzOq6Q4+QkjZXjLoKp7kcewYXJt75yDnxCwe+6RJJcsioP7wrO4DZj4 Iz2LALiQ7OfI55WonVIwGjWbblo4k4r4D5gifmnaXQCF7JfEULPneArWw0jvKJdJnSSUKl AG06gLvDWdgaAgiqyxVK/rfK8hbc7cQ= 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-539-YQYbBw4ION2a0QlSZMakDA-1; Thu, 19 Jan 2023 13:46:55 -0500 X-MC-Unique: YQYbBw4ION2a0QlSZMakDA-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.rdu2.redhat.com [10.11.54.8]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id EE8F329ABA29; Thu, 19 Jan 2023 18:46:53 +0000 (UTC) Received: from dmarchan.redhat.com (ovpn-193-92.brq.redhat.com [10.40.193.92]) by smtp.corp.redhat.com (Postfix) with ESMTP id EC8D5C15BAD; Thu, 19 Jan 2023 18:46:51 +0000 (UTC) From: David Marchand 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 Subject: [PATCH v4 7/9] vhost: annotate IOTLB lock Date: Thu, 19 Jan 2023 19:46:18 +0100 Message-Id: <20230119184620.3195267-8-david.marchand@redhat.com> In-Reply-To: <20230119184620.3195267-1-david.marchand@redhat.com> References: <20220328121758.26632-1-david.marchand@redhat.com> <20230119184620.3195267-1-david.marchand@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.8 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org The starting point for this is __vhost_iova_to_vva() which requires the lock to be taken. Annotate all the code leading to a call to it. vdpa and vhost_crypto code are annotated but they end up not taking a IOTLB lock and have been marked with a FIXME at the top level. Signed-off-by: David Marchand Reviewed-by: Maxime Coquelin --- Changes since RFC v3: - rebased, - moved unconditionnal lock acquire in separate patch, - fixed annotations, --- lib/vhost/iotlb.h | 4 ++++ lib/vhost/vdpa.c | 1 + lib/vhost/vhost.c | 8 +++----- lib/vhost/vhost.h | 22 ++++++++++++++++------ lib/vhost/vhost_crypto.c | 8 ++++++++ lib/vhost/virtio_net.c | 40 ++++++++++++++++++++++++++++++++++++++++ 6 files changed, 72 insertions(+), 11 deletions(-) diff --git a/lib/vhost/iotlb.h b/lib/vhost/iotlb.h index e27ebebcf5..be079a8aa7 100644 --- a/lib/vhost/iotlb.h +++ b/lib/vhost/iotlb.h @@ -11,24 +11,28 @@ static __rte_always_inline void vhost_user_iotlb_rd_lock(struct vhost_virtqueue *vq) + __rte_shared_lock_function(&vq->iotlb_lock) { rte_rwlock_read_lock(&vq->iotlb_lock); } static __rte_always_inline void vhost_user_iotlb_rd_unlock(struct vhost_virtqueue *vq) + __rte_unlock_function(&vq->iotlb_lock) { rte_rwlock_read_unlock(&vq->iotlb_lock); } static __rte_always_inline void vhost_user_iotlb_wr_lock(struct vhost_virtqueue *vq) + __rte_exclusive_lock_function(&vq->iotlb_lock) { rte_rwlock_write_lock(&vq->iotlb_lock); } static __rte_always_inline void vhost_user_iotlb_wr_unlock(struct vhost_virtqueue *vq) + __rte_unlock_function(&vq->iotlb_lock) { rte_rwlock_write_unlock(&vq->iotlb_lock); } diff --git a/lib/vhost/vdpa.c b/lib/vhost/vdpa.c index 577cb00a43..a430a66970 100644 --- a/lib/vhost/vdpa.c +++ b/lib/vhost/vdpa.c @@ -146,6 +146,7 @@ rte_vdpa_unregister_device(struct rte_vdpa_device *dev) int rte_vdpa_relay_vring_used(int vid, uint16_t qid, void *vring_m) + __rte_no_thread_safety_analysis /* FIXME: requires iotlb_lock? */ { struct virtio_net *dev = get_device(vid); uint16_t idx, idx_m, desc_id; diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c index 1e0c30791e..c36dc06a0a 100644 --- a/lib/vhost/vhost.c +++ b/lib/vhost/vhost.c @@ -52,7 +52,6 @@ static const struct vhost_vq_stats_name_off vhost_vq_stat_strings[] = { #define VHOST_NB_VQ_STATS RTE_DIM(vhost_vq_stat_strings) -/* Called with iotlb_lock read-locked */ uint64_t __vhost_iova_to_vva(struct virtio_net *dev, struct vhost_virtqueue *vq, uint64_t iova, uint64_t *size, uint8_t perm) @@ -419,6 +418,7 @@ free_device(struct virtio_net *dev) static __rte_always_inline int log_translate(struct virtio_net *dev, struct vhost_virtqueue *vq) + __rte_shared_locks_required(&vq->iotlb_lock) { if (likely(!(vq->ring_addrs.flags & (1 << VHOST_VRING_F_LOG)))) return 0; @@ -435,8 +435,6 @@ log_translate(struct virtio_net *dev, struct vhost_virtqueue *vq) * Converts vring log address to GPA * If IOMMU is enabled, the log address is IOVA * If IOMMU not enabled, the log address is already GPA - * - * Caller should have iotlb_lock read-locked */ uint64_t translate_log_addr(struct virtio_net *dev, struct vhost_virtqueue *vq, @@ -467,9 +465,9 @@ translate_log_addr(struct virtio_net *dev, struct vhost_virtqueue *vq, return log_addr; } -/* Caller should have iotlb_lock read-locked */ static int vring_translate_split(struct virtio_net *dev, struct vhost_virtqueue *vq) + __rte_shared_locks_required(&vq->iotlb_lock) { uint64_t req_size, size; @@ -506,9 +504,9 @@ vring_translate_split(struct virtio_net *dev, struct vhost_virtqueue *vq) return 0; } -/* Caller should have iotlb_lock read-locked */ static int vring_translate_packed(struct virtio_net *dev, struct vhost_virtqueue *vq) + __rte_shared_locks_required(&vq->iotlb_lock) { uint64_t req_size, size; diff --git a/lib/vhost/vhost.h b/lib/vhost/vhost.h index 239ed02bd4..46aacd79c0 100644 --- a/lib/vhost/vhost.h +++ b/lib/vhost/vhost.h @@ -562,12 +562,15 @@ void __vhost_log_cache_write(struct virtio_net *dev, uint64_t addr, uint64_t len); void __vhost_log_cache_write_iova(struct virtio_net *dev, struct vhost_virtqueue *vq, - uint64_t iova, uint64_t len); + uint64_t iova, uint64_t len) + __rte_shared_locks_required(&vq->iotlb_lock); void __vhost_log_cache_sync(struct virtio_net *dev, struct vhost_virtqueue *vq); + void __vhost_log_write(struct virtio_net *dev, uint64_t addr, uint64_t len); void __vhost_log_write_iova(struct virtio_net *dev, struct vhost_virtqueue *vq, - uint64_t iova, uint64_t len); + uint64_t iova, uint64_t len) + __rte_shared_locks_required(&vq->iotlb_lock); static __rte_always_inline void vhost_log_write(struct virtio_net *dev, uint64_t addr, uint64_t len) @@ -617,6 +620,7 @@ vhost_log_used_vring(struct virtio_net *dev, struct vhost_virtqueue *vq, static __rte_always_inline void vhost_log_cache_write_iova(struct virtio_net *dev, struct vhost_virtqueue *vq, uint64_t iova, uint64_t len) + __rte_shared_locks_required(&vq->iotlb_lock) { if (likely(!(dev->features & (1ULL << VHOST_F_LOG_ALL)))) return; @@ -630,6 +634,7 @@ vhost_log_cache_write_iova(struct virtio_net *dev, struct vhost_virtqueue *vq, static __rte_always_inline void vhost_log_write_iova(struct virtio_net *dev, struct vhost_virtqueue *vq, uint64_t iova, uint64_t len) + __rte_shared_locks_required(&vq->iotlb_lock) { if (likely(!(dev->features & (1ULL << VHOST_F_LOG_ALL)))) return; @@ -833,18 +838,23 @@ struct rte_vhost_device_ops const *vhost_driver_callback_get(const char *path); void vhost_backend_cleanup(struct virtio_net *dev); uint64_t __vhost_iova_to_vva(struct virtio_net *dev, struct vhost_virtqueue *vq, - uint64_t iova, uint64_t *len, uint8_t perm); + uint64_t iova, uint64_t *len, uint8_t perm) + __rte_shared_locks_required(&vq->iotlb_lock); void *vhost_alloc_copy_ind_table(struct virtio_net *dev, struct vhost_virtqueue *vq, - uint64_t desc_addr, uint64_t desc_len); -int vring_translate(struct virtio_net *dev, struct vhost_virtqueue *vq); + uint64_t desc_addr, uint64_t desc_len) + __rte_shared_locks_required(&vq->iotlb_lock); +int vring_translate(struct virtio_net *dev, struct vhost_virtqueue *vq) + __rte_shared_locks_required(&vq->iotlb_lock); uint64_t translate_log_addr(struct virtio_net *dev, struct vhost_virtqueue *vq, - uint64_t log_addr); + uint64_t log_addr) + __rte_shared_locks_required(&vq->iotlb_lock); void vring_invalidate(struct virtio_net *dev, struct vhost_virtqueue *vq); static __rte_always_inline uint64_t vhost_iova_to_vva(struct virtio_net *dev, struct vhost_virtqueue *vq, uint64_t iova, uint64_t *len, uint8_t perm) + __rte_shared_locks_required(&vq->iotlb_lock) { if (!(dev->features & (1ULL << VIRTIO_F_IOMMU_PLATFORM))) return rte_vhost_va_from_guest_pa(dev->mem, iova, len); diff --git a/lib/vhost/vhost_crypto.c b/lib/vhost/vhost_crypto.c index b448b6685d..f02bf865c3 100644 --- a/lib/vhost/vhost_crypto.c +++ b/lib/vhost/vhost_crypto.c @@ -490,6 +490,7 @@ static __rte_always_inline struct virtio_crypto_inhdr * reach_inhdr(struct vhost_crypto_data_req *vc_req, struct vhost_crypto_desc *head, uint32_t max_n_descs) + __rte_shared_locks_required(&vc_req->vq->iotlb_lock) { struct virtio_crypto_inhdr *inhdr; struct vhost_crypto_desc *last = head + (max_n_descs - 1); @@ -536,6 +537,7 @@ static __rte_always_inline void * get_data_ptr(struct vhost_crypto_data_req *vc_req, struct vhost_crypto_desc *cur_desc, uint8_t perm) + __rte_shared_locks_required(&vc_req->vq->iotlb_lock) { void *data; uint64_t dlen = cur_desc->len; @@ -552,6 +554,7 @@ get_data_ptr(struct vhost_crypto_data_req *vc_req, static __rte_always_inline uint32_t copy_data_from_desc(void *dst, struct vhost_crypto_data_req *vc_req, struct vhost_crypto_desc *desc, uint32_t size) + __rte_shared_locks_required(&vc_req->vq->iotlb_lock) { uint64_t remain; uint64_t addr; @@ -582,6 +585,7 @@ static __rte_always_inline int copy_data(void *data, struct vhost_crypto_data_req *vc_req, struct vhost_crypto_desc *head, struct vhost_crypto_desc **cur_desc, uint32_t size, uint32_t max_n_descs) + __rte_shared_locks_required(&vc_req->vq->iotlb_lock) { struct vhost_crypto_desc *desc = *cur_desc; uint32_t left = size; @@ -665,6 +669,7 @@ prepare_write_back_data(struct vhost_crypto_data_req *vc_req, uint32_t offset, uint64_t write_back_len, uint32_t max_n_descs) + __rte_shared_locks_required(&vc_req->vq->iotlb_lock) { struct vhost_crypto_writeback_data *wb_data, *head; struct vhost_crypto_desc *desc = *cur_desc; @@ -785,6 +790,7 @@ prepare_sym_cipher_op(struct vhost_crypto *vcrypto, struct rte_crypto_op *op, struct virtio_crypto_cipher_data_req *cipher, struct vhost_crypto_desc *head, uint32_t max_n_descs) + __rte_shared_locks_required(&vc_req->vq->iotlb_lock) { struct vhost_crypto_desc *desc = head; struct vhost_crypto_writeback_data *ewb = NULL; @@ -938,6 +944,7 @@ prepare_sym_chain_op(struct vhost_crypto *vcrypto, struct rte_crypto_op *op, struct virtio_crypto_alg_chain_data_req *chain, struct vhost_crypto_desc *head, uint32_t max_n_descs) + __rte_shared_locks_required(&vc_req->vq->iotlb_lock) { struct vhost_crypto_desc *desc = head, *digest_desc; struct vhost_crypto_writeback_data *ewb = NULL, *ewb2 = NULL; @@ -1123,6 +1130,7 @@ vhost_crypto_process_one_req(struct vhost_crypto *vcrypto, struct vhost_virtqueue *vq, struct rte_crypto_op *op, struct vring_desc *head, struct vhost_crypto_desc *descs, uint16_t desc_idx) + __rte_no_thread_safety_analysis /* FIXME: requires iotlb_lock? */ { struct vhost_crypto_data_req *vc_req = rte_mbuf_to_priv(op->sym->m_src); struct rte_cryptodev_sym_session *session; diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c index ed92a855f8..9853adee1a 100644 --- a/lib/vhost/virtio_net.c +++ b/lib/vhost/virtio_net.c @@ -233,6 +233,7 @@ vhost_async_dma_check_completed(struct virtio_net *dev, int16_t dma_id, uint16_t static inline void do_data_copy_enqueue(struct virtio_net *dev, struct vhost_virtqueue *vq) + __rte_shared_locks_required(&vq->iotlb_lock) { struct batch_copy_elem *elem = vq->batch_copy_elems; uint16_t count = vq->batch_copy_nb_elems; @@ -579,6 +580,7 @@ vhost_shadow_enqueue_single_packed(struct virtio_net *dev, uint16_t *id, uint16_t *count, uint16_t num_buffers) + __rte_shared_locks_required(&vq->iotlb_lock) { vhost_shadow_enqueue_packed(vq, len, id, count, num_buffers); @@ -670,6 +672,7 @@ static __rte_always_inline int map_one_desc(struct virtio_net *dev, struct vhost_virtqueue *vq, struct buf_vector *buf_vec, uint16_t *vec_idx, uint64_t desc_iova, uint64_t desc_len, uint8_t perm) + __rte_shared_locks_required(&vq->iotlb_lock) { uint16_t vec_id = *vec_idx; @@ -707,6 +710,7 @@ fill_vec_buf_split(struct virtio_net *dev, struct vhost_virtqueue *vq, uint32_t avail_idx, uint16_t *vec_idx, struct buf_vector *buf_vec, uint16_t *desc_chain_head, uint32_t *desc_chain_len, uint8_t perm) + __rte_shared_locks_required(&vq->iotlb_lock) { uint16_t idx = vq->avail->ring[avail_idx & (vq->size - 1)]; uint16_t vec_id = *vec_idx; @@ -790,6 +794,7 @@ reserve_avail_buf_split(struct virtio_net *dev, struct vhost_virtqueue *vq, uint64_t size, struct buf_vector *buf_vec, uint16_t *num_buffers, uint16_t avail_head, uint16_t *nr_vec) + __rte_shared_locks_required(&vq->iotlb_lock) { uint16_t cur_idx; uint16_t vec_idx = 0; @@ -840,6 +845,7 @@ fill_vec_buf_packed_indirect(struct virtio_net *dev, struct vhost_virtqueue *vq, struct vring_packed_desc *desc, uint16_t *vec_idx, struct buf_vector *buf_vec, uint32_t *len, uint8_t perm) + __rte_shared_locks_required(&vq->iotlb_lock) { uint16_t i; uint32_t nr_descs; @@ -898,6 +904,7 @@ fill_vec_buf_packed(struct virtio_net *dev, struct vhost_virtqueue *vq, uint16_t avail_idx, uint16_t *desc_count, struct buf_vector *buf_vec, uint16_t *vec_idx, uint16_t *buf_id, uint32_t *len, uint8_t perm) + __rte_shared_locks_required(&vq->iotlb_lock) { bool wrap_counter = vq->avail_wrap_counter; struct vring_packed_desc *descs = vq->desc_packed; @@ -963,6 +970,7 @@ static __rte_noinline void copy_vnet_hdr_to_desc(struct virtio_net *dev, struct vhost_virtqueue *vq, struct buf_vector *buf_vec, struct virtio_net_hdr_mrg_rxbuf *hdr) + __rte_shared_locks_required(&vq->iotlb_lock) { uint64_t len; uint64_t remain = dev->vhost_hlen; @@ -1066,6 +1074,7 @@ async_fill_seg(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mbuf *m, uint32_t mbuf_offset, uint64_t buf_iova, uint32_t cpy_len, bool to_desc) __rte_exclusive_locks_required(&vq->access_lock) + __rte_shared_locks_required(&vq->iotlb_lock) { struct vhost_async *async = vq->async; uint64_t mapped_len; @@ -1106,6 +1115,7 @@ static __rte_always_inline void sync_fill_seg(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mbuf *m, uint32_t mbuf_offset, uint64_t buf_addr, uint64_t buf_iova, uint32_t cpy_len, bool to_desc) + __rte_shared_locks_required(&vq->iotlb_lock) { struct batch_copy_elem *batch_copy = vq->batch_copy_elems; @@ -1144,6 +1154,7 @@ mbuf_to_desc(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mbuf *m, struct buf_vector *buf_vec, uint16_t nr_vec, uint16_t num_buffers, bool is_async) __rte_exclusive_locks_required(&vq->access_lock) + __rte_shared_locks_required(&vq->iotlb_lock) { uint32_t vec_idx = 0; uint32_t mbuf_offset, mbuf_avail; @@ -1273,6 +1284,7 @@ vhost_enqueue_single_packed(struct virtio_net *dev, struct buf_vector *buf_vec, uint16_t *nr_descs) __rte_exclusive_locks_required(&vq->access_lock) + __rte_shared_locks_required(&vq->iotlb_lock) { uint16_t nr_vec = 0; uint16_t avail_idx = vq->last_avail_idx; @@ -1334,6 +1346,7 @@ static __rte_noinline uint32_t virtio_dev_rx_split(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mbuf **pkts, uint32_t count) __rte_exclusive_locks_required(&vq->access_lock) + __rte_shared_locks_required(&vq->iotlb_lock) { uint32_t pkt_idx = 0; uint16_t num_buffers; @@ -1390,6 +1403,7 @@ virtio_dev_rx_sync_batch_check(struct virtio_net *dev, struct rte_mbuf **pkts, uint64_t *desc_addrs, uint64_t *lens) + __rte_shared_locks_required(&vq->iotlb_lock) { bool wrap_counter = vq->avail_wrap_counter; struct vring_packed_desc *descs = vq->desc_packed; @@ -1441,6 +1455,7 @@ virtio_dev_rx_batch_packed_copy(struct virtio_net *dev, struct rte_mbuf **pkts, uint64_t *desc_addrs, uint64_t *lens) + __rte_shared_locks_required(&vq->iotlb_lock) { uint32_t buf_offset = sizeof(struct virtio_net_hdr_mrg_rxbuf); struct virtio_net_hdr_mrg_rxbuf *hdrs[PACKED_BATCH_SIZE]; @@ -1482,6 +1497,7 @@ static __rte_always_inline int virtio_dev_rx_sync_batch_packed(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mbuf **pkts) + __rte_shared_locks_required(&vq->iotlb_lock) { uint64_t desc_addrs[PACKED_BATCH_SIZE]; uint64_t lens[PACKED_BATCH_SIZE]; @@ -1504,6 +1520,7 @@ virtio_dev_rx_single_packed(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mbuf *pkt) __rte_exclusive_locks_required(&vq->access_lock) + __rte_shared_locks_required(&vq->iotlb_lock) { struct buf_vector buf_vec[BUF_VECTOR_MAX]; uint16_t nr_descs = 0; @@ -1529,6 +1546,7 @@ virtio_dev_rx_packed(struct virtio_net *dev, struct rte_mbuf **__rte_restrict pkts, uint32_t count) __rte_exclusive_locks_required(&vq->access_lock) + __rte_shared_locks_required(&vq->iotlb_lock) { uint32_t pkt_idx = 0; @@ -1673,6 +1691,7 @@ static __rte_noinline uint32_t virtio_dev_rx_async_submit_split(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mbuf **pkts, uint32_t count, int16_t dma_id, uint16_t vchan_id) __rte_exclusive_locks_required(&vq->access_lock) + __rte_shared_locks_required(&vq->iotlb_lock) { struct buf_vector buf_vec[BUF_VECTOR_MAX]; uint32_t pkt_idx = 0; @@ -1780,6 +1799,7 @@ vhost_enqueue_async_packed(struct virtio_net *dev, uint16_t *nr_descs, uint16_t *nr_buffers) __rte_exclusive_locks_required(&vq->access_lock) + __rte_shared_locks_required(&vq->iotlb_lock) { uint16_t nr_vec = 0; uint16_t avail_idx = vq->last_avail_idx; @@ -1838,6 +1858,7 @@ static __rte_always_inline int16_t virtio_dev_rx_async_packed(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mbuf *pkt, uint16_t *nr_descs, uint16_t *nr_buffers) __rte_exclusive_locks_required(&vq->access_lock) + __rte_shared_locks_required(&vq->iotlb_lock) { struct buf_vector buf_vec[BUF_VECTOR_MAX]; @@ -1885,6 +1906,7 @@ static __rte_noinline uint32_t virtio_dev_rx_async_submit_packed(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mbuf **pkts, uint32_t count, int16_t dma_id, uint16_t vchan_id) __rte_exclusive_locks_required(&vq->access_lock) + __rte_shared_locks_required(&vq->iotlb_lock) { uint32_t pkt_idx = 0; uint16_t n_xfer; @@ -2656,6 +2678,7 @@ desc_to_mbuf(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mbuf *m, struct rte_mempool *mbuf_pool, bool legacy_ol_flags, uint16_t slot_idx, bool is_async) __rte_exclusive_locks_required(&vq->access_lock) + __rte_shared_locks_required(&vq->iotlb_lock) { uint32_t buf_avail, buf_offset, buf_len; uint64_t buf_addr, buf_iova; @@ -2862,6 +2885,7 @@ virtio_dev_tx_split(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mempool *mbuf_pool, struct rte_mbuf **pkts, uint16_t count, bool legacy_ol_flags) __rte_exclusive_locks_required(&vq->access_lock) + __rte_shared_locks_required(&vq->iotlb_lock) { uint16_t i; uint16_t avail_entries; @@ -2966,6 +2990,7 @@ virtio_dev_tx_split_legacy(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mempool *mbuf_pool, struct rte_mbuf **pkts, uint16_t count) __rte_exclusive_locks_required(&vq->access_lock) + __rte_shared_locks_required(&vq->iotlb_lock) { return virtio_dev_tx_split(dev, vq, mbuf_pool, pkts, count, true); } @@ -2976,6 +3001,7 @@ virtio_dev_tx_split_compliant(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mempool *mbuf_pool, struct rte_mbuf **pkts, uint16_t count) __rte_exclusive_locks_required(&vq->access_lock) + __rte_shared_locks_required(&vq->iotlb_lock) { return virtio_dev_tx_split(dev, vq, mbuf_pool, pkts, count, false); } @@ -2987,6 +3013,7 @@ vhost_reserve_avail_batch_packed(struct virtio_net *dev, uint16_t avail_idx, uintptr_t *desc_addrs, uint16_t *ids) + __rte_shared_locks_required(&vq->iotlb_lock) { bool wrap = vq->avail_wrap_counter; struct vring_packed_desc *descs = vq->desc_packed; @@ -3056,6 +3083,7 @@ virtio_dev_tx_batch_packed(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mbuf **pkts, bool legacy_ol_flags) + __rte_shared_locks_required(&vq->iotlb_lock) { uint16_t avail_idx = vq->last_avail_idx; uint32_t buf_offset = sizeof(struct virtio_net_hdr_mrg_rxbuf); @@ -3103,6 +3131,7 @@ vhost_dequeue_single_packed(struct virtio_net *dev, uint16_t *desc_count, bool legacy_ol_flags) __rte_exclusive_locks_required(&vq->access_lock) + __rte_shared_locks_required(&vq->iotlb_lock) { struct buf_vector buf_vec[BUF_VECTOR_MAX]; uint32_t buf_len; @@ -3152,6 +3181,7 @@ virtio_dev_tx_single_packed(struct virtio_net *dev, struct rte_mbuf *pkts, bool legacy_ol_flags) __rte_exclusive_locks_required(&vq->access_lock) + __rte_shared_locks_required(&vq->iotlb_lock) { uint16_t buf_id, desc_count = 0; @@ -3183,6 +3213,7 @@ virtio_dev_tx_packed(struct virtio_net *dev, uint32_t count, bool legacy_ol_flags) __rte_exclusive_locks_required(&vq->access_lock) + __rte_shared_locks_required(&vq->iotlb_lock) { uint32_t pkt_idx = 0; @@ -3227,6 +3258,7 @@ virtio_dev_tx_packed_legacy(struct virtio_net *dev, struct vhost_virtqueue *__rte_restrict vq, struct rte_mempool *mbuf_pool, struct rte_mbuf **__rte_restrict pkts, uint32_t count) __rte_exclusive_locks_required(&vq->access_lock) + __rte_shared_locks_required(&vq->iotlb_lock) { return virtio_dev_tx_packed(dev, vq, mbuf_pool, pkts, count, true); } @@ -3237,6 +3269,7 @@ virtio_dev_tx_packed_compliant(struct virtio_net *dev, struct vhost_virtqueue *__rte_restrict vq, struct rte_mempool *mbuf_pool, struct rte_mbuf **__rte_restrict pkts, uint32_t count) __rte_exclusive_locks_required(&vq->access_lock) + __rte_shared_locks_required(&vq->iotlb_lock) { return virtio_dev_tx_packed(dev, vq, mbuf_pool, pkts, count, false); } @@ -3400,6 +3433,7 @@ virtio_dev_tx_async_split(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mempool *mbuf_pool, struct rte_mbuf **pkts, uint16_t count, int16_t dma_id, uint16_t vchan_id, bool legacy_ol_flags) __rte_exclusive_locks_required(&vq->access_lock) + __rte_shared_locks_required(&vq->iotlb_lock) { static bool allocerr_warned; bool dropped = false; @@ -3547,6 +3581,7 @@ virtio_dev_tx_async_split_legacy(struct virtio_net *dev, struct rte_mbuf **pkts, uint16_t count, int16_t dma_id, uint16_t vchan_id) __rte_exclusive_locks_required(&vq->access_lock) + __rte_shared_locks_required(&vq->iotlb_lock) { return virtio_dev_tx_async_split(dev, vq, mbuf_pool, pkts, count, dma_id, vchan_id, true); @@ -3559,6 +3594,7 @@ virtio_dev_tx_async_split_compliant(struct virtio_net *dev, struct rte_mbuf **pkts, uint16_t count, int16_t dma_id, uint16_t vchan_id) __rte_exclusive_locks_required(&vq->access_lock) + __rte_shared_locks_required(&vq->iotlb_lock) { return virtio_dev_tx_async_split(dev, vq, mbuf_pool, pkts, count, dma_id, vchan_id, false); @@ -3590,6 +3626,7 @@ virtio_dev_tx_async_single_packed(struct virtio_net *dev, uint16_t slot_idx, bool legacy_ol_flags) __rte_exclusive_locks_required(&vq->access_lock) + __rte_shared_locks_required(&vq->iotlb_lock) { int err; uint16_t buf_id, desc_count = 0; @@ -3641,6 +3678,7 @@ virtio_dev_tx_async_packed(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mempool *mbuf_pool, struct rte_mbuf **pkts, uint16_t count, uint16_t dma_id, uint16_t vchan_id, bool legacy_ol_flags) __rte_exclusive_locks_required(&vq->access_lock) + __rte_shared_locks_required(&vq->iotlb_lock) { uint16_t pkt_idx; uint16_t slot_idx = 0; @@ -3735,6 +3773,7 @@ virtio_dev_tx_async_packed_legacy(struct virtio_net *dev, struct vhost_virtqueue struct rte_mempool *mbuf_pool, struct rte_mbuf **pkts, uint16_t count, uint16_t dma_id, uint16_t vchan_id) __rte_exclusive_locks_required(&vq->access_lock) + __rte_shared_locks_required(&vq->iotlb_lock) { return virtio_dev_tx_async_packed(dev, vq, mbuf_pool, pkts, count, dma_id, vchan_id, true); @@ -3746,6 +3785,7 @@ virtio_dev_tx_async_packed_compliant(struct virtio_net *dev, struct vhost_virtqu struct rte_mempool *mbuf_pool, struct rte_mbuf **pkts, uint16_t count, uint16_t dma_id, uint16_t vchan_id) __rte_exclusive_locks_required(&vq->access_lock) + __rte_shared_locks_required(&vq->iotlb_lock) { return virtio_dev_tx_async_packed(dev, vq, mbuf_pool, pkts, count, dma_id, vchan_id, false); From patchwork Thu Jan 19 18:46:19 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Marchand X-Patchwork-Id: 122374 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id B1AEE4241A; Thu, 19 Jan 2023 19:47:23 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 46D3A42DC0; Thu, 19 Jan 2023 19:47:02 +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 BFE5A42D3D for ; Thu, 19 Jan 2023 19:46:59 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1674154019; 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=UmfZOs7C0lJZnMVT+jstMq1LdcOPVnuu/MY3N/XBP+o=; b=IvshlgMyeH9NDAmjSdFlusxqRq1ChYSNJyR4WBD54IFBt50OeNTYNBlFih1921qY7XP0Ek kUdDWB72N5G88TssphxL6/HXuPPLYCHVHhYoVzebBXY74JH6cg68rd62T2KfXl/RD2jkNr Y3/WNBPJccczonblk9h6c+EhRawhMVY= 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-39-TDiKf03mN2iiswbpX1iUWw-1; Thu, 19 Jan 2023 13:46:58 -0500 X-MC-Unique: TDiKf03mN2iiswbpX1iUWw-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 93FAF3806103; Thu, 19 Jan 2023 18:46:57 +0000 (UTC) Received: from dmarchan.redhat.com (ovpn-193-92.brq.redhat.com [10.40.193.92]) by smtp.corp.redhat.com (Postfix) with ESMTP id 200B451EF; Thu, 19 Jan 2023 18:46:55 +0000 (UTC) From: David Marchand 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 Subject: [PATCH v4 8/9] vhost: annotate vDPA device list accesses Date: Thu, 19 Jan 2023 19:46:19 +0100 Message-Id: <20230119184620.3195267-9-david.marchand@redhat.com> In-Reply-To: <20230119184620.3195267-1-david.marchand@redhat.com> References: <20220328121758.26632-1-david.marchand@redhat.com> <20230119184620.3195267-1-david.marchand@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.5 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Access to vdpa_device_list must be protected with vdpa_device_list_lock spinlock. Signed-off-by: David Marchand Reviewed-by: Maxime Coquelin --- Changes since RFC v3: - rebased, --- lib/vhost/vdpa.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/lib/vhost/vdpa.c b/lib/vhost/vdpa.c index a430a66970..6284ea2ed1 100644 --- a/lib/vhost/vdpa.c +++ b/lib/vhost/vdpa.c @@ -23,21 +23,22 @@ /** Double linked list of vDPA devices. */ TAILQ_HEAD(vdpa_device_list, rte_vdpa_device); -static struct vdpa_device_list vdpa_device_list = - TAILQ_HEAD_INITIALIZER(vdpa_device_list); +static struct vdpa_device_list vdpa_device_list__ = + TAILQ_HEAD_INITIALIZER(vdpa_device_list__); static rte_spinlock_t vdpa_device_list_lock = RTE_SPINLOCK_INITIALIZER; +static struct vdpa_device_list * const vdpa_device_list + __rte_guarded_by(&vdpa_device_list_lock) = &vdpa_device_list__; - -/* Unsafe, needs to be called with vdpa_device_list_lock held */ static struct rte_vdpa_device * __vdpa_find_device_by_name(const char *name) + __rte_exclusive_locks_required(&vdpa_device_list_lock) { struct rte_vdpa_device *dev, *ret = NULL; if (name == NULL) return NULL; - TAILQ_FOREACH(dev, &vdpa_device_list, next) { + TAILQ_FOREACH(dev, vdpa_device_list, next) { if (!strncmp(dev->device->name, name, RTE_DEV_NAME_MAX_LEN)) { ret = dev; break; @@ -116,7 +117,7 @@ rte_vdpa_register_device(struct rte_device *rte_dev, dev->type = RTE_VHOST_VDPA_DEVICE_TYPE_NET; } - TAILQ_INSERT_TAIL(&vdpa_device_list, dev, next); + TAILQ_INSERT_TAIL(vdpa_device_list, dev, next); out_unlock: rte_spinlock_unlock(&vdpa_device_list_lock); @@ -130,11 +131,11 @@ rte_vdpa_unregister_device(struct rte_vdpa_device *dev) int ret = -1; rte_spinlock_lock(&vdpa_device_list_lock); - RTE_TAILQ_FOREACH_SAFE(cur_dev, &vdpa_device_list, next, tmp_dev) { + RTE_TAILQ_FOREACH_SAFE(cur_dev, vdpa_device_list, next, tmp_dev) { if (dev != cur_dev) continue; - TAILQ_REMOVE(&vdpa_device_list, dev, next); + TAILQ_REMOVE(vdpa_device_list, dev, next); rte_free(dev); ret = 0; break; @@ -336,7 +337,7 @@ vdpa_find_device(const struct rte_vdpa_device *start, rte_vdpa_cmp_t cmp, rte_spinlock_lock(&vdpa_device_list_lock); if (start == NULL) - dev = TAILQ_FIRST(&vdpa_device_list); + dev = TAILQ_FIRST(vdpa_device_list); else dev = TAILQ_NEXT(start, next); From patchwork Thu Jan 19 18:46:20 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Marchand X-Patchwork-Id: 122375 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 7DFCF4241A; Thu, 19 Jan 2023 19:47:30 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id C7258410EE; Thu, 19 Jan 2023 19:47:09 +0100 (CET) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mails.dpdk.org (Postfix) with ESMTP id D82C24067E for ; Thu, 19 Jan 2023 19:47:07 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1674154027; 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=MYXS1QE/Ih3tto1vhIOgLLfXQoI+qwrqHIR82SM684Y=; b=Abzskk1FynK+1Am0rXNWhXyLeFR7U68umF1bhApKQtPnSq7XJgfgYeJERY7skKdnUp3s+Z VIG06FMX0PiTLQjyqJo8a3qHhzrmhlFMZY+4WdZ1vDZSRTt8CQWl/uiz5XogfWbSbwhX9l lSJUd6bB89GOmKxnlyoyLw9pjuhNF3c= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-282-jUz0w7kDO8SZjS4FGFr9Qg-1; Thu, 19 Jan 2023 13:47:01 -0500 X-MC-Unique: jUz0w7kDO8SZjS4FGFr9Qg-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 838D3800B30; Thu, 19 Jan 2023 18:47:01 +0000 (UTC) Received: from dmarchan.redhat.com (ovpn-193-92.brq.redhat.com [10.40.193.92]) by smtp.corp.redhat.com (Postfix) with ESMTP id 1229A51EF; Thu, 19 Jan 2023 18:46:59 +0000 (UTC) From: David Marchand 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 Subject: [PATCH v4 9/9] vhost: enable lock check Date: Thu, 19 Jan 2023 19:46:20 +0100 Message-Id: <20230119184620.3195267-10-david.marchand@redhat.com> In-Reply-To: <20230119184620.3195267-1-david.marchand@redhat.com> References: <20220328121758.26632-1-david.marchand@redhat.com> <20230119184620.3195267-1-david.marchand@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.5 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Now that all locks in this library are annotated, we can enable the check. Signed-off-by: David Marchand Reviewed-by: Maxime Coquelin --- lib/vhost/meson.build | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/vhost/meson.build b/lib/vhost/meson.build index bc7272053b..197a51d936 100644 --- a/lib/vhost/meson.build +++ b/lib/vhost/meson.build @@ -17,6 +17,8 @@ elif (toolchain == 'icc' and cc.version().version_compare('>=16.0.0')) endif dpdk_conf.set('RTE_LIBRTE_VHOST_POSTCOPY', cc.has_header('linux/userfaultfd.h')) cflags += '-fno-strict-aliasing' + +annotate_locks = true sources = files( 'fd_man.c', 'iotlb.c',