[v1,2/2] mcslock: use wait until equal API for tight loop

  Instead of polling for previous lock holder unlocking, use
wait_until_equal API.

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

On Wed, Aug 25, 2021 at 10:02 AM Feifei Wang <feifei.wang2@arm.com> wrote:
>
> Instead of polling for previous lock holder unlocking, use
> wait_until_equal API.
>
> Signed-off-by: Feifei Wang <feifei.wang2@arm.com>
> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> ---
>  lib/eal/include/generic/rte_mcslock.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/lib/eal/include/generic/rte_mcslock.h b/lib/eal/include/generic/rte_mcslock.h
> index 9f323bd2a2..c99343f22c 100644
> --- a/lib/eal/include/generic/rte_mcslock.h
> +++ b/lib/eal/include/generic/rte_mcslock.h
> @@ -84,8 +84,8 @@ rte_mcslock_lock(rte_mcslock_t **msl, rte_mcslock_t *me)
>          * to spin on me->locked until the previous lock holder resets
>          * the me->locked using mcslock_unlock().
>          */
> -       while (__atomic_load_n(&me->locked, __ATOMIC_ACQUIRE))
> -               rte_pause();
> +       rte_wait_until_equal_32((volatile uint32_t *)&me->locked,
> +                       0, __ATOMIC_ACQUIRE);

Why do you need to cast as volatile?
  

> -----邮件原件-----
> 发件人: dev <dev-bounces@dpdk.org> 代表 David Marchand
> 发送时间: Tuesday, October 19, 2021 7:10 PM
> 收件人: Feifei Wang <Feifei.Wang2@arm.com>
> 抄送: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>; dev
> <dev@dpdk.org>; nd <nd@arm.com>; Ruifeng Wang
> <Ruifeng.Wang@arm.com>
> 主题: Re: [dpdk-dev] [PATCH v1 2/2] mcslock: use wait until equal API for
> tight loop
> 
> On Wed, Aug 25, 2021 at 10:02 AM Feifei Wang <feifei.wang2@arm.com>
> wrote:
> >
> > Instead of polling for previous lock holder unlocking, use
> > wait_until_equal API.
> >
> > Signed-off-by: Feifei Wang <feifei.wang2@arm.com>
> > Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> > ---
> >  lib/eal/include/generic/rte_mcslock.h | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/eal/include/generic/rte_mcslock.h
> > b/lib/eal/include/generic/rte_mcslock.h
> > index 9f323bd2a2..c99343f22c 100644
> > --- a/lib/eal/include/generic/rte_mcslock.h
> > +++ b/lib/eal/include/generic/rte_mcslock.h
> > @@ -84,8 +84,8 @@ rte_mcslock_lock(rte_mcslock_t **msl,
> rte_mcslock_t *me)
> >          * to spin on me->locked until the previous lock holder resets
> >          * the me->locked using mcslock_unlock().
> >          */
> > -       while (__atomic_load_n(&me->locked, __ATOMIC_ACQUIRE))
> > -               rte_pause();
> > +       rte_wait_until_equal_32((volatile uint32_t *)&me->locked,
> > +                       0, __ATOMIC_ACQUIRE);
> 
> Why do you need to cast as volatile?
Thanks for the comments.
This is firstly because rte_wait_until_equal API defines the variable as volatile.
However, with your comment, I find 'me->lock' is not volatile. And by the test,
I think you are right, it is necessary to add volatile here.
> 
> 
> --
> David Marchand
  

> -----邮件原件-----
> 发件人: Feifei Wang
> 发送时间: Wednesday, October 20, 2021 10:46 AM
> 收件人: David Marchand <david.marchand@redhat.com>
> 抄送: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>; dev
> <dev@dpdk.org>; nd <nd@arm.com>; Ruifeng Wang
> <Ruifeng.Wang@arm.com>; nd <nd@arm.com>
> 主题: 回复: [dpdk-dev] [PATCH v1 2/2] mcslock: use wait until equal API for
> tight loop
> 
> > -----邮件原件-----
> > 发件人: dev <dev-bounces@dpdk.org> 代表 David Marchand
> > 发送时间: Tuesday, October 19, 2021 7:10 PM
> > 收件人: Feifei Wang <Feifei.Wang2@arm.com>
> > 抄送: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>; dev
> > <dev@dpdk.org>; nd <nd@arm.com>; Ruifeng Wang
> <Ruifeng.Wang@arm.com>
> > 主题: Re: [dpdk-dev] [PATCH v1 2/2] mcslock: use wait until equal API
> > for tight loop
> >
> > On Wed, Aug 25, 2021 at 10:02 AM Feifei Wang <feifei.wang2@arm.com>
> > wrote:
> > >
> > > Instead of polling for previous lock holder unlocking, use
> > > wait_until_equal API.
> > >
> > > Signed-off-by: Feifei Wang <feifei.wang2@arm.com>
> > > Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> > > ---
> > >  lib/eal/include/generic/rte_mcslock.h | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/lib/eal/include/generic/rte_mcslock.h
> > > b/lib/eal/include/generic/rte_mcslock.h
> > > index 9f323bd2a2..c99343f22c 100644
> > > --- a/lib/eal/include/generic/rte_mcslock.h
> > > +++ b/lib/eal/include/generic/rte_mcslock.h
> > > @@ -84,8 +84,8 @@ rte_mcslock_lock(rte_mcslock_t **msl,
> > rte_mcslock_t *me)
> > >          * to spin on me->locked until the previous lock holder resets
> > >          * the me->locked using mcslock_unlock().
> > >          */
> > > -       while (__atomic_load_n(&me->locked, __ATOMIC_ACQUIRE))
> > > -               rte_pause();
> > > +       rte_wait_until_equal_32((volatile uint32_t *)&me->locked,
> > > +                       0, __ATOMIC_ACQUIRE);
> >
> > Why do you need to cast as volatile?
> Thanks for the comments.
> This is firstly because rte_wait_until_equal API defines the variable as volatile.
> However, with your comment, I find 'me->lock' is not volatile. And by the test,
> I think you are right, it is necessary to add volatile here.

Sorry, correct the writing mistakes:
'It is unnecessary to add volatile here.'
> >
> >
> > --
> > David Marchand