DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH v1 0/2] Use WFE for spinlock and ring
@ 2020-04-24  7:07 Gavin Hu
  2020-04-24  7:07 ` [dpdk-dev] [PATCH v1 1/2] spinlock: use wfe to reduce contention on aarch64 Gavin Hu
                   ` (5 more replies)
  0 siblings, 6 replies; 36+ messages in thread
From: Gavin Hu @ 2020-04-24  7:07 UTC (permalink / raw)
  To: dev
  Cc: nd, david.marchand, thomas, jerinj, Honnappa.Nagarahalli,
	Ruifeng.Wang, Phil.Yang, Joyce.Kong

The rte_wait_until_equal_xxx APIs abstract the functionality of 'polling
for a memory location to become equal to a given value'[1].

Use the API for the rte spinlock and ring implementations.

[1] http://patches.dpdk.org/cover/62703/

Gavin Hu (2):
  spinlock: use wfe to reduce contention on aarch64
  ring: use wfe to wait for ring tail update on aarch64

 lib/librte_eal/include/generic/rte_spinlock.h | 5 ++---
 lib/librte_ring/rte_ring_c11_mem.h            | 4 ++--
 lib/librte_ring/rte_ring_generic.h            | 3 +--
 3 files changed, 5 insertions(+), 7 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 36+ messages in thread

* [dpdk-dev] [PATCH v1 1/2] spinlock: use wfe to reduce contention on aarch64
  2020-04-24  7:07 [dpdk-dev] [PATCH v1 0/2] Use WFE for spinlock and ring Gavin Hu
@ 2020-04-24  7:07 ` Gavin Hu
  2020-04-24  7:07 ` [dpdk-dev] [PATCH v1 2/2] ring: use wfe to wait for ring tail update " Gavin Hu
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 36+ messages in thread
From: Gavin Hu @ 2020-04-24  7:07 UTC (permalink / raw)
  To: dev
  Cc: nd, david.marchand, thomas, jerinj, Honnappa.Nagarahalli,
	Ruifeng.Wang, Phil.Yang, Joyce.Kong

In acquiring a spinlock, cores repeatedly poll the lock variable.
This is replaced by rte_wait_until_equal API.

Running the micro benchmarking and the testpmd and l3fwd traffic tests
on ThunderX2, Ampere eMAG80 and Arm N1SDP, everything went well and no
notable performance gain nor degradation was measured.

Signed-off-by: Gavin Hu <gavin.hu@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
Reviewed-by: Phil Yang <phil.yang@arm.com>
Reviewed-by: Steve Capper <steve.capper@arm.com>
Reviewed-by: Ola Liljedahl <ola.liljedahl@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Tested-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
---
 lib/librte_eal/include/generic/rte_spinlock.h | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/lib/librte_eal/include/generic/rte_spinlock.h b/lib/librte_eal/include/generic/rte_spinlock.h
index 87ae7a4f1..5cc123247 100644
--- a/lib/librte_eal/include/generic/rte_spinlock.h
+++ b/lib/librte_eal/include/generic/rte_spinlock.h
@@ -28,7 +28,7 @@
  * The rte_spinlock_t type.
  */
 typedef struct {
-	volatile int locked; /**< lock status 0 = unlocked, 1 = locked */
+	volatile uint32_t locked; /**< lock status 0 = unlocked, 1 = locked */
 } rte_spinlock_t;
 
 /**
@@ -65,8 +65,7 @@ rte_spinlock_lock(rte_spinlock_t *sl)
 
 	while (!__atomic_compare_exchange_n(&sl->locked, &exp, 1, 0,
 				__ATOMIC_ACQUIRE, __ATOMIC_RELAXED)) {
-		while (__atomic_load_n(&sl->locked, __ATOMIC_RELAXED))
-			rte_pause();
+		rte_wait_until_equal_32(&sl->locked, 0, __ATOMIC_RELAXED);
 		exp = 0;
 	}
 }
-- 
2.17.1


^ permalink raw reply	[flat|nested] 36+ messages in thread

* [dpdk-dev] [PATCH v1 2/2] ring: use wfe to wait for ring tail update on aarch64
  2020-04-24  7:07 [dpdk-dev] [PATCH v1 0/2] Use WFE for spinlock and ring Gavin Hu
  2020-04-24  7:07 ` [dpdk-dev] [PATCH v1 1/2] spinlock: use wfe to reduce contention on aarch64 Gavin Hu
@ 2020-04-24  7:07 ` Gavin Hu
  2020-04-24 13:23   ` Ananyev, Konstantin
  2020-04-24 14:46   ` Honnappa Nagarahalli
  2020-04-26  8:39 ` [dpdk-dev] [PATCH v2 0/2] Use WFE for spinlock and ring Gavin Hu
                   ` (3 subsequent siblings)
  5 siblings, 2 replies; 36+ messages in thread
From: Gavin Hu @ 2020-04-24  7:07 UTC (permalink / raw)
  To: dev
  Cc: nd, david.marchand, thomas, jerinj, Honnappa.Nagarahalli,
	Ruifeng.Wang, Phil.Yang, Joyce.Kong

Instead of polling for tail to be updated, use wfe instruction.

Signed-off-by: Gavin Hu <gavin.hu@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
Reviewed-by: Steve Capper <steve.capper@arm.com>
Reviewed-by: Ola Liljedahl <ola.liljedahl@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
---
 lib/librte_ring/rte_ring_c11_mem.h | 4 ++--
 lib/librte_ring/rte_ring_generic.h | 3 +--
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/lib/librte_ring/rte_ring_c11_mem.h b/lib/librte_ring/rte_ring_c11_mem.h
index 0fb73a337..764d8f186 100644
--- a/lib/librte_ring/rte_ring_c11_mem.h
+++ b/lib/librte_ring/rte_ring_c11_mem.h
@@ -2,6 +2,7 @@
  *
  * Copyright (c) 2017,2018 HXT-semitech Corporation.
  * Copyright (c) 2007-2009 Kip Macy kmacy@freebsd.org
+ * Copyright (c) 2019 Arm Limited
  * All rights reserved.
  * Derived from FreeBSD's bufring.h
  * Used as BSD-3 Licensed with permission from Kip Macy.
@@ -21,8 +22,7 @@ update_tail(struct rte_ring_headtail *ht, uint32_t old_val, uint32_t new_val,
 	 * we need to wait for them to complete
 	 */
 	if (!single)
-		while (unlikely(ht->tail != old_val))
-			rte_pause();
+		rte_wait_until_equal_relaxed_32(&ht->tail, old_val);
 
 	__atomic_store_n(&ht->tail, new_val, __ATOMIC_RELEASE);
 }
diff --git a/lib/librte_ring/rte_ring_generic.h b/lib/librte_ring/rte_ring_generic.h
index 953cdbbd5..682852783 100644
--- a/lib/librte_ring/rte_ring_generic.h
+++ b/lib/librte_ring/rte_ring_generic.h
@@ -23,8 +23,7 @@ update_tail(struct rte_ring_headtail *ht, uint32_t old_val, uint32_t new_val,
 	 * we need to wait for them to complete
 	 */
 	if (!single)
-		while (unlikely(ht->tail != old_val))
-			rte_pause();
+		rte_wait_until_equal_relaxed_32(&ht->tail, old_val);
 
 	ht->tail = new_val;
 }
-- 
2.17.1


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [dpdk-dev] [PATCH v1 2/2] ring: use wfe to wait for ring tail update on aarch64
  2020-04-24  7:07 ` [dpdk-dev] [PATCH v1 2/2] ring: use wfe to wait for ring tail update " Gavin Hu
@ 2020-04-24 13:23   ` Ananyev, Konstantin
  2020-04-26  8:41     ` Gavin Hu
  2020-04-24 14:46   ` Honnappa Nagarahalli
  1 sibling, 1 reply; 36+ messages in thread
From: Ananyev, Konstantin @ 2020-04-24 13:23 UTC (permalink / raw)
  To: Gavin Hu, dev
  Cc: nd, david.marchand, thomas, jerinj, Honnappa.Nagarahalli,
	Ruifeng.Wang, Phil.Yang, Joyce.Kong


> 
> Instead of polling for tail to be updated, use wfe instruction.
> 
> Signed-off-by: Gavin Hu <gavin.hu@arm.com>
> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> Reviewed-by: Steve Capper <steve.capper@arm.com>
> Reviewed-by: Ola Liljedahl <ola.liljedahl@arm.com>
> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> ---
>  lib/librte_ring/rte_ring_c11_mem.h | 4 ++--
>  lib/librte_ring/rte_ring_generic.h | 3 +--
>  2 files changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/librte_ring/rte_ring_c11_mem.h b/lib/librte_ring/rte_ring_c11_mem.h
> index 0fb73a337..764d8f186 100644
> --- a/lib/librte_ring/rte_ring_c11_mem.h
> +++ b/lib/librte_ring/rte_ring_c11_mem.h
> @@ -2,6 +2,7 @@
>   *
>   * Copyright (c) 2017,2018 HXT-semitech Corporation.
>   * Copyright (c) 2007-2009 Kip Macy kmacy@freebsd.org
> + * Copyright (c) 2019 Arm Limited
>   * All rights reserved.
>   * Derived from FreeBSD's bufring.h
>   * Used as BSD-3 Licensed with permission from Kip Macy.
> @@ -21,8 +22,7 @@ update_tail(struct rte_ring_headtail *ht, uint32_t old_val, uint32_t new_val,
>  	 * we need to wait for them to complete
>  	 */
>  	if (!single)
> -		while (unlikely(ht->tail != old_val))
> -			rte_pause();
> +		rte_wait_until_equal_relaxed_32(&ht->tail, old_val);
> 
>  	__atomic_store_n(&ht->tail, new_val, __ATOMIC_RELEASE);
>  }
> diff --git a/lib/librte_ring/rte_ring_generic.h b/lib/librte_ring/rte_ring_generic.h
> index 953cdbbd5..682852783 100644
> --- a/lib/librte_ring/rte_ring_generic.h
> +++ b/lib/librte_ring/rte_ring_generic.h
> @@ -23,8 +23,7 @@ update_tail(struct rte_ring_headtail *ht, uint32_t old_val, uint32_t new_val,
>  	 * we need to wait for them to complete
>  	 */
>  	if (!single)
> -		while (unlikely(ht->tail != old_val))
> -			rte_pause();
> +		rte_wait_until_equal_relaxed_32(&ht->tail, old_val);

./lib/librte_ring/rte_ring_generic.h:26:3: error: implicit declaration of function 'rte_wait_until_equal_relaxed_32'; did you mean 'rte_wait_until_equal_32'? [-Werror=implicit-function-declaration]
   rte_wait_until_equal_relaxed_32(&ht->tail, old_val);
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


BTW, after patch #1 compilation fails also:
../lib/librte_eal/x86/include/rte_spinlock.h:125:24: error: pointer targets in passing argument 1 of 'rte_try_tm' differ in signedness [-Werror=pointer-sign]
  if (likely(rte_try_tm(&sl->locked)))
                        ^



> 
>  	ht->tail = new_val;
>  }
> --
> 2.17.1


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [dpdk-dev] [PATCH v1 2/2] ring: use wfe to wait for ring tail update on aarch64
  2020-04-24  7:07 ` [dpdk-dev] [PATCH v1 2/2] ring: use wfe to wait for ring tail update " Gavin Hu
  2020-04-24 13:23   ` Ananyev, Konstantin
@ 2020-04-24 14:46   ` Honnappa Nagarahalli
  2020-04-26  8:33     ` Gavin Hu
  1 sibling, 1 reply; 36+ messages in thread
From: Honnappa Nagarahalli @ 2020-04-24 14:46 UTC (permalink / raw)
  To: Gavin Hu, dev
  Cc: nd, david.marchand, thomas, jerinj, Ruifeng Wang, Phil Yang,
	Joyce Kong, Honnappa Nagarahalli, nd

<snip>

Hi Gavin,
	There are other sync modes added to rte_ring library. Can you please extend this to other rte_pause() calls in the library?

Thanks,
Honnappa

