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 4B55BA0C43; Fri, 22 Oct 2021 02:10:54 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 0FAF340040; Fri, 22 Oct 2021 02:10:54 +0200 (CEST) Received: from mail-il1-f172.google.com (mail-il1-f172.google.com [209.85.166.172]) by mails.dpdk.org (Postfix) with ESMTP id 115C64003F for ; Fri, 22 Oct 2021 02:10:53 +0200 (CEST) Received: by mail-il1-f172.google.com with SMTP id s3so2580379ild.0 for ; Thu, 21 Oct 2021 17:10:53 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=jGfpIIMO2D781g0EPo/T6sCeRuFcP1qXK58l5jql2kg=; b=MUijYUSMb9UQZFJkkgLKA6PIiNPHsAUXRWJkvEIg6UCZpdOxuuqXFDVfj1ZILoOYaq Mf4T7UZlMhfWc4EVv5wXFsyjrTEwB6d3i5qLM4jzlnsgSn2Xss0+Nvijtq9Ao9AefnTZ UvDBbAnyqG1jwB0ucH4nWV1146V6W2aQWh5tYTDB7js+ABN7GAHFxPB0PSqqa+ap0E/b cqCvL0LSBMGIQ327xyGGi7LMmA09KBMltdZjotHn9XZXfC37YGbr3+e7uzxftvLnSS+R b/0WwEMCCortmBQpvlte88yfToDqbrPkIjluybp5X5CBLHXe5A2EuqBpDOdzWnTDQOkt FAbg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=jGfpIIMO2D781g0EPo/T6sCeRuFcP1qXK58l5jql2kg=; b=AL1S2zx8kjAUpbwWATKD0w01WWDc0GPWtMPiwrYFHemSH+OfhcvkkeZ2uq9TVa0fal qzDpPlVumEZzC23eK275W1Ay2GtEZ9+agA9AaPZEbmCJqfsAplgbs5MCgBjfcFfP1Uk/ 2tS6BC8O3vSc4VyDVPy6EhPxTXlde5jrcghkHvAz5ml1CxyM3hRYfUAUeYTI9ZTysfE6 qjpB6R5rRNbd/kwgiiVkfEJCuDLthCyJK7HDQy4UrDt03/Qe+TOdWricJJOYeYH9Z7I8 plZ50zeXi0bXcadcOG2paoUxRfSv9vH9vNowq/NV5EJo/aliO8Lfz+DOIE4KLMwvLvo0 JbVg== X-Gm-Message-State: AOAM5318RVHUJC6ZB9SED9W/RmnQMfY1enuN69ZvzIOSmYW7JvxJT0xn cUSlqsB94Rd7Ehm893zs+PnzgRoopFoMybixanJbl188/PA= X-Google-Smtp-Source: ABdhPJxbAIwjn/K6yEUnqgh0Jbam+MOOCJt8qpDCcSuyEfBh4dAY5SnSWZCCuhV/x7UiNmcWHzot6UsvlMhgbw4+SdU= X-Received: by 2002:a05:6e02:1d8d:: with SMTP id h13mr5381029ila.94.1634861452274; Thu, 21 Oct 2021 17:10:52 -0700 (PDT) MIME-Version: 1.0 References: <20210926063302.1541193-1-feifei.wang2@arm.com> <20211020084523.1309177-1-feifei.wang2@arm.com> <20211020084523.1309177-2-feifei.wang2@arm.com> In-Reply-To: <20211020084523.1309177-2-feifei.wang2@arm.com> From: Jerin Jacob Date: Fri, 22 Oct 2021 05:40:26 +0530 Message-ID: To: Feifei Wang Cc: Ruifeng Wang , "Ananyev, Konstantin" , dpdk-dev , nd Content-Type: text/plain; charset="UTF-8" Subject: Re: [dpdk-dev] [PATCH v4 1/5] eal: add new definitions for wait scheme 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 Sender: "dev" On Wed, Oct 20, 2021 at 2:16 PM Feifei Wang wrote: > > Introduce macros as generic interface for address monitoring. > > Signed-off-by: Feifei Wang > Reviewed-by: Ruifeng Wang > --- > lib/eal/arm/include/rte_pause_64.h | 126 ++++++++++++++++------------ > lib/eal/include/generic/rte_pause.h | 32 +++++++ > 2 files changed, 104 insertions(+), 54 deletions(-) > > diff --git a/lib/eal/arm/include/rte_pause_64.h b/lib/eal/arm/include/rte_pause_64.h > index e87d10b8cc..23954c2de2 100644 > --- a/lib/eal/arm/include/rte_pause_64.h > +++ b/lib/eal/arm/include/rte_pause_64.h > @@ -31,20 +31,12 @@ static inline void rte_pause(void) > /* Put processor into low power WFE(Wait For Event) state. */ > #define __WFE() { asm volatile("wfe" : : : "memory"); } > > -static __rte_always_inline void > -rte_wait_until_equal_16(volatile uint16_t *addr, uint16_t expected, > - int memorder) > -{ > - uint16_t value; > - > - assert(memorder == __ATOMIC_ACQUIRE || memorder == __ATOMIC_RELAXED); > - > - /* > - * Atomic exclusive load from addr, it returns the 16-bit content of > - * *addr while making it 'monitored',when it is written by someone > - * else, the 'monitored' state is cleared and a event is generated a event -> an event in all the occurrence. > - * implicitly to exit WFE. > - */ > +/* > + * Atomic exclusive load from addr, it returns the 16-bit content of > + * *addr while making it 'monitored', when it is written by someone > + * else, the 'monitored' state is cleared and a event is generated > + * implicitly to exit WFE. > + */ > #define __LOAD_EXC_16(src, dst, memorder) { \ > if (memorder == __ATOMIC_RELAXED) { \ > asm volatile("ldxrh %w[tmp], [%x[addr]]" \ > @@ -58,6 +50,52 @@ rte_wait_until_equal_16(volatile uint16_t *addr, uint16_t expected, > : "memory"); \ > } } > > +/* > + * Atomic exclusive load from addr, it returns the 32-bit content of > + * *addr while making it 'monitored', when it is written by someone > + * else, the 'monitored' state is cleared and a event is generated > + * implicitly to exit WFE. > + */ > +#define __LOAD_EXC_32(src, dst, memorder) { \ > + if (memorder == __ATOMIC_RELAXED) { \ > + asm volatile("ldxr %w[tmp], [%x[addr]]" \ > + : [tmp] "=&r" (dst) \ > + : [addr] "r"(src) \ > + : "memory"); \ > + } else { \ > + asm volatile("ldaxr %w[tmp], [%x[addr]]" \ > + : [tmp] "=&r" (dst) \ > + : [addr] "r"(src) \ > + : "memory"); \ > + } } > + > +/* > + * Atomic exclusive load from addr, it returns the 64-bit content of > + * *addr while making it 'monitored', when it is written by someone > + * else, the 'monitored' state is cleared and a event is generated > + * implicitly to exit WFE. > + */ > +#define __LOAD_EXC_64(src, dst, memorder) { \ > + if (memorder == __ATOMIC_RELAXED) { \ > + asm volatile("ldxr %x[tmp], [%x[addr]]" \ > + : [tmp] "=&r" (dst) \ > + : [addr] "r"(src) \ > + : "memory"); \ > + } else { \ > + asm volatile("ldaxr %x[tmp], [%x[addr]]" \ > + : [tmp] "=&r" (dst) \ > + : [addr] "r"(src) \ > + : "memory"); \ > + } } > + > +static __rte_always_inline void > +rte_wait_until_equal_16(volatile uint16_t *addr, uint16_t expected, > + int memorder) > +{ > + uint16_t value; > + > + assert(memorder == __ATOMIC_ACQUIRE || memorder == __ATOMIC_RELAXED); > + > __LOAD_EXC_16(addr, value, memorder) > if (value != expected) { > __SEVL() > @@ -66,7 +104,6 @@ rte_wait_until_equal_16(volatile uint16_t *addr, uint16_t expected, > __LOAD_EXC_16(addr, value, memorder) > } while (value != expected); > } > -#undef __LOAD_EXC_16 > } > > static __rte_always_inline void > @@ -77,25 +114,6 @@ rte_wait_until_equal_32(volatile uint32_t *addr, uint32_t expected, > > assert(memorder == __ATOMIC_ACQUIRE || memorder == __ATOMIC_RELAXED); > > - /* > - * Atomic exclusive load from addr, it returns the 32-bit content of > - * *addr while making it 'monitored',when it is written by someone > - * else, the 'monitored' state is cleared and a event is generated > - * implicitly to exit WFE. > - */ > -#define __LOAD_EXC_32(src, dst, memorder) { \ > - if (memorder == __ATOMIC_RELAXED) { \ > - asm volatile("ldxr %w[tmp], [%x[addr]]" \ > - : [tmp] "=&r" (dst) \ > - : [addr] "r"(src) \ > - : "memory"); \ > - } else { \ > - asm volatile("ldaxr %w[tmp], [%x[addr]]" \ > - : [tmp] "=&r" (dst) \ > - : [addr] "r"(src) \ > - : "memory"); \ > - } } > - > __LOAD_EXC_32(addr, value, memorder) > if (value != expected) { > __SEVL() > @@ -104,7 +122,6 @@ rte_wait_until_equal_32(volatile uint32_t *addr, uint32_t expected, > __LOAD_EXC_32(addr, value, memorder) > } while (value != expected); > } > -#undef __LOAD_EXC_32 > } > > static __rte_always_inline void > @@ -115,25 +132,6 @@ rte_wait_until_equal_64(volatile uint64_t *addr, uint64_t expected, > > assert(memorder == __ATOMIC_ACQUIRE || memorder == __ATOMIC_RELAXED); > > - /* > - * Atomic exclusive load from addr, it returns the 64-bit content of > - * *addr while making it 'monitored',when it is written by someone > - * else, the 'monitored' state is cleared and a event is generated > - * implicitly to exit WFE. > - */ > -#define __LOAD_EXC_64(src, dst, memorder) { \ > - if (memorder == __ATOMIC_RELAXED) { \ > - asm volatile("ldxr %x[tmp], [%x[addr]]" \ > - : [tmp] "=&r" (dst) \ > - : [addr] "r"(src) \ > - : "memory"); \ > - } else { \ > - asm volatile("ldaxr %x[tmp], [%x[addr]]" \ > - : [tmp] "=&r" (dst) \ > - : [addr] "r"(src) \ > - : "memory"); \ > - } } > - > __LOAD_EXC_64(addr, value, memorder) > if (value != expected) { > __SEVL() > @@ -143,6 +141,26 @@ rte_wait_until_equal_64(volatile uint64_t *addr, uint64_t expected, > } while (value != expected); > } > } > + > +#define rte_wait_event(addr, mask, expected, cond, memorder, size) \ I think it is better to swap "cond" and "expected" positions to get better readability. rte_wait_event(&buf->bufptr64, RTE_DISTRIB_FLAGS_MASK, 0, !=, __ATOMIC_RELAXED, 64); Vs rte_wait_event(&buf->bufptr64, RTE_DISTRIB_FLAGS_MASK, !=, 0, __ATOMIC_RELAXED, 64); > +do { \ Any reason to not make an inline function instead of macro? > + RTE_BUILD_BUG_ON(!__builtin_constant_p(memorder)); \ Should n't we add __builtin_constant_p(size) of check? > + RTE_BUILD_BUG_ON(memorder != __ATOMIC_ACQUIRE && \ > + memorder != __ATOMIC_RELAXED); \ > + RTE_BUILD_BUG_ON(size != 16 && size != 32 && size != 64); \ > + uint##size_t value; \ > + __LOAD_EXC_##size(addr, value, memorder) \ > + if ((value & mask) cond expected) { \ > + __SEVL() \ > + do { \ > + __WFE() \ > + __LOAD_EXC_##size(addr, value, memorder) \ > + } while ((value & mask) cond expected); \ > + } \ > +} while (0) > + > +#undef __LOAD_EXC_16 > +#undef __LOAD_EXC_32 > #undef __LOAD_EXC_64 > > #undef __SEVL > diff --git a/lib/eal/include/generic/rte_pause.h b/lib/eal/include/generic/rte_pause.h > index 668ee4a184..20a5d2a9fd 100644 > --- a/lib/eal/include/generic/rte_pause.h > +++ b/lib/eal/include/generic/rte_pause.h > @@ -111,6 +111,38 @@ rte_wait_until_equal_64(volatile uint64_t *addr, uint64_t expected, > while (__atomic_load_n(addr, memorder) != expected) > rte_pause(); > } > + > +/* > + * Wait until *addr breaks the condition, with a relaxed memory > + * ordering model meaning the loads around this API can be reordered. > + * > + * @param addr > + * A pointer to the memory location. > + * @param mask > + * A mask of value bits in interest. > + * @param expected > + * A 16-bit expected value to be in the memory location. > + * @param cond > + * A symbol representing the condition (==, !=). > + * @param memorder > + * Two different memory orders that can be specified: > + * __ATOMIC_ACQUIRE and __ATOMIC_RELAXED. These map to > + * C++11 memory orders with the same names, see the C++11 standard or > + * the GCC wiki on atomic synchronization for detailed definition. > + * @param size > + * The bit size of *addr: > + * It is used for arm architecture to choose load instructions, > + * and the optional value is 16, 32 and 64. > + */ > +#define rte_wait_event(addr, mask, expected, cond, memorder, size) \ > +do { \ > + RTE_BUILD_BUG_ON(!__builtin_constant_p(memorder)); \ > + RTE_BUILD_BUG_ON(memorder != __ATOMIC_ACQUIRE && \ > + memorder != __ATOMIC_RELAXED); \ > + RTE_BUILD_BUG_ON(size != 16 && size != 32 && size != 64); \ > + while ((__atomic_load_n(addr, memorder) & mask) cond expected) \ > + rte_pause(); \ > +} while (0) > #endif > > #endif /* _RTE_PAUSE_H_ */ > -- > 2.25.1 >