[v3,3/6] test/spinlock: get timestamp more precisely

Message ID 20181227041349.3058-4-gavin.hu@arm.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series spinlock optimization and test case enhancements |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Gavin Hu Dec. 27, 2018, 4:13 a.m. UTC
  To precisely benchmark the spinlock performance, uses the precise
version of getting timestamps, which enforces the timestamps are
obtained at the expected places.

Signed-off-by: Gavin Hu <gavin.hu@arm.com>
Reviewed-by: Phil Yang <phil.yang@arm.com>
---
 test/test/test_spinlock.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
  

Comments

Jerin Jacob Kollanukkaran Dec. 27, 2018, 7:27 a.m. UTC | #1
On Thu, 2018-12-27 at 12:13 +0800, Gavin Hu wrote:
> -------------------------------------------------------------------
> ---
> To precisely benchmark the spinlock performance, uses the precise
> version of getting timestamps, which enforces the timestamps are
> obtained at the expected places.
> 
> Signed-off-by: Gavin Hu <gavin.hu@arm.com>
> Reviewed-by: Phil Yang <phil.yang@arm.com>
> ---
>  test/test/test_spinlock.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/test/test/test_spinlock.c b/test/test/test_spinlock.c
> index 6795195ae..648474833 100644
> --- a/test/test/test_spinlock.c
> +++ b/test/test/test_spinlock.c
> @@ -113,14 +113,14 @@ load_loop_fn(void *func_param)
>  	if (lcore != rte_get_master_lcore())
>  		while (rte_atomic32_read(&synchro) == 0);
>  
> -	begin = rte_get_timer_cycles();
> +	begin = rte_rdtsc_precise();
>  	while (time_diff < hz * TIME_MS / 1000) {
>  		if (use_lock)
>  			rte_spinlock_lock(&lk);
>  		lcount++;
>  		if (use_lock)
>  			rte_spinlock_unlock(&lk);
> -		time_diff = rte_get_timer_cycles() - begin;
> +		time_diff = rte_rdtsc_precise() - begin;

Since _precise() versions add a full barrier, time_diff would
include delay of a full barrier also. As mentioned in the commit 
message, Do you see rte_get_timer_cycles() called in unexpected places?
Is there difference in time_diff apart from delay in rte_mb() when
using with _precise() version?





>  	}
>  	lock_count[lcore] = lcount;
>  	return 0;
  
Honnappa Nagarahalli Jan. 3, 2019, 6:22 p.m. UTC | #2
> 
> On Thu, 2018-12-27 at 12:13 +0800, Gavin Hu wrote:
> > -------------------------------------------------------------------
> > ---
> > To precisely benchmark the spinlock performance, uses the precise
> > version of getting timestamps, which enforces the timestamps are
> > obtained at the expected places.
> >
> > Signed-off-by: Gavin Hu <gavin.hu@arm.com>
> > Reviewed-by: Phil Yang <phil.yang@arm.com>
> > ---
> >  test/test/test_spinlock.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/test/test/test_spinlock.c b/test/test/test_spinlock.c
> > index 6795195ae..648474833 100644
> > --- a/test/test/test_spinlock.c
> > +++ b/test/test/test_spinlock.c
> > @@ -113,14 +113,14 @@ load_loop_fn(void *func_param)
> >  	if (lcore != rte_get_master_lcore())
> >  		while (rte_atomic32_read(&synchro) == 0);
> >
> > -	begin = rte_get_timer_cycles();
> > +	begin = rte_rdtsc_precise();
> >  	while (time_diff < hz * TIME_MS / 1000) {
> >  		if (use_lock)
> >  			rte_spinlock_lock(&lk);
> >  		lcount++;
> >  		if (use_lock)
> >  			rte_spinlock_unlock(&lk);
> > -		time_diff = rte_get_timer_cycles() - begin;
> > +		time_diff = rte_rdtsc_precise() - begin;
> 
> Since _precise() versions add a full barrier, time_diff would include delay of
> a full barrier also. As mentioned in the commit message, Do you see
> rte_get_timer_cycles() called in unexpected places?
> Is there difference in time_diff apart from delay in rte_mb() when using with
> _precise() version?
> 
I do not see the need for this patch in the presence of 4/6 [1] in this series.

[1] http://mails.dpdk.org/archives/dev/2018-December/122110.html

> 
> 
> 
> 
> >  	}
> >  	lock_count[lcore] = lcount;
> >  	return 0;
  

Patch

diff --git a/test/test/test_spinlock.c b/test/test/test_spinlock.c
index 6795195ae..648474833 100644
--- a/test/test/test_spinlock.c
+++ b/test/test/test_spinlock.c
@@ -113,14 +113,14 @@  load_loop_fn(void *func_param)
 	if (lcore != rte_get_master_lcore())
 		while (rte_atomic32_read(&synchro) == 0);
 
-	begin = rte_get_timer_cycles();
+	begin = rte_rdtsc_precise();
 	while (time_diff < hz * TIME_MS / 1000) {
 		if (use_lock)
 			rte_spinlock_lock(&lk);
 		lcount++;
 		if (use_lock)
 			rte_spinlock_unlock(&lk);
-		time_diff = rte_get_timer_cycles() - begin;
+		time_diff = rte_rdtsc_precise() - begin;
 	}
 	lock_count[lcore] = lcount;
 	return 0;