> Subject: [PATCH v1 2/2] ring: use wfe to wait for ring tail update on aarch64
> 
> Instead of polling for tail to be updated, use wfe instruction.
> 
> Signed-off-by: Gavin Hu <gavin.hu@arm.com>
> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> Reviewed-by: Steve Capper <steve.capper@arm.com>
> Reviewed-by: Ola Liljedahl <ola.liljedahl@arm.com>
> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> ---
>  lib/librte_ring/rte_ring_c11_mem.h | 4 ++--
> lib/librte_ring/rte_ring_generic.h | 3 +--
>  2 files changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/librte_ring/rte_ring_c11_mem.h
> b/lib/librte_ring/rte_ring_c11_mem.h
> index 0fb73a337..764d8f186 100644
> --- a/lib/librte_ring/rte_ring_c11_mem.h
> +++ b/lib/librte_ring/rte_ring_c11_mem.h
> @@ -2,6 +2,7 @@
>   *
>   * Copyright (c) 2017,2018 HXT-semitech Corporation.
>   * Copyright (c) 2007-2009 Kip Macy kmacy@freebsd.org
> + * Copyright (c) 2019 Arm Limited
>   * All rights reserved.
>   * Derived from FreeBSD's bufring.h
>   * Used as BSD-3 Licensed with permission from Kip Macy.
> @@ -21,8 +22,7 @@ update_tail(struct rte_ring_headtail *ht, uint32_t
> old_val, uint32_t new_val,
>  	 * we need to wait for them to complete
>  	 */
>  	if (!single)
> -		while (unlikely(ht->tail != old_val))
> -			rte_pause();
> +		rte_wait_until_equal_relaxed_32(&ht->tail, old_val);
> 
>  	__atomic_store_n(&ht->tail, new_val, __ATOMIC_RELEASE);  } diff --
> git a/lib/librte_ring/rte_ring_generic.h b/lib/librte_ring/rte_ring_generic.h
> index 953cdbbd5..682852783 100644
> --- a/lib/librte_ring/rte_ring_generic.h
> +++ b/lib/librte_ring/rte_ring_generic.h
> @@ -23,8 +23,7 @@ update_tail(struct rte_ring_headtail *ht, uint32_t
> old_val, uint32_t new_val,
>  	 * we need to wait for them to complete
>  	 */
>  	if (!single)
> -		while (unlikely(ht->tail != old_val))
> -			rte_pause();
> +		rte_wait_until_equal_relaxed_32(&ht->tail, old_val);
> 
>  	ht->tail = new_val;
>  }
> --
> 2.17.1


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [dpdk-dev] [PATCH v1 2/2] ring: use wfe to wait for ring tail update on aarch64
  2020-04-24 14:46   ` Honnappa Nagarahalli
@ 2020-04-26  8:33     ` Gavin Hu
  0 siblings, 0 replies; 36+ messages in thread
From: Gavin Hu @ 2020-04-26  8:33 UTC (permalink / raw)
  To: Honnappa Nagarahalli, dev
  Cc: nd, david.marchand, thomas, jerinj, Ruifeng Wang, Phil Yang,
	Joyce Kong, nd, nd

Hi Honnappa,

> -----Original Message-----
> From: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> Sent: Friday, April 24, 2020 10:46 PM
> To: Gavin Hu <Gavin.Hu@arm.com>; dev@dpdk.org
> Cc: nd <nd@arm.com>; david.marchand@redhat.com;
> thomas@monjalon.net; jerinj@marvell.com; Ruifeng Wang
> <Ruifeng.Wang@arm.com>; Phil Yang <Phil.Yang@arm.com>; Joyce Kong
> <Joyce.Kong@arm.com>; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>; nd <nd@arm.com>
> Subject: RE: [PATCH v1 2/2] ring: use wfe to wait for ring tail update on
> aarch64
> 
> <snip>
> 
> Hi Gavin,
> 	There are other sync modes added to rte_ring library. Can you
> please extend this to other rte_pause() calls in the library?
I looked into the other two calls to rte_cause, they can't use the rte_wait_until_equal_XX APIs simply.
Maybe new APIs are required? 
Here is the 32-bit API for your reference:
rte_wait_until_equal_32(volatile uint32_t *addr, uint32_t expected, int memorder)
> 
> Thanks,
> Honnappa
> 
> > Subject: [PATCH v1 2/2] ring: use wfe to wait for ring tail update on
> aarch64
> >
> > Instead of polling for tail to be updated, use wfe instruction.
> >
> > Signed-off-by: Gavin Hu <gavin.hu@arm.com>
> > Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> > Reviewed-by: Steve Capper <steve.capper@arm.com>
> > Reviewed-by: Ola Liljedahl <ola.liljedahl@arm.com>
> > Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > ---
> >  lib/librte_ring/rte_ring_c11_mem.h | 4 ++--
> > lib/librte_ring/rte_ring_generic.h | 3 +--
> >  2 files changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git a/lib/librte_ring/rte_ring_c11_mem.h
> > b/lib/librte_ring/rte_ring_c11_mem.h
> > index 0fb73a337..764d8f186 100644
> > --- a/lib/librte_ring/rte_ring_c11_mem.h
> > +++ b/lib/librte_ring/rte_ring_c11_mem.h
> > @@ -2,6 +2,7 @@
> >   *
> >   * Copyright (c) 2017,2018 HXT-semitech Corporation.
> >   * Copyright (c) 2007-2009 Kip Macy kmacy@freebsd.org
> > + * Copyright (c) 2019 Arm Limited
> >   * All rights reserved.
> >   * Derived from FreeBSD's bufring.h
> >   * Used as BSD-3 Licensed with permission from Kip Macy.
> > @@ -21,8 +22,7 @@ update_tail(struct rte_ring_headtail *ht, uint32_t
> > old_val, uint32_t new_val,
> >  	 * we need to wait for them to complete
> >  	 */
> >  	if (!single)
> > -		while (unlikely(ht->tail != old_val))
> > -			rte_pause();
> > +		rte_wait_until_equal_relaxed_32(&ht->tail, old_val);
> >
> >  	__atomic_store_n(&ht->tail, new_val, __ATOMIC_RELEASE);  } diff --
> > git a/lib/librte_ring/rte_ring_generic.h b/lib/librte_ring/rte_ring_generic.h
> > index 953cdbbd5..682852783 100644
> > --- a/lib/librte_ring/rte_ring_generic.h
> > +++ b/lib/librte_ring/rte_ring_generic.h
> > @@ -23,8 +23,7 @@ update_tail(struct rte_ring_headtail *ht, uint32_t
> > old_val, uint32_t new_val,
> >  	 * we need to wait for them to complete
> >  	 */
> >  	if (!single)
> > -		while (unlikely(ht->tail != old_val))
> > -			rte_pause();
> > +		rte_wait_until_equal_relaxed_32(&ht->tail, old_val);
> >
> >  	ht->tail = new_val;
> >  }
> > --
> > 2.17.1


^ permalink raw reply	[flat|nested] 36+ messages in thread

* [dpdk-dev] [PATCH v2 0/2] Use WFE for spinlock and ring
  2020-04-24  7:07 [dpdk-dev] [PATCH v1 0/2] Use WFE for spinlock and ring Gavin Hu
  2020-04-24  7:07 ` [dpdk-dev] [PATCH v1 1/2] spinlock: use wfe to reduce contention on aarch64 Gavin Hu
  2020-04-24  7:07 ` [dpdk-dev] [PATCH v1 2/2] ring: use wfe to wait for ring tail update " Gavin Hu
@ 2020-04-26  8:39 ` Gavin Hu
  2020-05-01  9:47   ` Ananyev, Konstantin
  2020-05-03 14:54   ` David Marchand
  2020-04-26  8:39 ` [dpdk-dev] [PATCH v2 1/2] spinlock: use wfe to reduce contention on aarch64 Gavin Hu
                   ` (2 subsequent siblings)
  5 siblings, 2 replies; 36+ messages in thread
From: Gavin Hu @ 2020-04-26  8:39 UTC (permalink / raw)
  To: dev
  Cc: nd, david.marchand, thomas, jerinj, Honnappa.Nagarahalli,
	Ruifeng.Wang, Phil.Yang, Joyce.Kong

The rte_wait_until_equal_xxx APIs abstract the functionality of 'polling
for a memory location to become equal to a given value'[1].

Use the API for the rte spinlock and ring implementations.

[1] http://patches.dpdk.org/cover/62703/

Gavin Hu (2):
  spinlock: use wfe to reduce contention on aarch64
  ring: use wfe to wait for ring tail update on aarch64

 lib/librte_eal/include/generic/rte_spinlock.h | 4 ++--
 lib/librte_ring/rte_ring_c11_mem.h            | 4 ++--
 lib/librte_ring/rte_ring_generic.h            | 3 +--
 3 files changed, 5 insertions(+), 6 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 36+ messages in thread

* [dpdk-dev] [PATCH v2 1/2] spinlock: use wfe to reduce contention on aarch64
  2020-04-24  7:07 [dpdk-dev] [PATCH v1 0/2] Use WFE for spinlock and ring Gavin Hu
                   ` (2 preceding siblings ...)
  2020-04-26  8:39 ` [dpdk-dev] [PATCH v2 0/2] Use WFE for spinlock and ring Gavin Hu
@ 2020-04-26  8:39 ` Gavin Hu
  2020-04-28 12:22   ` Jerin Jacob
  2020-04-26  8:39 ` [dpdk-dev] [PATCH v2 2/2] ring: use wfe to wait for ring tail update " Gavin Hu
  2021-04-25  5:56 ` [dpdk-dev] Use WFE for spinlock and ring Ruifeng Wang
  5 siblings, 1 reply; 36+ messages in thread
From: Gavin Hu @ 2020-04-26  8:39 UTC (permalink / raw)
  To: dev
  Cc: nd, david.marchand, thomas, jerinj, Honnappa.Nagarahalli,
	Ruifeng.Wang, Phil.Yang, Joyce.Kong

In acquiring a spinlock, cores repeatedly poll the lock variable.
This is replaced by rte_wait_until_equal API.

Running the micro benchmarking and the testpmd and l3fwd traffic tests
on ThunderX2, Ampere eMAG80 and Arm N1SDP, everything went well and no
notable performance gain nor degradation was measured.

Signed-off-by: Gavin Hu <gavin.hu@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
Reviewed-by: Phil Yang <phil.yang@arm.com>
Reviewed-by: Steve Capper <steve.capper@arm.com>
Reviewed-by: Ola Liljedahl <ola.liljedahl@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Tested-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
---
 lib/librte_eal/include/generic/rte_spinlock.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/librte_eal/include/generic/rte_spinlock.h b/lib/librte_eal/include/generic/rte_spinlock.h
index 87ae7a4f1..40fe49d5a 100644
--- a/lib/librte_eal/include/generic/rte_spinlock.h
+++ b/lib/librte_eal/include/generic/rte_spinlock.h
@@ -65,8 +65,8 @@ rte_spinlock_lock(rte_spinlock_t *sl)
 
 	while (!__atomic_compare_exchange_n(&sl->locked, &exp, 1, 0,
 				__ATOMIC_ACQUIRE, __ATOMIC_RELAXED)) {
-		while (__atomic_load_n(&sl->locked, __ATOMIC_RELAXED))
-			rte_pause();
+		rte_wait_until_equal_32((volatile uint32_t *)&sl->locked,
+			       0, __ATOMIC_RELAXED);
 		exp = 0;
 	}
 }
-- 
2.17.1


^ permalink raw reply	[flat|nested] 36+ messages in thread

* [dpdk-dev] [PATCH v2 2/2] ring: use wfe to wait for ring tail update on aarch64
  2020-04-24  7:07 [dpdk-dev] [PATCH v1 0/2] Use WFE for spinlock and ring Gavin Hu
                   ` (3 preceding siblings ...)
  2020-04-26  8:39 ` [dpdk-dev] [PATCH v2 1/2] spinlock: use wfe to reduce contention on aarch64 Gavin Hu
@ 2020-04-26  8:39 ` Gavin Hu
  2021-04-25  5:56 ` [dpdk-dev] Use WFE for spinlock and ring Ruifeng Wang
  5 siblings, 0 replies; 36+ messages in thread
From: Gavin Hu @ 2020-04-26  8:39 UTC (permalink / raw)
  To: dev
  Cc: nd, david.marchand, thomas, jerinj, Honnappa.Nagarahalli,
	Ruifeng.Wang, Phil.Yang, Joyce.Kong

Instead of polling for tail to be updated, use wfe instruction.

Signed-off-by: Gavin Hu <gavin.hu@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
Reviewed-by: Steve Capper <steve.capper@arm.com>
Reviewed-by: Ola Liljedahl <ola.liljedahl@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
---
 lib/librte_ring/rte_ring_c11_mem.h | 4 ++--
 lib/librte_ring/rte_ring_generic.h | 3 +--
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/lib/librte_ring/rte_ring_c11_mem.h b/lib/librte_ring/rte_ring_c11_mem.h
index 0fb73a337..b37ef5995 100644
--- a/lib/librte_ring/rte_ring_c11_mem.h
+++ b/lib/librte_ring/rte_ring_c11_mem.h
@@ -2,6 +2,7 @@
  *
  * Copyright (c) 2017,2018 HXT-semitech Corporation.
  * Copyright (c) 2007-2009 Kip Macy kmacy@freebsd.org
+ * Copyright (c) 2019 Arm Limited
  * All rights reserved.
  * Derived from FreeBSD's bufring.h
  * Used as BSD-3 Licensed with permission from Kip Macy.
@@ -21,8 +22,7 @@ update_tail(struct rte_ring_headtail *ht, uint32_t old_val, uint32_t new_val,
 	 * we need to wait for them to complete
 	 */
 	if (!single)
-		while (unlikely(ht->tail != old_val))
-			rte_pause();
+		rte_wait_until_equal_32(&ht->tail, old_val, __ATOMIC_RELAXED);
 
 	__atomic_store_n(&ht->tail, new_val, __ATOMIC_RELEASE);
 }
diff --git a/lib/librte_ring/rte_ring_generic.h b/lib/librte_ring/rte_ring_generic.h
index 953cdbbd5..afce89aec 100644
--- a/lib/librte_ring/rte_ring_generic.h
+++ b/lib/librte_ring/rte_ring_generic.h
@@ -23,8 +23,7 @@ update_tail(struct rte_ring_headtail *ht, uint32_t old_val, uint32_t new_val,
 	 * we need to wait for them to complete
 	 */
 	if (!single)
-		while (unlikely(ht->tail != old_val))
-			rte_pause();
+		rte_wait_until_equal_32(&ht->tail, old_val, __ATOMIC_RELAXED);
 
 	ht->tail = new_val;
 }
-- 
2.17.1


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [dpdk-dev] [PATCH v1 2/2] ring: use wfe to wait for ring tail update on aarch64
  2020-04-24 13:23   ` Ananyev, Konstantin
