From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id F0ACF428D4; Wed, 5 Apr 2023 17:42:25 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 899F741153; Wed, 5 Apr 2023 17:42:25 +0200 (CEST) Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mails.dpdk.org (Postfix) with ESMTP id EF22641133 for ; Wed, 5 Apr 2023 17:42:23 +0200 (CEST) Received: by linux.microsoft.com (Postfix, from userid 1086) id 4D787210DED8; Wed, 5 Apr 2023 08:42:23 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 4D787210DED8 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1680709343; bh=fJuZ1xxlvLcjnZGVxc94NLphI46tnSpSht7PerPfRuw=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=gmMzrvgD0qR1qntIUlBzLW+wPVbmz50a7XChoDOENfiihLaP2gz3z+j4vEJkG6UG6 XMikr2xyuYr86BlbKlfOlos1WP9HrBJaj3Qdo7hxm9NXwXDsTNyjGgX4Ry+MRDZWx/ 0IrOurTvvMXMLIhcShXhoo8YevqO8GEYD3QgbFpI= Date: Wed, 5 Apr 2023 08:42:23 -0700 From: Tyler Retzlaff To: Konstantin Ananyev Cc: "dev@dpdk.org" , "bruce.richardson@intel.com" , "david.marchand@redhat.com" , "thomas@monjalon.net" , "mb@smartsharesystems.com" Subject: Re: [PATCH v2 3/9] eal: use barrier intrinsics when compiling with msvc Message-ID: <20230405154223.GB31673@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> References: <1680558751-17931-1-git-send-email-roretzla@linux.microsoft.com> <1680638847-26430-1-git-send-email-roretzla@linux.microsoft.com> <1680638847-26430-4-git-send-email-roretzla@linux.microsoft.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) 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 On Wed, Apr 05, 2023 at 10:45:26AM +0000, Konstantin Ananyev wrote: > > > > Inline assembly is not supported for msvc x64 instead use > > _mm_{s,l,m}fence() intrinsics. > > > > Signed-off-by: Tyler Retzlaff > > --- > > lib/eal/include/generic/rte_atomic.h | 4 ++++ > > lib/eal/x86/include/rte_atomic.h | 10 +++++++++- > > 2 files changed, 13 insertions(+), 1 deletion(-) > > > > diff --git a/lib/eal/include/generic/rte_atomic.h b/lib/eal/include/generic/rte_atomic.h > > index 234b268..e973184 100644 > > --- a/lib/eal/include/generic/rte_atomic.h > > +++ b/lib/eal/include/generic/rte_atomic.h > > @@ -116,9 +116,13 @@ > > * Guarantees that operation reordering does not occur at compile time > > * for operations directly before and after the barrier. > > */ > > +#ifndef RTE_TOOLCHAIN_MSVC > > #define rte_compiler_barrier() do { \ > > asm volatile ("" : : : "memory"); \ > > } while(0) > > +#else > > +#define rte_compiler_barrier() _ReadWriteBarrier() > > +#endif > > > > /** > > * Synchronization fence between threads based on the specified memory order. > > diff --git a/lib/eal/x86/include/rte_atomic.h b/lib/eal/x86/include/rte_atomic.h > > index f2ee1a9..7ae3a41 100644 > > --- a/lib/eal/x86/include/rte_atomic.h > > +++ b/lib/eal/x86/include/rte_atomic.h > > @@ -27,9 +27,13 @@ > > > > #define rte_rmb() _mm_lfence() > > > > +#ifndef RTE_TOOLCHAIN_MSVC > > #define rte_smp_wmb() rte_compiler_barrier() > > - > > #define rte_smp_rmb() rte_compiler_barrier() > > +#else > > +#define rte_smp_wmb() _mm_sfence() > > +#define rte_smp_rmb() _mm_lfence() > > With x86 memory model CPU doesn't reorder with older reads and write with older writes > (there are few exceptions for writes: NT stores, fast string ops, but I think it can be skipped here). > For more info pls refer to: IA Software Developer's Manual, 3.8.3 8.2 MEMORY ORDERING. > That's why DPDK uses compiler_barrier() as expansion of smp_wmb() and smp_rmb() for x86 platforms. > There is nothing wrong in using sfence and lfence here, except that it is probably an overkill. thank you and Bruce for the explanation. how it was documented confused me. now i understand and i will drop this change. thanks! > > > +#endif > > > > /* > > * From Intel Software Development Manual; Vol 3; > > @@ -66,11 +70,15 @@ > > static __rte_always_inline void > > rte_smp_mb(void) > > { > > +#ifndef RTE_TOOLCHAIN_MSVC > > #ifdef RTE_ARCH_I686 > > asm volatile("lock addl $0, -128(%%esp); " ::: "memory"); > > #else > > asm volatile("lock addl $0, -128(%%rsp); " ::: "memory"); > > #endif > > +#else > > + _mm_mfence(); > > +#endif > > } > > > > #define rte_io_mb() rte_mb() > > -- > > 1.8.3.1