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 9167542DD4; Tue, 4 Jul 2023 14:08:31 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 21C8640E03; Tue, 4 Jul 2023 14:08:31 +0200 (CEST) Received: from mail.lysator.liu.se (mail.lysator.liu.se [130.236.254.3]) by mails.dpdk.org (Postfix) with ESMTP id 7869840042 for ; Tue, 4 Jul 2023 14:08:29 +0200 (CEST) Received: from mail.lysator.liu.se (localhost [127.0.0.1]) by mail.lysator.liu.se (Postfix) with ESMTP id 1BD1D1F058 for ; Tue, 4 Jul 2023 14:08:29 +0200 (CEST) Received: by mail.lysator.liu.se (Postfix, from userid 1004) id 1A77C1F588; Tue, 4 Jul 2023 14:08:29 +0200 (CEST) X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on hermod.lysator.liu.se X-Spam-Level: X-Spam-Status: No, score=-1.5 required=5.0 tests=ALL_TRUSTED, AWL, NICE_REPLY_A, T_SCC_BODY_TEXT_LINE autolearn=disabled version=3.4.6 X-Spam-Score: -1.5 Received: from [192.168.1.59] (h-62-63-215-114.A163.priv.bahnhof.se [62.63.215.114]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature RSA-PSS (2048 bits)) (No client certificate requested) by mail.lysator.liu.se (Postfix) with ESMTPSA id 2AC071F413; Tue, 4 Jul 2023 14:08:26 +0200 (CEST) Message-ID: <7d352cdc-5395-49b3-c8a6-1d2dcc80864a@lysator.liu.se> Date: Tue, 4 Jul 2023 14:08:26 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.11.0 Subject: Re: [PATCH v2] eal: add notes to SMP memory barrier APIs Content-Language: en-US To: Ruifeng Wang , "thomas@monjalon.net" , "david.marchand@redhat.com" Cc: "dev@dpdk.org" , "roretzla@linux.microsoft.com" , "konstantin.v.ananyev@yandex.ru" , Honnappa Nagarahalli , nd References: <20230621064420.163931-1-ruifeng.wang@arm.com> <20230626071240.615611-1-ruifeng.wang@arm.com> <4d29cd1e-7963-2804-4a13-58ad0a83397d@lysator.liu.se> From: =?UTF-8?Q?Mattias_R=c3=b6nnblom?= In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Virus-Scanned: ClamAV using ClamSMTP 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 2023-07-03 09:02, Ruifeng Wang wrote: >> -----Original Message----- >> From: Mattias Rönnblom >> Sent: Friday, June 30, 2023 3:44 AM >> To: Ruifeng Wang ; thomas@monjalon.net; david.marchand@redhat.com >> Cc: dev@dpdk.org; roretzla@linux.microsoft.com; konstantin.v.ananyev@yandex.ru; Honnappa >> Nagarahalli ; nd >> Subject: Re: [PATCH v2] eal: add notes to SMP memory barrier APIs >> >> On 2023-06-26 09:12, Ruifeng Wang wrote: >>> The rte_smp_xx() APIs are deprecated. But it is not mentioned in the >>> function header. >>> Added notes in function header for clarification. >>> >>> Signed-off-by: Ruifeng Wang >>> --- >>> v2: Made the notes more specific. >>> >>> lib/eal/include/generic/rte_atomic.h | 19 +++++++++++++++++++ >>> 1 file changed, 19 insertions(+) >>> >>> diff --git a/lib/eal/include/generic/rte_atomic.h >>> b/lib/eal/include/generic/rte_atomic.h >>> index 58df843c54..35e0041ce6 100644 >>> --- a/lib/eal/include/generic/rte_atomic.h >>> +++ b/lib/eal/include/generic/rte_atomic.h >>> @@ -55,6 +55,11 @@ static inline void rte_rmb(void); >>> * Guarantees that the LOAD and STORE operations that precede the >>> * rte_smp_mb() call are globally visible across the lcores >>> * before the LOAD and STORE operations that follows it. >>> + * >>> + * @note >>> + * This function is deprecated. It provides fence synchronization >>> + * primitive but doesn't take memory order parameter. >>> + * rte_atomic_thread_fence() should be used instead. >> >> I can't see why coding the memory model semantics into the name, rather than by >> specification-by-means-of-a-parameter, could be the real issue. >> Could you explain? Seems like just different syntax to me. > > Yes, rte_smp_xx and rte_atomic_thread_fence have different syntaxes. > > The compiler atomic builtins were accepted for memory ordering. It comprises atomic arithmetic, > atomic load/store, and atomic fence. It is simpler and clearer to do memory ordering by using > the atomic builtins whenever possible. > rte_smp_xx has functionality overlap with atomic fence builtins but with different memory model > semantics and different syntaxes. Because of the differences, it will make memory ordering a little > more complex if rte_smp_xx is kept aside atomic builtins suite. > I wasn't arguing for keeping Linux kernel-style barriers. It was just the rationale that seemed flawed to me. If Linux kernel-style memory barriers took a memory model parameter, we would still prefer C11-style GCC barrier intrinsics (for this release). >> >> The old atomic arithmetic and atomic load/store operations suffered from >> unspecified semantics in regards to any ordering they imposed on other memory accesses. I >> guess that shortcoming could be described as a "missing parameter", although that too >> would be misleading. Unclear semantics seems not be the case for the kernel-style barriers >> though. >> >>> */ >>> static inline void rte_smp_mb(void); >>> >>> @@ -64,6 +69,11 @@ static inline void rte_smp_mb(void); >>> * Guarantees that the STORE operations that precede the >>> * rte_smp_wmb() call are globally visible across the lcores >>> * before the STORE operations that follows it. >>> + * >>> + * @note >>> + * This function is deprecated. It provides fence synchronization >>> + * primitive but doesn't take memory order parameter. >>> + * rte_atomic_thread_fence() should be used instead. >>> */ >>> static inline void rte_smp_wmb(void); >>> >>> @@ -73,6 +83,11 @@ static inline void rte_smp_wmb(void); >>> * Guarantees that the LOAD operations that precede the >>> * rte_smp_rmb() call are globally visible across the lcores >>> * before the LOAD operations that follows it. >>> + * >>> + * @note >>> + * This function is deprecated. It provides fence synchronization >>> + * primitive but doesn't take memory order parameter. >>> + * rte_atomic_thread_fence() should be used instead. >>> */ >>> static inline void rte_smp_rmb(void); >>> ///@} >>> @@ -122,6 +137,10 @@ static inline void rte_io_rmb(void); >>> >>> /** >>> * Synchronization fence between threads based on the specified memory order. >>> + * >>> + * @param memorder >>> + * The memory order defined by compiler atomic builtin at: >>> + * https://gcc.gnu.org/onlinedocs/gcc/_005f_005fatomic-Builtins.html >>> */ >>> static inline void rte_atomic_thread_fence(int memorder); >>>