@ 2020-04-26  8:41     ` Gavin Hu
  0 siblings, 0 replies; 36+ messages in thread
From: Gavin Hu @ 2020-04-26  8:41 UTC (permalink / raw)
  To: Ananyev, Konstantin, dev
  Cc: nd, david.marchand, thomas, jerinj, Honnappa Nagarahalli,
	Ruifeng Wang, Phil Yang, Joyce Kong, nd

Hi Konstantin,

> -----Original Message-----
> From: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Sent: Friday, April 24, 2020 9:24 PM
> To: Gavin Hu <Gavin.Hu@arm.com>; dev@dpdk.org
> Cc: nd <nd@arm.com>; david.marchand@redhat.com;
> thomas@monjalon.net; jerinj@marvell.com; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>; Ruifeng Wang
> <Ruifeng.Wang@arm.com>; Phil Yang <Phil.Yang@arm.com>; Joyce Kong
> <Joyce.Kong@arm.com>
> Subject: RE: [dpdk-dev] [PATCH v1 2/2] ring: use wfe to wait for ring tail
> update on aarch64
> 
> 
> >
> > Instead of polling for tail to be updated, use wfe instruction.
> >
> > Signed-off-by: Gavin Hu <gavin.hu@arm.com>
> > Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> > Reviewed-by: Steve Capper <steve.capper@arm.com>
> > Reviewed-by: Ola Liljedahl <ola.liljedahl@arm.com>
> > Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > ---
> >  lib/librte_ring/rte_ring_c11_mem.h | 4 ++--
> >  lib/librte_ring/rte_ring_generic.h | 3 +--
> >  2 files changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git a/lib/librte_ring/rte_ring_c11_mem.h
> b/lib/librte_ring/rte_ring_c11_mem.h
> > index 0fb73a337..764d8f186 100644
> > --- a/lib/librte_ring/rte_ring_c11_mem.h
> > +++ b/lib/librte_ring/rte_ring_c11_mem.h
> > @@ -2,6 +2,7 @@
> >   *
> >   * Copyright (c) 2017,2018 HXT-semitech Corporation.
> >   * Copyright (c) 2007-2009 Kip Macy kmacy@freebsd.org
> > + * Copyright (c) 2019 Arm Limited
> >   * All rights reserved.
> >   * Derived from FreeBSD's bufring.h
> >   * Used as BSD-3 Licensed with permission from Kip Macy.
> > @@ -21,8 +22,7 @@ update_tail(struct rte_ring_headtail *ht, uint32_t
> old_val, uint32_t new_val,
> >  	 * we need to wait for them to complete
> >  	 */
> >  	if (!single)
> > -		while (unlikely(ht->tail != old_val))
> > -			rte_pause();
> > +		rte_wait_until_equal_relaxed_32(&ht->tail, old_val);
> >
> >  	__atomic_store_n(&ht->tail, new_val, __ATOMIC_RELEASE);
> >  }
> > diff --git a/lib/librte_ring/rte_ring_generic.h
> b/lib/librte_ring/rte_ring_generic.h
> > index 953cdbbd5..682852783 100644
> > --- a/lib/librte_ring/rte_ring_generic.h
> > +++ b/lib/librte_ring/rte_ring_generic.h
> > @@ -23,8 +23,7 @@ update_tail(struct rte_ring_headtail *ht, uint32_t
> old_val, uint32_t new_val,
> >  	 * we need to wait for them to complete
> >  	 */
> >  	if (!single)
> > -		while (unlikely(ht->tail != old_val))
> > -			rte_pause();
> > +		rte_wait_until_equal_relaxed_32(&ht->tail, old_val);
> 
> ./lib/librte_ring/rte_ring_generic.h:26:3: error: implicit declaration of
> function 'rte_wait_until_equal_relaxed_32'; did you mean
> 'rte_wait_until_equal_32'? [-Werror=implicit-function-declaration]
>    rte_wait_until_equal_relaxed_32(&ht->tail, old_val);
>    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Sorry for the leakage, fixed in v2.
> 
> BTW, after patch #1 compilation fails also:
> ../lib/librte_eal/x86/include/rte_spinlock.h:125:24: error: pointer targets in
> passing argument 1 of 'rte_try_tm' differ in signedness [-Werror=pointer-
> sign]
>   if (likely(rte_try_tm(&sl->locked)))
Fixed in v2, thanks!
>                         ^
> 
> 
> 
> >
> >  	ht->tail = new_val;
> >  }
> > --
> > 2.17.1


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [dpdk-dev] [PATCH v2 1/2] spinlock: use wfe to reduce contention on aarch64
  2020-04-26  8:39 ` [dpdk-dev] [PATCH v2 1/2] spinlock: use wfe to reduce contention on aarch64 Gavin Hu
@ 2020-04-28 12:22   ` Jerin Jacob
  0 siblings, 0 replies; 36+ messages in thread
From: Jerin Jacob @ 2020-04-28 12:22 UTC (permalink / raw)
  To: Gavin Hu
  Cc: dpdk-dev, nd, David Marchand, Thomas Monjalon, Jerin Jacob,
	Honnappa Nagarahalli, Ruifeng Wang (Arm Technology China),
	Phil Yang, Joyce Kong

On Sun, Apr 26, 2020 at 2:09 PM Gavin Hu <gavin.hu@arm.com> wrote:
>
> In acquiring a spinlock, cores repeatedly poll the lock variable.
> This is replaced by rte_wait_until_equal API.
>
> Running the micro benchmarking and the testpmd and l3fwd traffic tests
> on ThunderX2, Ampere eMAG80 and Arm N1SDP, everything went well and no
> notable performance gain nor degradation was measured.
>
> Signed-off-by: Gavin Hu <gavin.hu@arm.com>
> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> Reviewed-by: Phil Yang <phil.yang@arm.com>
> Reviewed-by: Steve Capper <steve.capper@arm.com>
> Reviewed-by: Ola Liljedahl <ola.liljedahl@arm.com>
> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Tested-by: Pavan Nikhilesh <pbhagavatula@marvell.com>

Acked-by: Jerin Jacob <jerinj@marvell.com>


> ---
>  lib/librte_eal/include/generic/rte_spinlock.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/lib/librte_eal/include/generic/rte_spinlock.h b/lib/librte_eal/include/generic/rte_spinlock.h
> index 87ae7a4f1..40fe49d5a 100644
> --- a/lib/librte_eal/include/generic/rte_spinlock.h
> +++ b/lib/librte_eal/include/generic/rte_spinlock.h
> @@ -65,8 +65,8 @@ rte_spinlock_lock(rte_spinlock_t *sl)
>
>         while (!__atomic_compare_exchange_n(&sl->locked, &exp, 1, 0,
>                                 __ATOMIC_ACQUIRE, __ATOMIC_RELAXED)) {
> -               while (__atomic_load_n(&sl->locked, __ATOMIC_RELAXED))
> -                       rte_pause();
> +               rte_wait_until_equal_32((volatile uint32_t *)&sl->locked,
> +                              0, __ATOMIC_RELAXED);
>                 exp = 0;
>         }
>  }
> --
> 2.17.1
>

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [dpdk-dev] [PATCH v2 0/2] Use WFE for spinlock and ring
  2020-04-26  8:39 ` [dpdk-dev] [PATCH v2 0/2] Use WFE for spinlock and ring Gavin Hu
@ 2020-05-01  9:47   ` Ananyev, Konstantin
  2020-05-03 14:54   ` David Marchand
  1 sibling, 0 replies; 36+ messages in thread
From: Ananyev, Konstantin @ 2020-05-01  9:47 UTC (permalink / raw)
  To: Gavin Hu, dev
  Cc: nd, david.marchand, thomas, jerinj, Honnappa.Nagarahalli,
	Ruifeng.Wang, Phil.Yang, Joyce.Kong

> 
> The rte_wait_until_equal_xxx APIs abstract the functionality of 'polling
> for a memory location to become equal to a given value'[1].
> 
> Use the API for the rte spinlock and ring implementations.
> 
> [1] http://patches.dpdk.org/cover/62703/
> 
> Gavin Hu (2):
>   spinlock: use wfe to reduce contention on aarch64
>   ring: use wfe to wait for ring tail update on aarch64
> 
>  lib/librte_eal/include/generic/rte_spinlock.h | 4 ++--
>  lib/librte_ring/rte_ring_c11_mem.h            | 4 ++--
>  lib/librte_ring/rte_ring_generic.h            | 3 +--
>  3 files changed, 5 insertions(+), 6 deletions(-)
> 

Series Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

> --
> 2.17.1


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [dpdk-dev] [PATCH v2 0/2] Use WFE for spinlock and ring
  2020-04-26  8:39 ` [dpdk-dev] [PATCH v2 0/2] Use WFE for spinlock and ring Gavin Hu
  2020-05-01  9:47   ` Ananyev, Konstantin
@ 2020-05-03 14:54   ` David Marchand
  2020-05-03 21:31     ` Honnappa Nagarahalli
  1 sibling, 1 reply; 36+ messages in thread
From: David Marchand @ 2020-05-03 14:54 UTC (permalink / raw)
  To: Gavin Hu
  Cc: dev, nd, Thomas Monjalon, Jerin Jacob Kollanukkaran,
	Honnappa Nagarahalli, Ruifeng Wang (Arm Technology China),
	Phil Yang, Joyce Kong, Ananyev, Konstantin

