test/mcslock: wait for lcore completion

Message ID 20190807145844.23670-1-aconole@redhat.com (mailing list archive)
State Accepted, archived
Headers
Series test/mcslock: wait for lcore completion |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-Compile-Testing success Compile Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Performance-Testing success Performance Testing PASS
ci/mellanox-Performance-Testing success Performance Testing PASS

Commit Message

Aaron Conole Aug. 7, 2019, 2:58 p.m. UTC
  It's possible that the mcsunlock occurs before the test_mcslock_try has
a chance to execute, which will result in the trylock being successful,
making the test case fail.  Fix this by waiting until all lcores have
completed their test before unlocking the master lock.

Fixes: 32dcb9fd2a22 ("test/mcslock: add MCS queued lock unit test")
Cc: Phil Yang <phil.yang@arm.com>
Cc: Gavin Hu <gavin.hu@arm.com>
Cc: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Signed-off-by: Aaron Conole <aconole@redhat.com>
---
 app/test/test_mcslock.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

Phil Yang Aug. 8, 2019, 3:44 a.m. UTC | #1
> -----Original Message-----
> From: Aaron Conole <aconole@redhat.com>
> Sent: Wednesday, August 7, 2019 10:59 PM
> To: dev@dpdk.org
> Cc: Phil Yang (Arm Technology China) <Phil.Yang@arm.com>; Gavin Hu (Arm
> Technology China) <Gavin.Hu@arm.com>; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>
> Subject: [PATCH] test/mcslock: wait for lcore completion
> 
> It's possible that the mcsunlock occurs before the test_mcslock_try has
> a chance to execute, which will result in the trylock being successful,
> making the test case fail.  Fix this by waiting until all lcores have
> completed their test before unlocking the master lock.
> 
> Fixes: 32dcb9fd2a22 ("test/mcslock: add MCS queued lock unit test")
> Cc: Phil Yang <phil.yang@arm.com>
> Cc: Gavin Hu <gavin.hu@arm.com>
> Cc: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Signed-off-by: Aaron Conole <aconole@redhat.com>
> ---
>  app/test/test_mcslock.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/app/test/test_mcslock.c b/app/test/test_mcslock.c
> index ed384b81d..e9359df2e 100644
> --- a/app/test/test_mcslock.c
> +++ b/app/test/test_mcslock.c
> @@ -225,8 +225,8 @@ test_mcslock(void)
>  	RTE_LCORE_FOREACH_SLAVE(i) {
>  		rte_eal_remote_launch(test_mcslock_try, NULL, i);
>  	}
> -	rte_mcslock_unlock(&p_ml_try, &ml_try_me);
>  	rte_eal_mp_wait_lcore();
> +	rte_mcslock_unlock(&p_ml_try, &ml_try_me);
> 
>  	/* Test is_locked API */
>  	if (rte_mcslock_is_locked(p_ml)) {
> --
> 2.21.0

Thanks for the patch.
Acked-by: Phil Yang <phil.yang@arm.com>
  
David Marchand Aug. 8, 2019, 6:58 a.m. UTC | #2
On Wed, Aug 7, 2019 at 4:58 PM Aaron Conole <aconole@redhat.com> wrote:
>
> It's possible that the mcsunlock occurs before the test_mcslock_try has
> a chance to execute, which will result in the trylock being successful,
> making the test case fail.  Fix this by waiting until all lcores have
> completed their test before unlocking the master lock.
>
> Fixes: 32dcb9fd2a22 ("test/mcslock: add MCS queued lock unit test")
> Cc: Phil Yang <phil.yang@arm.com>
> Cc: Gavin Hu <gavin.hu@arm.com>
> Cc: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Signed-off-by: Aaron Conole <aconole@redhat.com>
> ---
>  app/test/test_mcslock.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/app/test/test_mcslock.c b/app/test/test_mcslock.c
> index ed384b81d..e9359df2e 100644
> --- a/app/test/test_mcslock.c
> +++ b/app/test/test_mcslock.c
> @@ -225,8 +225,8 @@ test_mcslock(void)
>         RTE_LCORE_FOREACH_SLAVE(i) {
>                 rte_eal_remote_launch(test_mcslock_try, NULL, i);
>         }
> -       rte_mcslock_unlock(&p_ml_try, &ml_try_me);
>         rte_eal_mp_wait_lcore();
> +       rte_mcslock_unlock(&p_ml_try, &ml_try_me);
>
>         /* Test is_locked API */
>         if (rte_mcslock_is_locked(p_ml)) {
> --
> 2.21.0
>

Reviewed-by: David Marchand <david.marchand@redhat.com>
Thanks.
  
Thomas Monjalon Aug. 8, 2019, 10:22 a.m. UTC | #3
08/08/2019 05:44, Phil Yang (Arm Technology China):
> From: Aaron Conole <aconole@redhat.com>
> > 
> > It's possible that the mcsunlock occurs before the test_mcslock_try has
> > a chance to execute, which will result in the trylock being successful,
> > making the test case fail.  Fix this by waiting until all lcores have
> > completed their test before unlocking the master lock.
> > 
> > Fixes: 32dcb9fd2a22 ("test/mcslock: add MCS queued lock unit test")
> > Cc: Phil Yang <phil.yang@arm.com>
> > Cc: Gavin Hu <gavin.hu@arm.com>
> > Cc: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > Signed-off-by: Aaron Conole <aconole@redhat.com>
> 
> Thanks for the patch.
> Acked-by: Phil Yang <phil.yang@arm.com>

Applied, thanks
  

Patch

diff --git a/app/test/test_mcslock.c b/app/test/test_mcslock.c
index ed384b81d..e9359df2e 100644
--- a/app/test/test_mcslock.c
+++ b/app/test/test_mcslock.c
@@ -225,8 +225,8 @@  test_mcslock(void)
 	RTE_LCORE_FOREACH_SLAVE(i) {
 		rte_eal_remote_launch(test_mcslock_try, NULL, i);
 	}
-	rte_mcslock_unlock(&p_ml_try, &ml_try_me);
 	rte_eal_mp_wait_lcore();
+	rte_mcslock_unlock(&p_ml_try, &ml_try_me);
 
 	/* Test is_locked API */
 	if (rte_mcslock_is_locked(p_ml)) {