[v7,3/5] eal: use wait event scheme for mcslock

Message ID 20211028065640.139655-4-feifei.wang2@arm.com (mailing list archive)
State Superseded, archived
Delegated to: David Marchand
Headers
Series add new definitions for wait scheme |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Feifei Wang Oct. 28, 2021, 6:56 a.m. UTC
  Instead of polling for mcslock to be updated, use wait event scheme
for this case.

Furthermore, use 'uintptr_t *' is for different size of pointer in 32/64
bits architecture.

And define a new pointer 'next' for the compilation error:
-------------------------------------------------------------------
'dereferencing type-punned pointer will break strict-aliasing rules'
-------------------------------------------------------------------

Signed-off-by: Feifei Wang <feifei.wang2@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
---
 lib/eal/include/generic/rte_mcslock.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
  

Comments

Jerin Jacob Oct. 28, 2021, 7:02 a.m. UTC | #1
On Thu, Oct 28, 2021 at 12:27 PM Feifei Wang <feifei.wang2@arm.com> wrote:
>
> Instead of polling for mcslock to be updated, use wait event scheme
> for this case.
>
> Furthermore, use 'uintptr_t *' is for different size of pointer in 32/64
> bits architecture.
>
> And define a new pointer 'next' for the compilation error:
> -------------------------------------------------------------------
> 'dereferencing type-punned pointer will break strict-aliasing rules'
> -------------------------------------------------------------------
>
> Signed-off-by: Feifei Wang <feifei.wang2@arm.com>
> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> ---
>  lib/eal/include/generic/rte_mcslock.h | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/lib/eal/include/generic/rte_mcslock.h b/lib/eal/include/generic/rte_mcslock.h
> index 34f33c64a5..d5b9b293cd 100644
> --- a/lib/eal/include/generic/rte_mcslock.h
> +++ b/lib/eal/include/generic/rte_mcslock.h
> @@ -116,8 +116,9 @@ rte_mcslock_unlock(rte_mcslock_t **msl, rte_mcslock_t *me)
>                 /* More nodes added to the queue by other CPUs.
>                  * Wait until the next pointer is set.
>                  */
> -               while (__atomic_load_n(&me->next, __ATOMIC_RELAXED) == NULL)
> -                       rte_pause();
> +               uintptr_t *next = NULL;

It is going to update in the next line. Why explicit NULL assignment?

> +               next = (uintptr_t *)&me->next;
> +               rte_wait_event(next, UINTPTR_MAX, ==, 0, __ATOMIC_RELAXED);
>         }
>
>         /* Pass lock to next waiter. */
> --
> 2.25.1
>
  
Feifei Wang Oct. 28, 2021, 7:14 a.m. UTC | #2
> -----邮件原件-----
> 发件人: dev <dev-bounces@dpdk.org> 代表 Jerin Jacob
> 发送时间: Thursday, October 28, 2021 3:02 PM
> 收件人: Feifei Wang <Feifei.Wang2@arm.com>
> 抄送: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>; dpdk-dev
> <dev@dpdk.org>; nd <nd@arm.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; Stephen Hemminger
> <stephen@networkplumber.org>; David Marchand
> <david.marchand@redhat.com>; thomas@monjalon.net; Mattias Rönnblom
> <mattias.ronnblom@ericsson.com>; Ruifeng Wang <Ruifeng.Wang@arm.com>
> 主题: Re: [dpdk-dev] [PATCH v7 3/5] eal: use wait event scheme for mcslock
> 
> On Thu, Oct 28, 2021 at 12:27 PM Feifei Wang <feifei.wang2@arm.com>
> wrote:
> >
> > Instead of polling for mcslock to be updated, use wait event scheme
> > for this case.
> >
> > Furthermore, use 'uintptr_t *' is for different size of pointer in
> > 32/64 bits architecture.
> >
> > And define a new pointer 'next' for the compilation error:
> > -------------------------------------------------------------------
> > 'dereferencing type-punned pointer will break strict-aliasing rules'
> > -------------------------------------------------------------------
> >
> > Signed-off-by: Feifei Wang <feifei.wang2@arm.com>
> > Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> > ---
> >  lib/eal/include/generic/rte_mcslock.h | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/eal/include/generic/rte_mcslock.h
> > b/lib/eal/include/generic/rte_mcslock.h
> > index 34f33c64a5..d5b9b293cd 100644
> > --- a/lib/eal/include/generic/rte_mcslock.h
> > +++ b/lib/eal/include/generic/rte_mcslock.h
> > @@ -116,8 +116,9 @@ rte_mcslock_unlock(rte_mcslock_t **msl,
> rte_mcslock_t *me)
> >                 /* More nodes added to the queue by other CPUs.
> >                  * Wait until the next pointer is set.
> >                  */
> > -               while (__atomic_load_n(&me->next, __ATOMIC_RELAXED) ==
> NULL)
> > -                       rte_pause();
> > +               uintptr_t *next = NULL;
> 
> It is going to update in the next line. Why explicit NULL assignment?
You are right, it is unnecessary to initialize it as NULL. I will update this.
> 
> > +               next = (uintptr_t *)&me->next;
> > +               rte_wait_event(next, UINTPTR_MAX, ==, 0,
> > + __ATOMIC_RELAXED);
> >         }
> >
> >         /* Pass lock to next waiter. */
> > --
> > 2.25.1
> >
  

Patch

diff --git a/lib/eal/include/generic/rte_mcslock.h b/lib/eal/include/generic/rte_mcslock.h
index 34f33c64a5..d5b9b293cd 100644
--- a/lib/eal/include/generic/rte_mcslock.h
+++ b/lib/eal/include/generic/rte_mcslock.h
@@ -116,8 +116,9 @@  rte_mcslock_unlock(rte_mcslock_t **msl, rte_mcslock_t *me)
 		/* More nodes added to the queue by other CPUs.
 		 * Wait until the next pointer is set.
 		 */
-		while (__atomic_load_n(&me->next, __ATOMIC_RELAXED) == NULL)
-			rte_pause();
+		uintptr_t *next = NULL;
+		next = (uintptr_t *)&me->next;
+		rte_wait_event(next, UINTPTR_MAX, ==, 0, __ATOMIC_RELAXED);
 	}
 
 	/* Pass lock to next waiter. */