On Sun, Apr 26, 2020 at 10:39 AM Gavin Hu <gavin.hu@arm.com> wrote:
>
> The rte_wait_until_equal_xxx APIs abstract the functionality of 'polling
> for a memory location to become equal to a given value'[1].
>
> Use the API for the rte spinlock and ring implementations.
>
> [1] http://patches.dpdk.org/cover/62703/
>
> Gavin Hu (2):
>   spinlock: use wfe to reduce contention on aarch64
>   ring: use wfe to wait for ring tail update on aarch64

This would result in rte_ring and rte_spinlock APIs becoming
experimental and this breaks compilation for external applications
using stable ring and spinlock APIs.
IIRC, it was the reason why these patches were dropped with the
introduction of the rte_wait_until_equal_* API.


-- 
David Marchand


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [dpdk-dev] [PATCH v2 0/2] Use WFE for spinlock and ring
  2020-05-03 14:54   ` David Marchand
@ 2020-05-03 21:31     ` Honnappa Nagarahalli
  2020-09-25 14:30       ` David Marchand
  0 siblings, 1 reply; 36+ messages in thread
From: Honnappa Nagarahalli @ 2020-05-03 21:31 UTC (permalink / raw)
  To: David Marchand
  Cc: dev, nd, thomas, jerinj, Ruifeng Wang, Phil Yang, Joyce Kong,
	Ananyev, Konstantin, Honnappa Nagarahalli, nd, nd

<snip>

> Subject: Re: [PATCH v2 0/2] Use WFE for spinlock and ring
> 
> On Sun, Apr 26, 2020 at 10:39 AM Gavin Hu <gavin.hu@arm.com> wrote:
> >
> > The rte_wait_until_equal_xxx APIs abstract the functionality of
> > 'polling for a memory location to become equal to a given value'[1].
> >
> > Use the API for the rte spinlock and ring implementations.
> >
> > [1] http://patches.dpdk.org/cover/62703/
> >
> > Gavin Hu (2):
> >   spinlock: use wfe to reduce contention on aarch64
> >   ring: use wfe to wait for ring tail update on aarch64
> 
> This would result in rte_ring and rte_spinlock APIs becoming experimental
> and this breaks compilation for external applications using stable ring and
> spinlock APIs.
> IIRC, it was the reason why these patches were dropped with the introduction
> of the rte_wait_until_equal_* API.
Agreed, the rte_ring new sync modes are resulting in different use cases for these APIs. We need to take a relook at the APIs.

> 
> 
> --
> David Marchand


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [dpdk-dev] [PATCH v2 0/2] Use WFE for spinlock and ring
  2020-05-03 21:31     ` Honnappa Nagarahalli
@ 2020-09-25 14:30       ` David Marchand
  2021-03-25 14:58         ` David Marchand
  0 siblings, 1 reply; 36+ messages in thread
From: David Marchand @ 2020-09-25 14:30 UTC (permalink / raw)
  To: Honnappa Nagarahalli
  Cc: dev, nd, thomas, jerinj, Ruifeng Wang, Phil Yang, Joyce Kong,
	Ananyev, Konstantin

Hello Honnappa,

On Sun, May 3, 2020 at 11:32 PM Honnappa Nagarahalli
<Honnappa.Nagarahalli@arm.com> wrote:
>
> <snip>
>
> > Subject: Re: [PATCH v2 0/2] Use WFE for spinlock and ring
> >
> > On Sun, Apr 26, 2020 at 10:39 AM Gavin Hu <gavin.hu@arm.com> wrote:
> > >
> > > The rte_wait_until_equal_xxx APIs abstract the functionality of
> > > 'polling for a memory location to become equal to a given value'[1].
> > >
> > > Use the API for the rte spinlock and ring implementations.
> > >
> > > [1] http://patches.dpdk.org/cover/62703/
> > >
> > > Gavin Hu (2):
> > >   spinlock: use wfe to reduce contention on aarch64
> > >   ring: use wfe to wait for ring tail update on aarch64
> >
> > This would result in rte_ring and rte_spinlock APIs becoming experimental
> > and this breaks compilation for external applications using stable ring and
> > spinlock APIs.
> > IIRC, it was the reason why these patches were dropped with the introduction
> > of the rte_wait_until_equal_* API.
> Agreed, the rte_ring new sync modes are resulting in different use cases for these APIs. We need to take a relook at the APIs.

Will we reconsider this series now that the wait until equal is going stable?


-- 
David Marchand


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [dpdk-dev] [PATCH v2 0/2] Use WFE for spinlock and ring
  2020-09-25 14:30       ` David Marchand
@ 2021-03-25 14:58         ` David Marchand
  2021-04-25  6:06           ` Ruifeng Wang
  0 siblings, 1 reply; 36+ messages in thread
From: David Marchand @ 2021-03-25 14:58 UTC (permalink / raw)
  To: Honnappa Nagarahalli
  Cc: dev, nd, thomas, jerinj, Ruifeng Wang, Phil Yang, Joyce Kong,
	Ananyev, Konstantin

Hello,

On Fri, Sep 25, 2020 at 4:30 PM David Marchand
<david.marchand@redhat.com> wrote:
>
> Hello Honnappa,
>
> On Sun, May 3, 2020 at 11:32 PM Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com> wrote:
> >
> > <snip>
> >
> > > Subject: Re: [PATCH v2 0/2] Use WFE for spinlock and ring
> > >
> > > On Sun, Apr 26, 2020 at 10:39 AM Gavin Hu <gavin.hu@arm.com> wrote:
> > > >
> > > > The rte_wait_until_equal_xxx APIs abstract the functionality of
> > > > 'polling for a memory location to become equal to a given value'[1].
> > > >
> > > > Use the API for the rte spinlock and ring implementations.
> > > >
> > > > [1] http://patches.dpdk.org/cover/62703/
> > > >
> > > > Gavin Hu (2):
> > > >   spinlock: use wfe to reduce contention on aarch64
> > > >   ring: use wfe to wait for ring tail update on aarch64
> > >
> > > This would result in rte_ring and rte_spinlock APIs becoming experimental
> > > and this breaks compilation for external applications using stable ring and
> > > spinlock APIs.
> > > IIRC, it was the reason why these patches were dropped with the introduction
> > > of the rte_wait_until_equal_* API.
> > Agreed, the rte_ring new sync modes are resulting in different use cases for these APIs. We need to take a relook at the APIs.
>
> Will we reconsider this series now that the wait until equal is going stable?

Any update?
I suppose this would need some rebasing after the ring library changes.

Thanks.


-- 
David Marchand


^ permalink raw reply	[flat|nested] 36+ messages in thread

* [dpdk-dev] Use WFE for spinlock and ring
  2020-04-24  7:07 [dpdk-dev] [PATCH v1 0/2] Use WFE for spinlock and ring Gavin Hu
                   ` (4 preceding siblings ...)
  2020-04-26  8:39 ` [dpdk-dev] [PATCH v2 2/2] ring: use wfe to wait for ring tail update " Gavin Hu
@ 2021-04-25  5:56 ` Ruifeng Wang
  2021-04-25  5:56   ` [dpdk-dev] [PATCH v3 1/2] spinlock: use wfe to reduce contention on aarch64 Ruifeng Wang
                     ` (2 more replies)
  5 siblings, 3 replies; 36+ messages in thread
From: Ruifeng Wang @ 2021-04-25  5:56 UTC (permalink / raw)
  Cc: dev, david.marchand, thomas, jerinj, nd, honnappa.nagarahalli,
	ruifeng.wang

The rte_wait_until_equal_xxx APIs abstract the functionality of 'polling
for a memory location to become equal to a given value'[1].

Use the API for the rte spinlock and ring implementations.
With the wait until equal APIs being stable, changes will not impact ABI.

[1] http://patches.dpdk.org/cover/62703/

v3:
Series rebased. (David)

Gavin Hu (1):
  spinlock: use wfe to reduce contention on aarch64

Ruifeng Wang (1):
  ring: use wfe to wait for ring tail update on aarch64

 lib/eal/include/generic/rte_spinlock.h | 4 ++--
 lib/ring/rte_ring_c11_pvt.h            | 4 ++--
 lib/ring/rte_ring_generic_pvt.h        | 3 +--
 3 files changed, 5 insertions(+), 6 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 36+ messages in thread

* [dpdk-dev] [PATCH v3 1/2] spinlock: use wfe to reduce contention on aarch64
  2021-04-25  5:56 ` [dpdk-dev] Use WFE for spinlock and ring Ruifeng Wang
@ 2021-04-25  5:56   ` Ruifeng Wang
  2021-04-25  5:56   ` [dpdk-dev] [PATCH v3 2/2] ring: use wfe to wait for ring tail update " Ruifeng Wang
  2021-04-28  7:42   ` [dpdk-dev] Use WFE for spinlock and ring David Marchand
  2 siblings, 0 replies; 36+ messages in thread
From: Ruifeng Wang @ 2021-04-25  5:56 UTC (permalink / raw)
  Cc: dev, david.marchand, thomas, jerinj, nd, honnappa.nagarahalli,
	ruifeng.wang, Gavin Hu, Phil Yang, Steve Capper, Ola Liljedahl,
	Pavan Nikhilesh, Konstantin Ananyev

From: Gavin Hu <gavin.hu@arm.com>

In acquiring a spinlock, cores repeatedly poll the lock variable.
This is replaced by rte_wait_until_equal API.

Running the micro benchmarking and the testpmd and l3fwd traffic tests
on ThunderX2, Ampere eMAG80 and Arm N1SDP, everything went well and no
notable performance gain nor degradation was measured.

Signed-off-by: Gavin Hu <gavin.hu@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
Reviewed-by: Phil Yang <phil.yang@arm.com>
Reviewed-by: Steve Capper <steve.capper@arm.com>
Reviewed-by: Ola Liljedahl <ola.liljedahl@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Tested-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
Acked-by: Jerin Jacob <jerinj@marvell.com>
---
 lib/eal/include/generic/rte_spinlock.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/eal/include/generic/rte_spinlock.h b/lib/eal/include/generic/rte_spinlock.h
index 87ae7a4f18..40fe49d5ad 100644
--- a/lib/eal/include/generic/rte_spinlock.h
+++ b/lib/eal/include/generic/rte_spinlock.h
@@ -65,8 +65,8 @@ rte_spinlock_lock(rte_spinlock_t *sl)
 
 	while (!__atomic_compare_exchange_n(&sl->locked, &exp, 1, 0,
 				__ATOMIC_ACQUIRE, __ATOMIC_RELAXED)) {
-		while (__atomic_load_n(&sl->locked, __ATOMIC_RELAXED))
-			rte_pause();
+		rte_wait_until_equal_32((volatile uint32_t *)&sl->locked,
+			       0, __ATOMIC_RELAXED);
 		exp = 0;
 	}
 }
-- 
2.25.1


^ permalink raw reply	[flat|nested] 36+ messages in thread

* [dpdk-dev] [PATCH v3 2/2] ring: use wfe to wait for ring tail update on aarch64
  2021-04-25  5:56 ` [dpdk-dev] Use WFE for spinlock and ring Ruifeng Wang
  2021-04-25  5:56   ` [dpdk-dev] [PATCH v3 1/2] spinlock: use wfe to reduce contention on aarch64 Ruifeng Wang
@ 2021-04-25  5:56   ` Ruifeng Wang
  2021-04-26  5:38     ` Jerin Jacob
  2021-04-28 17:17     ` Stephen Hemminger
  2021-04-28  7:42   ` [dpdk-dev] Use WFE for spinlock and ring David Marchand
  2 siblings, 2 replies; 36+ messages in thread
From: Ruifeng Wang @ 2021-04-25  5:56 UTC (permalink / raw)
  To: Honnappa Nagarahalli, Konstantin Ananyev
  Cc: dev, david.marchand, thomas, jerinj, nd, ruifeng.wang, Gavin Hu,
	Steve Capper, Ola Liljedahl

Instead of polling for tail to be updated, use wfe instruction.

Signed-off-by: Gavin Hu <gavin.hu@arm.com>
Signed-off-by: Ruifeng Wang <ruifeng.wang@arm.com>
Reviewed-by: Steve Capper <steve.capper@arm.com>
Reviewed-by: Ola Liljedahl <ola.liljedahl@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
---
 lib/ring/rte_ring_c11_pvt.h     | 4 ++--
 lib/ring/rte_ring_generic_pvt.h | 3 +--
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/lib/ring/rte_ring_c11_pvt.h b/lib/ring/rte_ring_c11_pvt.h
index 759192f4c4..37e0b2afd6 100644
--- a/lib/ring/rte_ring_c11_pvt.h
+++ b/lib/ring/rte_ring_c11_pvt.h
@@ -2,6 +2,7 @@
  *
  * Copyright (c) 2017,2018 HXT-semitech Corporation.
  * Copyright (c) 2007-2009 Kip Macy kmacy@freebsd.org
+ * Copyright (c) 2021 Arm Limited
  * All rights reserved.
  * Derived from FreeBSD's bufring.h
  * Used as BSD-3 Licensed with permission from Kip Macy.
@@ -21,8 +22,7 @@ __rte_ring_update_tail(struct rte_ring_headtail *ht, uint32_t old_val,
 	 * we need to wait for them to complete
 	 */
 	if (!single)
-		while (unlikely(ht->tail != old_val))
-			rte_pause();
+		rte_wait_until_equal_32(&ht->tail, old_val, __ATOMIC_RELAXED);
 
 	__atomic_store_n(&ht->tail, new_val, __ATOMIC_RELEASE);
 }
diff --git a/lib/ring/rte_ring_generic_pvt.h b/lib/ring/rte_ring_generic_pvt.h
index 532deb5e7a..c95ad7e12c 100644
--- a/lib/ring/rte_ring_generic_pvt.h
+++ b/lib/ring/rte_ring_generic_pvt.h
@@ -23,8 +23,7 @@ __rte_ring_update_tail(struct rte_ring_headtail *ht, uint32_t old_val,
 	 * we need to wait for them to complete
 	 */
 	if (!single)
-		while (unlikely(ht->tail != old_val))
-			rte_pause();
+		rte_wait_until_equal_32(&ht->tail, old_val, __ATOMIC_RELAXED);
 
 	ht->tail = new_val;
 }
-- 
2.25.1


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [dpdk-dev] [PATCH v2 0/2] Use WFE for spinlock and ring
  2021-03-25 14:58         ` David Marchand
@ 2021-04-25  6:06           ` Ruifeng Wang
  2021-04-25 15:56             ` Thomas Monjalon
  0 siblings, 1 reply; 36+ messages in thread
From: Ruifeng Wang @ 2021-04-25  6:06 UTC (permalink / raw)
  To: David Marchand, Honnappa Nagarahalli
  Cc: dev, nd, thomas, jerinj, Joyce Kong, Ananyev, Konstantin, nd

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Thursday, March 25, 2021 10:58 PM
> To: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> Cc: dev <dev@dpdk.org>; nd <nd@arm.com>; thomas@monjalon.net;
> jerinj@marvell.com; Ruifeng Wang <Ruifeng.Wang@arm.com>; Phil Yang
> <Phil.Yang@arm.com>; Joyce Kong <Joyce.Kong@arm.com>; Ananyev,
> Konstantin <konstantin.ananyev@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v2 0/2] Use WFE for spinlock and ring
> 
> Hello,
> 
> On Fri, Sep 25, 2020 at 4:30 PM David Marchand
> <david.marchand@redhat.com> wrote:
> >
> > Hello Honnappa,
> >
> > On Sun, May 3, 2020 at 11:32 PM Honnappa Nagarahalli
> > <Honnappa.Nagarahalli@arm.com> wrote:
> > >
> > > <snip>
> > >
> > > > Subject: Re: [PATCH v2 0/2] Use WFE for spinlock and ring
> > > >
> > > > On Sun, Apr 26, 2020 at 10:39 AM Gavin Hu <gavin.hu@arm.com> wrote:
> > > > >
> > > > > The rte_wait_until_equal_xxx APIs abstract the functionality of
> > > > > 'polling for a memory location to become equal to a given value'[1].
> > > > >
> > > > > Use the API for the rte spinlock and ring implementations.
> > > > >
> > > > > [1] http://patches.dpdk.org/cover/62703/
> > > > >
> > > > > Gavin Hu (2):
> > > > >   spinlock: use wfe to reduce contention on aarch64
> > > > >   ring: use wfe to wait for ring tail update on aarch64
> > > >
> > > > This would result in rte_ring and rte_spinlock APIs becoming
> > > > experimental and this breaks compilation for external applications
> > > > using stable ring and spinlock APIs.
> > > > IIRC, it was the reason why these patches were dropped with the
> > > > introduction of the rte_wait_until_equal_* API.
> > > Agreed, the rte_ring new sync modes are resulting in different use cases
> for these APIs. We need to take a relook at the APIs.
> >
> > Will we reconsider this series now that the wait until equal is going stable?
> 
> Any update?
> I suppose this would need some rebasing after the ring library changes.

Sorry for the late response.
This series has been rebased and v3 posted.
I have problem to mark v2 as superseded.

Thank you.
> 
> Thanks.
> 
> 
> --
> David Marchand


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [dpdk-dev] [PATCH v2 0/2] Use WFE for spinlock and ring
  2021-04-25  6:06           ` Ruifeng Wang
@ 2021-04-25 15:56             ` Thomas Monjalon
  0 siblings, 0 replies; 36+ messages in thread
From: Thomas Monjalon @ 2021-04-25 15:56 UTC (permalink / raw)
  To: Ruifeng Wang, David Marchand, Honnappa Nagarahalli
  Cc: dev, nd, Jerin Jacob Kollanukkaran, Joyce Kong, Konstantin Ananyev

Ruifeng Wang:
> From: David Marchand <david.marchand@redhat.com>
> > Any update?
> > I suppose this would need some rebasing after the ring library changes.
> 
> Sorry for the late response.
> This series has been rebased and v3 posted.
> I have problem to mark v2 as superseded.

I did change v2 as superseded.

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [dpdk-dev] [PATCH v3 2/2] ring: use wfe to wait for ring tail update on aarch64
  2021-04-25  5:56   ` [dpdk-dev] [PATCH v3 2/2] ring: use wfe to wait for ring tail update " Ruifeng Wang
@ 2021-04-26  5:38     ` Jerin Jacob
  2021-04-28 17:17     ` Stephen Hemminger
  1 sibling, 0 replies; 36+ messages in thread
From: Jerin Jacob @ 2021-04-26  5:38 UTC (permalink / raw)
  To: Ruifeng Wang
  Cc: Honnappa Nagarahalli, Konstantin Ananyev, dpdk-dev,
	David Marchand, Thomas Monjalon, Jerin Jacob, nd, Gavin Hu,
	Steve Capper, Ola Liljedahl

On Sun, Apr 25, 2021 at 11:27 AM Ruifeng Wang <ruifeng.wang@arm.com> wrote:
>
> Instead of polling for tail to be updated, use wfe instruction.
>
> Signed-off-by: Gavin Hu <gavin.hu@arm.com>
> Signed-off-by: Ruifeng Wang <ruifeng.wang@arm.com>
> Reviewed-by: Steve Capper <steve.capper@arm.com>
> Reviewed-by: Ola Liljedahl <ola.liljedahl@arm.com>
> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

Acked-by: Jerin Jacob <jerinj@marvell.com>



> ---
>  lib/ring/rte_ring_c11_pvt.h     | 4 ++--
>  lib/ring/rte_ring_generic_pvt.h | 3 +--
>  2 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/lib/ring/rte_ring_c11_pvt.h b/lib/ring/rte_ring_c11_pvt.h
> index 759192f4c4..37e0b2afd6 100644
> --- a/lib/ring/rte_ring_c11_pvt.h
> +++ b/lib/ring/rte_ring_c11_pvt.h
> @@ -2,6 +2,7 @@
>   *
>   * Copyright (c) 2017,2018 HXT-semitech Corporation.
>   * Copyright (c) 2007-2009 Kip Macy kmacy@freebsd.org
> + * Copyright (c) 2021 Arm Limited
>   * All rights reserved.
>   * Derived from FreeBSD's bufring.h
>   * Used as BSD-3 Licensed with permission from Kip Macy.
> @@ -21,8 +22,7 @@ __rte_ring_update_tail(struct rte_ring_headtail *ht, uint32_t old_val,
>          * we need to wait for them to complete
>          */
>         if (!single)
> -               while (unlikely(ht->tail != old_val))
> -                       rte_pause();
> +               rte_wait_until_equal_32(&ht->tail, old_val, __ATOMIC_RELAXED);
>
>         __atomic_store_n(&ht->tail, new_val, __ATOMIC_RELEASE);
>  }
> diff --git a/lib/ring/rte_ring_generic_pvt.h b/lib/ring/rte_ring_generic_pvt.h
> index 532deb5e7a..c95ad7e12c 100644
> --- a/lib/ring/rte_ring_generic_pvt.h
> +++ b/lib/ring/rte_ring_generic_pvt.h
> @@ -23,8 +23,7 @@ __rte_ring_update_tail(struct rte_ring_headtail *ht, uint32_t old_val,
>          * we need to wait for them to complete
>          */
>         if (!single)
> -               while (unlikely(ht->tail != old_val))
> -                       rte_pause();
> +               rte_wait_until_equal_32(&ht->tail, old_val, __ATOMIC_RELAXED);
>
>         ht->tail = new_val;
>  }
> --
> 2.25.1
>

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [dpdk-dev] Use WFE for spinlock and ring
  2021-04-25  5:56 ` [dpdk-dev] Use WFE for spinlock and ring Ruifeng Wang
  2021-04-25  5:56   ` [dpdk-dev] [PATCH v3 1/2] spinlock: use wfe to reduce contention on aarch64 Ruifeng Wang
  2021-04-25  5:56   ` [dpdk-dev] [PATCH v3 2/2] ring: use wfe to wait for ring tail update " Ruifeng Wang
@ 2021-04-28  7:42   ` David Marchand
  2021-04-28  9:30     ` Ruifeng Wang
  2 siblings, 1 reply; 36+ messages in thread
From: David Marchand @ 2021-04-28  7:42 UTC (permalink / raw)
  To: Ruifeng Wang
  Cc: dev, Thomas Monjalon, Jerin Jacob Kollanukkaran, nd,
	Honnappa Nagarahalli

Hello Ruifeng,

On Sun, Apr 25, 2021 at 7:57 AM Ruifeng Wang <ruifeng.wang@arm.com> wrote:
>
> The rte_wait_until_equal_xxx APIs abstract the functionality of 'polling
> for a memory location to become equal to a given value'[1].
>
> Use the API for the rte spinlock and ring implementations.
> With the wait until equal APIs being stable, changes will not impact ABI.

Afaics, there is no ARM target with WFE enabled and we lost ability to
enable WFE support with removal of the make build system.

$ git grep RTE_ARM_USE_WFE
config/arm/meson.build:        ['RTE_ARM_USE_WFE', false],
lib/eal/arm/include/rte_pause_64.h:#ifdef RTE_ARM_USE_WFE

How did you enable WFE to test this series?


-- 
David Marchand


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [dpdk-dev] Use WFE for spinlock and ring
  2021-04-28  7:42   ` [dpdk-dev] Use WFE for spinlock and ring David Marchand
@ 2021-04-28  9:30     ` Ruifeng Wang
  2021-04-28 11:13       ` Thomas Monjalon
  0 siblings, 1 reply; 36+ messages in thread
From: Ruifeng Wang @ 2021-04-28  9:30 UTC (permalink / raw)
  To: David Marchand; +Cc: dev, thomas, jerinj, nd, Honnappa Nagarahalli, nd

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Wednesday, April 28, 2021 3:42 PM
> To: Ruifeng Wang <Ruifeng.Wang@arm.com>
> Cc: dev <dev@dpdk.org>; thomas@monjalon.net; jerinj@marvell.com; nd
> <nd@arm.com>; Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> Subject: Re: Use WFE for spinlock and ring
> 
> Hello Ruifeng,

Hello David,
> 
> On Sun, Apr 25, 2021 at 7:57 AM Ruifeng Wang <ruifeng.wang@arm.com>
> wrote:
> >
> > The rte_wait_until_equal_xxx APIs abstract the functionality of
> > 'polling for a memory location to become equal to a given value'[1].
> >
> > Use the API for the rte spinlock and ring implementations.
> > With the wait until equal APIs being stable, changes will not impact ABI.
> 
> Afaics, there is no ARM target with WFE enabled and we lost ability to enable
> WFE support with removal of the make build system.

WFE can be enabled with direct meson file change.
WFE is not intended to be enabled by default. It can be enabled based on benchmarking
result on hardware.
> 
> $ git grep RTE_ARM_USE_WFE
> config/arm/meson.build:        ['RTE_ARM_USE_WFE', false],
> lib/eal/arm/include/rte_pause_64.h:#ifdef RTE_ARM_USE_WFE
> 
> How did you enable WFE to test this series?

I modified meson file to test.
Tests were also done with WFE disabled to make sure no degradation with generic implementation.
> 
> 
> --
> David Marchand


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [dpdk-dev] Use WFE for spinlock and ring
  2021-04-28  9:30     ` Ruifeng Wang
@ 2021-04-28 11:13       ` Thomas Monjalon
  2021-04-29 14:28         ` Ruifeng Wang
  0 siblings, 1 reply; 36+ messages in thread
From: Thomas Monjalon @ 2021-04-28 11:13 UTC (permalink / raw)
  To: Ruifeng Wang; +Cc: David Marchand, dev, jerinj, nd, Honnappa Nagarahalli

28/04/2021 11:30, Ruifeng Wang:
> From: David Marchand <david.marchand@redhat.com>
> > On Sun, Apr 25, 2021 at 7:57 AM Ruifeng Wang <ruifeng.wang@arm.com>
> > wrote:
> > >
> > > The rte_wait_until_equal_xxx APIs abstract the functionality of
> > > 'polling for a memory location to become equal to a given value'[1].
> > >
> > > Use the API for the rte spinlock and ring implementations.
> > > With the wait until equal APIs being stable, changes will not impact ABI.
> > 
> > Afaics, there is no ARM target with WFE enabled and we lost ability to enable
> > WFE support with removal of the make build system.
> 
> WFE can be enabled with direct meson file change.
> WFE is not intended to be enabled by default. It can be enabled based on benchmarking
> result on hardware.
> > 
> > $ git grep RTE_ARM_USE_WFE
> > config/arm/meson.build:        ['RTE_ARM_USE_WFE', false],
> > lib/eal/arm/include/rte_pause_64.h:#ifdef RTE_ARM_USE_WFE
> > 
> > How did you enable WFE to test this series?
> 
> I modified meson file to test.
> Tests were also done with WFE disabled to make sure no degradation with generic implementation.

I don't understand the usage.
Which platform should use it?
Should it be a compile-time option?



^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [dpdk-dev] [PATCH v3 2/2] ring: use wfe to wait for ring tail update on aarch64
  2021-04-25  5:56   ` [dpdk-dev] [PATCH v3 2/2] ring: use wfe to wait for ring tail update " Ruifeng Wang
  2021-04-26  5:38     ` Jerin Jacob
@ 2021-04-28 17:17     ` Stephen Hemminger
  2021-04-29 14:35       ` Ruifeng Wang
  1 sibling, 1 reply; 36+ messages in thread
From: Stephen Hemminger @ 2021-04-28 17:17 UTC (permalink / raw)
  To: Ruifeng Wang
  Cc: Honnappa Nagarahalli, Konstantin Ananyev, dev, david.marchand,
	thomas, jerinj, nd, Gavin Hu, Steve Capper, Ola Liljedahl

On Sun, 25 Apr 2021 05:56:53 +0000
Ruifeng Wang <ruifeng.wang@arm.com> wrote:

> Instead of polling for tail to be updated, use wfe instruction.
> 
> Signed-off-by: Gavin Hu <gavin.hu@arm.com>
> Signed-off-by: Ruifeng Wang <ruifeng.wang@arm.com>
> Reviewed-by: Steve Capper <steve.capper@arm.com>
> Reviewed-by: Ola Liljedahl <ola.liljedahl@arm.com>
> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

Looks ok to me, but it does raise an interesting question.
Shouldn't the original code have been using atomic load to look at ht->tail.

This another place where "volatile considered harmful" applies.

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [dpdk-dev] Use WFE for spinlock and ring
  2021-04-28 11:13       ` Thomas Monjalon
@ 2021-04-29 14:28         ` Ruifeng Wang
  2021-04-29 15:20           ` Thomas Monjalon
  0 siblings, 1 reply; 36+ messages in thread
From: Ruifeng Wang @ 2021-04-29 14:28 UTC (permalink / raw)
  To: thomas; +Cc: David Marchand, dev, jerinj, nd, Honnappa Nagarahalli, nd

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Wednesday, April 28, 2021 7:14 PM
> To: Ruifeng Wang <Ruifeng.Wang@arm.com>
> Cc: David Marchand <david.marchand@redhat.com>; dev <dev@dpdk.org>;
> jerinj@marvell.com; nd <nd@arm.com>; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>
> Subject: Re: Use WFE for spinlock and ring
> 
> 28/04/2021 11:30, Ruifeng Wang:
> > From: David Marchand <david.marchand@redhat.com>
> > > On Sun, Apr 25, 2021 at 7:57 AM Ruifeng Wang <ruifeng.wang@arm.com>
> > > wrote:
> > > >
> > > > The rte_wait_until_equal_xxx APIs abstract the functionality of
> > > > 'polling for a memory location to become equal to a given value'[1].
> > > >
> > > > Use the API for the rte spinlock and ring implementations.
> > > > With the wait until equal APIs being stable, changes will not impact ABI.
> > >
> > > Afaics, there is no ARM target with WFE enabled and we lost ability
> > > to enable WFE support with removal of the make build system.
> >
> > WFE can be enabled with direct meson file change.
> > WFE is not intended to be enabled by default. It can be enabled based
> > on benchmarking result on hardware.
> > >
> > > $ git grep RTE_ARM_USE_WFE
> > > config/arm/meson.build:        ['RTE_ARM_USE_WFE', false],
> > > lib/eal/arm/include/rte_pause_64.h:#ifdef RTE_ARM_USE_WFE
> > >
> > > How did you enable WFE to test this series?
> >
> > I modified meson file to test.
> > Tests were also done with WFE disabled to make sure no degradation with
> generic implementation.
> 
> I don't understand the usage.
> Which platform should use it?

Platforms that implement WFE semantic (e.g. N1) can use.
The user can enable this feature for power efficiency purpose. But there is something to
note as described in commit message 1be7855d77 when the API was introduced. 

> Should it be a compile-time option?

Yes, it should be a compile-time option.
It can be configured via c_args meson option?
> 


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [dpdk-dev] [PATCH v3 2/2] ring: use wfe to wait for ring tail update on aarch64
  2021-04-28 17:17     ` Stephen Hemminger
@ 2021-04-29 14:35       ` Ruifeng Wang
  2021-04-29 15:05         ` Stephen Hemminger
  0 siblings, 1 reply; 36+ messages in thread
From: Ruifeng Wang @ 2021-04-29 14:35 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Honnappa Nagarahalli, Konstantin Ananyev, dev, david.marchand,
	thomas, jerinj, nd, Gavin Hu, Steve Capper, Ola Liljedahl, nd

> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Thursday, April 29, 2021 1:17 AM
> To: Ruifeng Wang <Ruifeng.Wang@arm.com>
> Cc: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>; Konstantin
> Ananyev <konstantin.ananyev@intel.com>; dev@dpdk.org;
> david.marchand@redhat.com; thomas@monjalon.net; jerinj@marvell.com;
> nd <nd@arm.com>; Gavin Hu <Gavin.Hu@arm.com>; Steve Capper
> <Steve.Capper@arm.com>; Ola Liljedahl <Ola.Liljedahl@arm.com>
> Subject: Re: [dpdk-dev] [PATCH v3 2/2] ring: use wfe to wait for ring tail
> update on aarch64
> 
> On Sun, 25 Apr 2021 05:56:53 +0000
> Ruifeng Wang <ruifeng.wang@arm.com> wrote:
> 
> > Instead of polling for tail to be updated, use wfe instruction.
> >
> > Signed-off-by: Gavin Hu <gavin.hu@arm.com>
> > Signed-off-by: Ruifeng Wang <ruifeng.wang@arm.com>
> > Reviewed-by: Steve Capper <steve.capper@arm.com>
> > Reviewed-by: Ola Liljedahl <ola.liljedahl@arm.com>
> > Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> 
> Looks ok to me, but it does raise an interesting question.
> Shouldn't the original code have been using atomic load to look at ht->tail.
> 
> This another place where "volatile considered harmful" applies.

Do you mean 'volatile' should be removed from rte_wait_until_equal_xxx parameters?


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [dpdk-dev] [PATCH v3 2/2] ring: use wfe to wait for ring tail update on aarch64
  2021-04-29 14:35       ` Ruifeng Wang
@ 2021-04-29 15:05         ` Stephen Hemminger
  2021-05-07  8:25           ` Ruifeng Wang
  0 siblings, 1 reply; 36+ messages in thread
From: Stephen Hemminger @ 2021-04-29 15:05 UTC (permalink / raw)
  To: Ruifeng Wang
  Cc: Honnappa Nagarahalli, Konstantin Ananyev, dev, david.marchand,
	thomas, jerinj, nd, Gavin Hu, Steve Capper, Ola Liljedahl

On Thu, 29 Apr 2021 14:35:35 +0000
Ruifeng Wang <Ruifeng.Wang@arm.com> wrote:

> > -----Original Message-----
> > From: Stephen Hemminger <stephen@networkplumber.org>
> > Sent: Thursday, April 29, 2021 1:17 AM
> > To: Ruifeng Wang <Ruifeng.Wang@arm.com>
> > Cc: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>; Konstantin
> > Ananyev <konstantin.ananyev@intel.com>; dev@dpdk.org;
> > david.marchand@redhat.com; thomas@monjalon.net; jerinj@marvell.com;
> > nd <nd@arm.com>; Gavin Hu <Gavin.Hu@arm.com>; Steve Capper
> > <Steve.Capper@arm.com>; Ola Liljedahl <Ola.Liljedahl@arm.com>
> > Subject: Re: [dpdk-dev] [PATCH v3 2/2] ring: use wfe to wait for ring tail
> > update on aarch64
> > 
> > On Sun, 25 Apr 2021 05:56:53 +0000
> > Ruifeng Wang <ruifeng.wang@arm.com> wrote:
> >   
> > > Instead of polling for tail to be updated, use wfe instruction.
> > >
> > > Signed-off-by: Gavin Hu <gavin.hu@arm.com>
> > > Signed-off-by: Ruifeng Wang <ruifeng.wang@arm.com>
> > > Reviewed-by: Steve Capper <steve.capper@arm.com>
> > > Reviewed-by: Ola Liljedahl <ola.liljedahl@arm.com>
> > > Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > > Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>  
> > 
> > Looks ok to me, but it does raise an interesting question.
> > Shouldn't the original code have been using atomic load to look at ht->tail.
> > 
> > This another place where "volatile considered harmful" applies.  
> 
> Do you mean 'volatile' should be removed from rte_wait_until_equal_xxx parameters?
> 

I meant that all access to tail should be via C11 atomic builtin. At that point,
the volatile on the data structure elements does not matter.

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [dpdk-dev] Use WFE for spinlock and ring
  2021-04-29 14:28         ` Ruifeng Wang
@ 2021-04-29 15:20           ` Thomas Monjalon
  2021-04-30  9:16             ` Bruce Richardson
  0 siblings, 1 reply; 36+ messages in thread
From: Thomas Monjalon @ 2021-04-29 15:20 UTC (permalink / raw)
  To: Ruifeng Wang
  Cc: David Marchand, dev, jerinj, nd, Honnappa Nagarahalli, bruce.richardson

29/04/2021 16:28, Ruifeng Wang:
> From: Thomas Monjalon <thomas@monjalon.net>
> > 28/04/2021 11:30, Ruifeng Wang:
> > > From: David Marchand <david.marchand@redhat.com>
> > > > On Sun, Apr 25, 2021 at 7:57 AM Ruifeng Wang <ruifeng.wang@arm.com>
> > > > wrote:
> > > > >
> > > > > The rte_wait_until_equal_xxx APIs abstract the functionality of
> > > > > 'polling for a memory location to become equal to a given value'[1].
> > > > >
> > > > > Use the API for the rte spinlock and ring implementations.
> > > > > With the wait until equal APIs being stable, changes will not impact ABI.
> > > >
> > > > Afaics, there is no ARM target with WFE enabled and we lost ability
> > > > to enable WFE support with removal of the make build system.
> > >
> > > WFE can be enabled with direct meson file change.
> > > WFE is not intended to be enabled by default. It can be enabled based
> > > on benchmarking result on hardware.
> > > >
> > > > $ git grep RTE_ARM_USE_WFE
> > > > config/arm/meson.build:        ['RTE_ARM_USE_WFE', false],
> > > > lib/eal/arm/include/rte_pause_64.h:#ifdef RTE_ARM_USE_WFE
> > > >
> > > > How did you enable WFE to test this series?
> > >
> > > I modified meson file to test.
> > > Tests were also done with WFE disabled to make sure no degradation with
> > generic implementation.
> > 
> > I don't understand the usage.
> > Which platform should use it?
> 
> Platforms that implement WFE semantic (e.g. N1) can use.
> The user can enable this feature for power efficiency purpose. But there is something to
> note as described in commit message 1be7855d77 when the API was introduced. 
> 
> > Should it be a compile-time option?
> 
> Yes, it should be a compile-time option.
> It can be configured via c_args meson option?

+Cc Bruce for discussing how to enable such feature.

The problem with c_args is that the application has no way to know.



^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [dpdk-dev] Use WFE for spinlock and ring
  2021-04-29 15:20           ` Thomas Monjalon
@ 2021-04-30  9:16             ` Bruce Richardson
  2021-04-30 13:41               ` Honnappa Nagarahalli
  0 siblings, 1 reply; 36+ messages in thread
From: Bruce Richardson @ 2021-04-30  9:16 UTC (permalink / raw)
  To: Thomas Monjalon
  Cc: Ruifeng Wang, David Marchand, dev, jerinj, nd, Honnappa Nagarahalli

On Thu, Apr 29, 2021 at 05:20:05PM +0200, Thomas Monjalon wrote:
> 29/04/2021 16:28, Ruifeng Wang:
> > From: Thomas Monjalon <thomas@monjalon.net>
> > > 28/04/2021 11:30, Ruifeng Wang:
> > > > From: David Marchand <david.marchand@redhat.com>
> > > > > On Sun, Apr 25, 2021 at 7:57 AM Ruifeng Wang <ruifeng.wang@arm.com>
> > > > > wrote:
> > > > > >
> > > > > > The rte_wait_until_equal_xxx APIs abstract the functionality of
> > > > > > 'polling for a memory location to become equal to a given value'[1].
> > > > > >
> > > > > > Use the API for the rte spinlock and ring implementations.
> > > > > > With the wait until equal APIs being stable, changes will not impact ABI.
> > > > >
> > > > > Afaics, there is no ARM target with WFE enabled and we lost ability
> > > > > to enable WFE support with removal of the make build system.
> > > >
> > > > WFE can be enabled with direct meson file change.
> > > > WFE is not intended to be enabled by default. It can be enabled based
> > > > on benchmarking result on hardware.
> > > > >
> > > > > $ git grep RTE_ARM_USE_WFE
> > > > > config/arm/meson.build:        ['RTE_ARM_USE_WFE', false],
> > > > > lib/eal/arm/include/rte_pause_64.h:#ifdef RTE_ARM_USE_WFE
> > > > >
> > > > > How did you enable WFE to test this series?
> > > >
> > > > I modified meson file to test.
> > > > Tests were also done with WFE disabled to make sure no degradation with
> > > generic implementation.
> > > 
> > > I don't understand the usage.
> > > Which platform should use it?
> > 
> > Platforms that implement WFE semantic (e.g. N1) can use.
> > The user can enable this feature for power efficiency purpose. But there is something to
> > note as described in commit message 1be7855d77 when the API was introduced. 
> > 
> > > Should it be a compile-time option?
> > 
> > Yes, it should be a compile-time option.
> > It can be configured via c_args meson option?
> 
> +Cc Bruce for discussing how to enable such feature.
> 
> The problem with c_args is that the application has no way to know.
> 
Agree about c_args not being a great choice. Why does this need to be a
compile-time option? Can runtime support not be detected in some manner?

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [dpdk-dev] Use WFE for spinlock and ring
  2021-04-30  9:16             ` Bruce Richardson
@ 2021-04-30 13:41               ` Honnappa Nagarahalli
  2021-04-30 14:19                 ` Bruce Richardson
  0 siblings, 1 reply; 36+ messages in thread
From: Honnappa Nagarahalli @ 2021-04-30 13:41 UTC (permalink / raw)
  To: Bruce Richardson, thomas
  Cc: Ruifeng Wang, David Marchand, dev, jerinj, nd, Honnappa Nagarahalli, nd

<snip>

> > > > > > >
> > > > > > > The rte_wait_until_equal_xxx APIs abstract the functionality
> > > > > > > of 'polling for a memory location to become equal to a given
> value'[1].
> > > > > > >
> > > > > > > Use the API for the rte spinlock and ring implementations.
> > > > > > > With the wait until equal APIs being stable, changes will not impact
> ABI.
> > > > > >
> > > > > > Afaics, there is no ARM target with WFE enabled and we lost
> > > > > > ability to enable WFE support with removal of the make build
> system.
> > > > >
> > > > > WFE can be enabled with direct meson file change.
> > > > > WFE is not intended to be enabled by default. It can be enabled
> > > > > based on benchmarking result on hardware.
> > > > > >
> > > > > > $ git grep RTE_ARM_USE_WFE
> > > > > > config/arm/meson.build:        ['RTE_ARM_USE_WFE', false],
> > > > > > lib/eal/arm/include/rte_pause_64.h:#ifdef RTE_ARM_USE_WFE
> > > > > >
> > > > > > How did you enable WFE to test this series?
> > > > >
> > > > > I modified meson file to test.
> > > > > Tests were also done with WFE disabled to make sure no
> > > > > degradation with
> > > > generic implementation.
> > > >
> > > > I don't understand the usage.
> > > > Which platform should use it?
> > >
> > > Platforms that implement WFE semantic (e.g. N1) can use.
> > > The user can enable this feature for power efficiency purpose. But
> > > there is something to note as described in commit message 1be7855d77
> when the API was introduced.
> > >
> > > > Should it be a compile-time option?
> > >
> > > Yes, it should be a compile-time option.
> > > It can be configured via c_args meson option?
> >
> > +Cc Bruce for discussing how to enable such feature.
> >
> > The problem with c_args is that the application has no way to know.
> >
> Agree about c_args not being a great choice. Why does this need to be a
> compile-time option? Can runtime support not be detected in some
> manner?
The problem is inconsistency in performance on different Arm platforms. We had decided that each platform needs to enable it after some testing.

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [dpdk-dev] Use WFE for spinlock and ring
  2021-04-30 13:41               ` Honnappa Nagarahalli
@ 2021-04-30 14:19                 ` Bruce Richardson
  2021-05-07 10:18                   ` Ruifeng Wang
  0 siblings, 1 reply; 36+ messages in thread
From: Bruce Richardson @ 2021-04-30 14:19 UTC (permalink / raw)
  To: Honnappa Nagarahalli
  Cc: thomas, Ruifeng Wang, David Marchand, dev, jerinj, nd

On Fri, Apr 30, 2021 at 01:41:22PM +0000, Honnappa Nagarahalli wrote:
> <snip>
> 
> > > > > > > >
> > > > > > > > The rte_wait_until_equal_xxx APIs abstract the functionality
> > > > > > > > of 'polling for a memory location to become equal to a given
> > value'[1].
> > > > > > > >
> > > > > > > > Use the API for the rte spinlock and ring implementations.
> > > > > > > > With the wait until equal APIs being stable, changes will not impact
> > ABI.
> > > > > > >
> > > > > > > Afaics, there is no ARM target with WFE enabled and we lost
> > > > > > > ability to enable WFE support with removal of the make build
> > system.
> > > > > >
> > > > > > WFE can be enabled with direct meson file change.
> > > > > > WFE is not intended to be enabled by default. It can be enabled
> > > > > > based on benchmarking result on hardware.
> > > > > > >
> > > > > > > $ git grep RTE_ARM_USE_WFE
> > > > > > > config/arm/meson.build:        ['RTE_ARM_USE_WFE', false],
> > > > > > > lib/eal/arm/include/rte_pause_64.h:#ifdef RTE_ARM_USE_WFE
> > > > > > >
> > > > > > > How did you enable WFE to test this series?
> > > > > >
> > > > > > I modified meson file to test.
> > > > > > Tests were also done with WFE disabled to make sure no
> > > > > > degradation with
> > > > > generic implementation.
> > > > >
> > > > > I don't understand the usage.
> > > > > Which platform should use it?
> > > >
> > > > Platforms that implement WFE semantic (e.g. N1) can use.
> > > > The user can enable this feature for power efficiency purpose. But
> > > > there is something to note as described in commit message 1be7855d77
> > when the API was introduced.
> > > >
> > > > > Should it be a compile-time option?
> > > >
> > > > Yes, it should be a compile-time option.
> > > > It can be configured via c_args meson option?
> > >
> > > +Cc Bruce for discussing how to enable such feature.
> > >
> > > The problem with c_args is that the application has no way to know.
> > >
> > Agree about c_args not being a great choice. Why does this need to be a
> > compile-time option? Can runtime support not be detected in some
> > manner?
> The problem is inconsistency in performance on different Arm platforms. We had decided that each platform needs to enable it after some testing.

Then it sounds like it does indeed need to be a build option. Does it need
to be added to the meson_options.txt, or can it just be specified in
cross-files and optionally via c_args?

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [dpdk-dev] [PATCH v3 2/2] ring: use wfe to wait for ring tail update on aarch64
  2021-04-29 15:05         ` Stephen Hemminger
@ 2021-05-07  8:25           ` Ruifeng Wang
  0 siblings, 0 replies; 36+ messages in thread
From: Ruifeng Wang @ 2021-05-07  8:25 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Honnappa Nagarahalli, Konstantin Ananyev, dev, david.marchand,
	thomas, jerinj, nd, Gavin Hu, Steve Capper, Ola Liljedahl, nd

> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Thursday, April 29, 2021 11:06 PM
> To: Ruifeng Wang <Ruifeng.Wang@arm.com>
> Cc: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>; Konstantin
> Ananyev <konstantin.ananyev@intel.com>; dev@dpdk.org;
> david.marchand@redhat.com; thomas@monjalon.net; jerinj@marvell.com;
> nd <nd@arm.com>; Gavin Hu <Gavin.Hu@arm.com>; Steve Capper
> <Steve.Capper@arm.com>; Ola Liljedahl <Ola.Liljedahl@arm.com>
> Subject: Re: [dpdk-dev] [PATCH v3 2/2] ring: use wfe to wait for ring tail
> update on aarch64
> 
> On Thu, 29 Apr 2021 14:35:35 +0000
> Ruifeng Wang <Ruifeng.Wang@arm.com> wrote:
> 
> > > -----Original Message-----
> > > From: Stephen Hemminger <stephen@networkplumber.org>
> > > Sent: Thursday, April 29, 2021 1:17 AM
> > > To: Ruifeng Wang <Ruifeng.Wang@arm.com>
> > > Cc: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>; Konstantin
> > > Ananyev <konstantin.ananyev@intel.com>; dev@dpdk.org;
> > > david.marchand@redhat.com; thomas@monjalon.net;
> jerinj@marvell.com;
> > > nd <nd@arm.com>; Gavin Hu <Gavin.Hu@arm.com>; Steve Capper
> > > <Steve.Capper@arm.com>; Ola Liljedahl <Ola.Liljedahl@arm.com>
> > > Subject: Re: [dpdk-dev] [PATCH v3 2/2] ring: use wfe to wait for
> > > ring tail update on aarch64
> > >
> > > On Sun, 25 Apr 2021 05:56:53 +0000
> > > Ruifeng Wang <ruifeng.wang@arm.com> wrote:
> > >
> > > > Instead of polling for tail to be updated, use wfe instruction.
> > > >
> > > > Signed-off-by: Gavin Hu <gavin.hu@arm.com>
> > > > Signed-off-by: Ruifeng Wang <ruifeng.wang@arm.com>
> > > > Reviewed-by: Steve Capper <steve.capper@arm.com>
> > > > Reviewed-by: Ola Liljedahl <ola.liljedahl@arm.com>
> > > > Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > > > Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> > >
> > > Looks ok to me, but it does raise an interesting question.
> > > Shouldn't the original code have been using atomic load to look at ht->tail.
> > >
> > > This another place where "volatile considered harmful" applies.
> >
> > Do you mean 'volatile' should be removed from rte_wait_until_equal_xxx
> parameters?
> >
> 
> I meant that all access to tail should be via C11 atomic builtin. At that point,
> the volatile on the data structure elements does not matter.

Agreed. If synchronization is ensured by using C11 atomic builtin, 'volatile' on elements can be removed.

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [dpdk-dev] Use WFE for spinlock and ring
  2021-04-30 14:19                 ` Bruce Richardson
@ 2021-05-07 10:18                   ` Ruifeng Wang
  2021-05-07 10:24                     ` Bruce Richardson
  0 siblings, 1 reply; 36+ messages in thread
From: Ruifeng Wang @ 2021-05-07 10:18 UTC (permalink / raw)
  To: Bruce Richardson, Honnappa Nagarahalli
  Cc: thomas, David Marchand, dev, jerinj, nd, nd

> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: Friday, April 30, 2021 10:19 PM
> To: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> Cc: thomas@monjalon.net; Ruifeng Wang <Ruifeng.Wang@arm.com>; David
> Marchand <david.marchand@redhat.com>; dev <dev@dpdk.org>;
> jerinj@marvell.com; nd <nd@arm.com>
> Subject: Re: Use WFE for spinlock and ring
> 
> On Fri, Apr 30, 2021 at 01:41:22PM +0000, Honnappa Nagarahalli wrote:
> > <snip>
> >
> > > > > > > > >
> > > > > > > > > The rte_wait_until_equal_xxx APIs abstract the
> > > > > > > > > functionality of 'polling for a memory location to
> > > > > > > > > become equal to a given
> > > value'[1].
> > > > > > > > >
> > > > > > > > > Use the API for the rte spinlock and ring implementations.
> > > > > > > > > With the wait until equal APIs being stable, changes
> > > > > > > > > will not impact
> > > ABI.
> > > > > > > >
> > > > > > > > Afaics, there is no ARM target with WFE enabled and we
> > > > > > > > lost ability to enable WFE support with removal of the
> > > > > > > > make build
> > > system.
> > > > > > >
> > > > > > > WFE can be enabled with direct meson file change.
> > > > > > > WFE is not intended to be enabled by default. It can be
> > > > > > > enabled based on benchmarking result on hardware.
> > > > > > > >
> > > > > > > > $ git grep RTE_ARM_USE_WFE
> > > > > > > > config/arm/meson.build:        ['RTE_ARM_USE_WFE', false],
> > > > > > > > lib/eal/arm/include/rte_pause_64.h:#ifdef RTE_ARM_USE_WFE
> > > > > > > >
> > > > > > > > How did you enable WFE to test this series?
> > > > > > >
> > > > > > > I modified meson file to test.
> > > > > > > Tests were also done with WFE disabled to make sure no
> > > > > > > degradation with
> > > > > > generic implementation.
> > > > > >
> > > > > > I don't understand the usage.
> > > > > > Which platform should use it?
> > > > >
> > > > > Platforms that implement WFE semantic (e.g. N1) can use.
> > > > > The user can enable this feature for power efficiency purpose.
> > > > > But there is something to note as described in commit message
> > > > > 1be7855d77
> > > when the API was introduced.
> > > > >
> > > > > > Should it be a compile-time option?
> > > > >
> > > > > Yes, it should be a compile-time option.
> > > > > It can be configured via c_args meson option?
> > > >
> > > > +Cc Bruce for discussing how to enable such feature.
> > > >
> > > > The problem with c_args is that the application has no way to know.
> > > >
> > > Agree about c_args not being a great choice. Why does this need to
> > > be a compile-time option? Can runtime support not be detected in
> > > some manner?
> > The problem is inconsistency in performance on different Arm platforms.
> We had decided that each platform needs to enable it after some testing.
> 
> Then it sounds like it does indeed need to be a build option. Does it need to
> be added to the meson_options.txt, or can it just be specified in cross-files
> and optionally via c_args?

Add it to the meson_options.txt is good as the option will be clearly exposed.
My concern is more options (e.g. RTE_ARCH_ARM64_MEMCPY) need to be added in.
Will the options bloat meson_options.txt?

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [dpdk-dev] Use WFE for spinlock and ring
  2021-05-07 10:18                   ` Ruifeng Wang
@ 2021-05-07 10:24                     ` Bruce Richardson
  0 siblings, 0 replies; 36+ messages in thread
From: Bruce Richardson @ 2021-05-07 10:24 UTC (permalink / raw)
  To: Ruifeng Wang
  Cc: Honnappa Nagarahalli, thomas, David Marchand, dev, jerinj, nd

On Fri, May 07, 2021 at 10:18:52AM +0000, Ruifeng Wang wrote:
> > -----Original Message-----
> > From: Bruce Richardson <bruce.richardson@intel.com>
> > Sent: Friday, April 30, 2021 10:19 PM
> > To: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> > Cc: thomas@monjalon.net; Ruifeng Wang <Ruifeng.Wang@arm.com>; David
> > Marchand <david.marchand@redhat.com>; dev <dev@dpdk.org>;
> > jerinj@marvell.com; nd <nd@arm.com>
> > Subject: Re: Use WFE for spinlock and ring
> > 
> > On Fri, Apr 30, 2021 at 01:41:22PM +0000, Honnappa Nagarahalli wrote:
> > > <snip>
> > >
> > > > > > > > > >
> > > > > > > > > > The rte_wait_until_equal_xxx APIs abstract the
> > > > > > > > > > functionality of 'polling for a memory location to
> > > > > > > > > > become equal to a given
> > > > value'[1].
> > > > > > > > > >
> > > > > > > > > > Use the API for the rte spinlock and ring implementations.
> > > > > > > > > > With the wait until equal APIs being stable, changes
> > > > > > > > > > will not impact
> > > > ABI.
> > > > > > > > >
> > > > > > > > > Afaics, there is no ARM target with WFE enabled and we
> > > > > > > > > lost ability to enable WFE support with removal of the
> > > > > > > > > make build
> > > > system.
> > > > > > > >
> > > > > > > > WFE can be enabled with direct meson file change.
> > > > > > > > WFE is not intended to be enabled by default. It can be
> > > > > > > > enabled based on benchmarking result on hardware.
> > > > > > > > >
> > > > > > > > > $ git grep RTE_ARM_USE_WFE
> > > > > > > > > config/arm/meson.build:        ['RTE_ARM_USE_WFE', false],
> > > > > > > > > lib/eal/arm/include/rte_pause_64.h:#ifdef RTE_ARM_USE_WFE
> > > > > > > > >
> > > > > > > > > How did you enable WFE to test this series?
> > > > > > > >
> > > > > > > > I modified meson file to test.
> > > > > > > > Tests were also done with WFE disabled to make sure no
> > > > > > > > degradation with
> > > > > > > generic implementation.
> > > > > > >
> > > > > > > I don't understand the usage.
> > > > > > > Which platform should use it?
> > > > > >
> > > > > > Platforms that implement WFE semantic (e.g. N1) can use.
> > > > > > The user can enable this feature for power efficiency purpose.
> > > > > > But there is something to note as described in commit message
> > > > > > 1be7855d77
> > > > when the API was introduced.
> > > > > >
> > > > > > > Should it be a compile-time option?
> > > > > >
> > > > > > Yes, it should be a compile-time option.
> > > > > > It can be configured via c_args meson option?
> > > > >
> > > > > +Cc Bruce for discussing how to enable such feature.
> > > > >
> > > > > The problem with c_args is that the application has no way to know.
> > > > >
> > > > Agree about c_args not being a great choice. Why does this need to
> > > > be a compile-time option? Can runtime support not be detected in
> > > > some manner?
> > > The problem is inconsistency in performance on different Arm platforms.
> > We had decided that each platform needs to enable it after some testing.
> > 
> > Then it sounds like it does indeed need to be a build option. Does it need to
> > be added to the meson_options.txt, or can it just be specified in cross-files
> > and optionally via c_args?
> 
> Add it to the meson_options.txt is good as the option will be clearly exposed.
> My concern is more options (e.g. RTE_ARCH_ARM64_MEMCPY) need to be added in.
> Will the options bloat meson_options.txt?

That bloat would indeed become a concern. We may need to look at more use
of auto-detection and cross files for such options.

^ permalink raw reply	[flat|nested] 36+ messages in thread

end of thread, other threads:[~2021-05-07 10:24 UTC | newest]

Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-24  7:07 [dpdk-dev] [PATCH v1 0/2] Use WFE for spinlock and ring Gavin Hu
2020-04-24  7:07 ` [dpdk-dev] [PATCH v1 1/2] spinlock: use wfe to reduce contention on aarch64 Gavin Hu
2020-04-24  7:07 ` [dpdk-dev] [PATCH v1 2/2] ring: use wfe to wait for ring tail update " Gavin Hu
2020-04-24 13:23   ` Ananyev, Konstantin
2020-04-26  8:41     ` Gavin Hu
2020-04-24 14:46   ` Honnappa Nagarahalli
2020-04-26  8:33     ` Gavin Hu
2020-04-26  8:39 ` [dpdk-dev] [PATCH v2 0/2] Use WFE for spinlock and ring Gavin Hu
2020-05-01  9:47   ` Ananyev, Konstantin
2020-05-03 14:54   ` David Marchand
2020-05-03 21:31     ` Honnappa Nagarahalli
2020-09-25 14:30       ` David Marchand
2021-03-25 14:58         ` David Marchand
2021-04-25  6:06           ` Ruifeng Wang
2021-04-25 15:56             ` Thomas Monjalon
2020-04-26  8:39 ` [dpdk-dev] [PATCH v2 1/2] spinlock: use wfe to reduce contention on aarch64 Gavin Hu
2020-04-28 12:22   ` Jerin Jacob
2020-04-26  8:39 ` [dpdk-dev] [PATCH v2 2/2] ring: use wfe to wait for ring tail update " Gavin Hu
2021-04-25  5:56 ` [dpdk-dev] Use WFE for spinlock and ring Ruifeng Wang
2021-04-25  5:56   ` [dpdk-dev] [PATCH v3 1/2] spinlock: use wfe to reduce contention on aarch64 Ruifeng Wang
2021-04-25  5:56   ` [dpdk-dev] [PATCH v3 2/2] ring: use wfe to wait for ring tail update " Ruifeng Wang
2021-04-26  5:38     ` Jerin Jacob
2021-04-28 17:17     ` Stephen Hemminger
2021-04-29 14:35       ` Ruifeng Wang
2021-04-29 15:05         ` Stephen Hemminger
2021-05-07  8:25           ` Ruifeng Wang
2021-04-28  7:42   ` [dpdk-dev] Use WFE for spinlock and ring David Marchand
2021-04-28  9:30     ` Ruifeng Wang
2021-04-28 11:13       ` Thomas Monjalon
2021-04-29 14:28         ` Ruifeng Wang
2021-04-29 15:20           ` Thomas Monjalon
2021-04-30  9:16             ` Bruce Richardson
2021-04-30 13:41               ` Honnappa Nagarahalli
2021-04-30 14:19                 ` Bruce Richardson
2021-05-07 10:18                   ` Ruifeng Wang
2021-05-07 10:24                     ` Bruce Richardson

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git