diff mbox

[API-NEXT,v7,2/6] example: generator: compare ticks instead of ns in loop

Message ID 1446737630-2323-3-git-send-email-ivan.khoronzhuk@linaro.org
State Accepted
Commit 02694bc81037c538f670abee4978799d40e80ee8
Headers show

Commit Message

Ivan Khoronzhuk Nov. 5, 2015, 3:33 p.m. UTC
It's more accurate to compare ticks instead of ns in each
iteration, so calculate wait range before entering the loop.

Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
---
 example/generator/odp_generator.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

Comments

Maxim Uvarov Nov. 9, 2015, 1:09 p.m. UTC | #1
after that patch that scheduler fails:

FAIL: ../../../test/validation/scheduler/scheduler_main

for arm64.

Maxim.

On 11/05/2015 18:33, Ivan Khoronzhuk wrote:
> It's more accurate to compare ticks instead of ns in each
> iteration, so calculate wait range before entering the loop.
>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> ---
>   example/generator/odp_generator.c | 10 ++++------
>   1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/example/generator/odp_generator.c b/example/generator/odp_generator.c
> index 60e015b..dd30403 100644
> --- a/example/generator/odp_generator.c
> +++ b/example/generator/odp_generator.c
> @@ -586,7 +586,7 @@ static void *gen_recv_thread(void *arg)
>    */
>   static void print_global_stats(int num_workers)
>   {
> -	uint64_t start, now, diff;
> +	uint64_t start, wait, diff;
>   	uint64_t pkts, pkts_prev = 0, pps, maximum_pps = 0;
>   	int verbose_interval = 20;
>   	odp_thrmask_t thrd_mask;
> @@ -594,6 +594,7 @@ static void print_global_stats(int num_workers)
>   	while (odp_thrmask_worker(&thrd_mask) < num_workers)
>   		continue;
>   
> +	wait = odp_time_ns_to_cycles(verbose_interval * ODP_TIME_SEC);
>   	start = odp_time_cycles();
>   
>   	while (odp_thrmask_worker(&thrd_mask) == num_workers) {
> @@ -603,12 +604,9 @@ static void print_global_stats(int num_workers)
>   			break;
>   		}
>   
> -		now = odp_time_cycles();
> -		diff = odp_time_diff_cycles(start, now);
> -		if (odp_time_cycles_to_ns(diff) <
> -		    verbose_interval * ODP_TIME_SEC) {
> +		diff = odp_time_diff_cycles(start, odp_time_cycles());
> +		if (diff < wait)
>   			continue;
> -		}
>   
>   		start = odp_time_cycles();
>
Maxim Uvarov Nov. 9, 2015, 1:12 p.m. UTC | #2
On 11/09/2015 16:09, Maxim Uvarov wrote:
> after that patch that scheduler fails:
>
> FAIL: ../../../test/validation/scheduler/scheduler_main
>
> for arm64.
>
> Maxim.

brrr, that changes for generator.  Interesting...

Maxim.

>
> On 11/05/2015 18:33, Ivan Khoronzhuk wrote:
>> It's more accurate to compare ticks instead of ns in each
>> iteration, so calculate wait range before entering the loop.
>>
>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
>> ---
>>   example/generator/odp_generator.c | 10 ++++------
>>   1 file changed, 4 insertions(+), 6 deletions(-)
>>
>> diff --git a/example/generator/odp_generator.c 
>> b/example/generator/odp_generator.c
>> index 60e015b..dd30403 100644
>> --- a/example/generator/odp_generator.c
>> +++ b/example/generator/odp_generator.c
>> @@ -586,7 +586,7 @@ static void *gen_recv_thread(void *arg)
>>    */
>>   static void print_global_stats(int num_workers)
>>   {
>> -    uint64_t start, now, diff;
>> +    uint64_t start, wait, diff;
>>       uint64_t pkts, pkts_prev = 0, pps, maximum_pps = 0;
>>       int verbose_interval = 20;
>>       odp_thrmask_t thrd_mask;
>> @@ -594,6 +594,7 @@ static void print_global_stats(int num_workers)
>>       while (odp_thrmask_worker(&thrd_mask) < num_workers)
>>           continue;
>>   +    wait = odp_time_ns_to_cycles(verbose_interval * ODP_TIME_SEC);
>>       start = odp_time_cycles();
>>         while (odp_thrmask_worker(&thrd_mask) == num_workers) {
>> @@ -603,12 +604,9 @@ static void print_global_stats(int num_workers)
>>               break;
>>           }
>>   -        now = odp_time_cycles();
>> -        diff = odp_time_diff_cycles(start, now);
>> -        if (odp_time_cycles_to_ns(diff) <
>> -            verbose_interval * ODP_TIME_SEC) {
>> +        diff = odp_time_diff_cycles(start, odp_time_cycles());
>> +        if (diff < wait)
>>               continue;
>> -        }
>>             start = odp_time_cycles();
>
Ivan Khoronzhuk Nov. 9, 2015, 1:26 p.m. UTC | #3
On 09.11.15 15:12, Maxim Uvarov wrote:
> On 11/09/2015 16:09, Maxim Uvarov wrote:
>> after that patch that scheduler fails:
>>
>> FAIL: ../../../test/validation/scheduler/scheduler_main
>>
>> for arm64.
>>
>> Maxim.
>
> brrr, that changes for generator.  Interesting...
>
> Maxim.

Interesting, maybe it was after 3/6 patch....?
Then I can check again.
But after this change it can reveal some other bug only.
Strange.

>
>>
>> On 11/05/2015 18:33, Ivan Khoronzhuk wrote:
>>> It's more accurate to compare ticks instead of ns in each
>>> iteration, so calculate wait range before entering the loop.
>>>
>>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
>>> ---
>>>   example/generator/odp_generator.c | 10 ++++------
>>>   1 file changed, 4 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/example/generator/odp_generator.c b/example/generator/odp_generator.c
>>> index 60e015b..dd30403 100644
>>> --- a/example/generator/odp_generator.c
>>> +++ b/example/generator/odp_generator.c
>>> @@ -586,7 +586,7 @@ static void *gen_recv_thread(void *arg)
>>>    */
>>>   static void print_global_stats(int num_workers)
>>>   {
>>> -    uint64_t start, now, diff;
>>> +    uint64_t start, wait, diff;
>>>       uint64_t pkts, pkts_prev = 0, pps, maximum_pps = 0;
>>>       int verbose_interval = 20;
>>>       odp_thrmask_t thrd_mask;
>>> @@ -594,6 +594,7 @@ static void print_global_stats(int num_workers)
>>>       while (odp_thrmask_worker(&thrd_mask) < num_workers)
>>>           continue;
>>>   +    wait = odp_time_ns_to_cycles(verbose_interval * ODP_TIME_SEC);
>>>       start = odp_time_cycles();
>>>         while (odp_thrmask_worker(&thrd_mask) == num_workers) {
>>> @@ -603,12 +604,9 @@ static void print_global_stats(int num_workers)
>>>               break;
>>>           }
>>>   -        now = odp_time_cycles();
>>> -        diff = odp_time_diff_cycles(start, now);
>>> -        if (odp_time_cycles_to_ns(diff) <
>>> -            verbose_interval * ODP_TIME_SEC) {
>>> +        diff = odp_time_diff_cycles(start, odp_time_cycles());
>>> +        if (diff < wait)
>>>               continue;
>>> -        }
>>>             start = odp_time_cycles();
>>
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
Ivan Khoronzhuk Nov. 9, 2015, 1:29 p.m. UTC | #4
On 09.11.15 15:09, Maxim Uvarov wrote:
> after that patch that scheduler fails:
>
> FAIL: ../../../test/validation/scheduler/scheduler_main
>
> for arm64.
>
> Maxim.

Could you please point what exactly failed?

>
> On 11/05/2015 18:33, Ivan Khoronzhuk wrote:
>> It's more accurate to compare ticks instead of ns in each
>> iteration, so calculate wait range before entering the loop.
>>
>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
>> ---
>>   example/generator/odp_generator.c | 10 ++++------
>>   1 file changed, 4 insertions(+), 6 deletions(-)
>>
>> diff --git a/example/generator/odp_generator.c b/example/generator/odp_generator.c
>> index 60e015b..dd30403 100644
>> --- a/example/generator/odp_generator.c
>> +++ b/example/generator/odp_generator.c
>> @@ -586,7 +586,7 @@ static void *gen_recv_thread(void *arg)
>>    */
>>   static void print_global_stats(int num_workers)
>>   {
>> -    uint64_t start, now, diff;
>> +    uint64_t start, wait, diff;
>>       uint64_t pkts, pkts_prev = 0, pps, maximum_pps = 0;
>>       int verbose_interval = 20;
>>       odp_thrmask_t thrd_mask;
>> @@ -594,6 +594,7 @@ static void print_global_stats(int num_workers)
>>       while (odp_thrmask_worker(&thrd_mask) < num_workers)
>>           continue;
>> +    wait = odp_time_ns_to_cycles(verbose_interval * ODP_TIME_SEC);
>>       start = odp_time_cycles();
>>       while (odp_thrmask_worker(&thrd_mask) == num_workers) {
>> @@ -603,12 +604,9 @@ static void print_global_stats(int num_workers)
>>               break;
>>           }
>> -        now = odp_time_cycles();
>> -        diff = odp_time_diff_cycles(start, now);
>> -        if (odp_time_cycles_to_ns(diff) <
>> -            verbose_interval * ODP_TIME_SEC) {
>> +        diff = odp_time_diff_cycles(start, odp_time_cycles());
>> +        if (diff < wait)
>>               continue;
>> -        }
>>           start = odp_time_cycles();
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
Ivan Khoronzhuk Nov. 9, 2015, 1:35 p.m. UTC | #5
On 09.11.15 15:29, Ivan Khoronzhuk wrote:
>
>
> On 09.11.15 15:09, Maxim Uvarov wrote:
>> after that patch that scheduler fails:
>>
>> FAIL: ../../../test/validation/scheduler/scheduler_main
>>
>> for arm64.
>>
>> Maxim.
>

Also, pat attention that for ARM64 the time API is based on cycle API
that is not accurate enough....my next patch will be to help a little with it.
So if this bug is connected with not correct cpu_freq_max or smth like this
then I have patch for it. But it's based on this series and cpu diff order patch.
I'm going to send it after this is applied.

>
>>
>> On 11/05/2015 18:33, Ivan Khoronzhuk wrote:
>>> It's more accurate to compare ticks instead of ns in each
>>> iteration, so calculate wait range before entering the loop.
>>>
>>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
>>> ---
>>>   example/generator/odp_generator.c | 10 ++++------
>>>   1 file changed, 4 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/example/generator/odp_generator.c b/example/generator/odp_generator.c
>>> index 60e015b..dd30403 100644
>>> --- a/example/generator/odp_generator.c
>>> +++ b/example/generator/odp_generator.c
>>> @@ -586,7 +586,7 @@ static void *gen_recv_thread(void *arg)
>>>    */
>>>   static void print_global_stats(int num_workers)
>>>   {
>>> -    uint64_t start, now, diff;
>>> +    uint64_t start, wait, diff;
>>>       uint64_t pkts, pkts_prev = 0, pps, maximum_pps = 0;
>>>       int verbose_interval = 20;
>>>       odp_thrmask_t thrd_mask;
>>> @@ -594,6 +594,7 @@ static void print_global_stats(int num_workers)
>>>       while (odp_thrmask_worker(&thrd_mask) < num_workers)
>>>           continue;
>>> +    wait = odp_time_ns_to_cycles(verbose_interval * ODP_TIME_SEC);
>>>       start = odp_time_cycles();
>>>       while (odp_thrmask_worker(&thrd_mask) == num_workers) {
>>> @@ -603,12 +604,9 @@ static void print_global_stats(int num_workers)
>>>               break;
>>>           }
>>> -        now = odp_time_cycles();
>>> -        diff = odp_time_diff_cycles(start, now);
>>> -        if (odp_time_cycles_to_ns(diff) <
>>> -            verbose_interval * ODP_TIME_SEC) {
>>> +        diff = odp_time_diff_cycles(start, odp_time_cycles());
>>> +        if (diff < wait)
>>>               continue;
>>> -        }
>>>           start = odp_time_cycles();
>>
>> _______________________________________________
>> lng-odp mailing list
>> lng-odp@lists.linaro.org
>> https://lists.linaro.org/mailman/listinfo/lng-odp
>
Ivan Khoronzhuk Nov. 9, 2015, 1:42 p.m. UTC | #6
Or issue can be that you forgot to revert patch you currently incorrectly applied.
"[lng-odp] [API-NEXT PATCH] api: cpu: change order of arguments for diff function"


The right order is:
"[lng-odp] [API-NEXT PATCH v7 0/6] api: time: unbind CPU cycles from time API" series.
(don't forget v8 of 3 patch)
then
"[lng-odp] [API-NEXT PATCH] api: cpu: change order of arguments for diff function"

On 09.11.15 15:29, Ivan Khoronzhuk wrote:
>
>
> On 09.11.15 15:09, Maxim Uvarov wrote:
>> after that patch that scheduler fails:
>>
>> FAIL: ../../../test/validation/scheduler/scheduler_main
>>
>> for arm64.
>>
>> Maxim.
>
> Could you please point what exactly failed?
>
>>
>> On 11/05/2015 18:33, Ivan Khoronzhuk wrote:
>>> It's more accurate to compare ticks instead of ns in each
>>> iteration, so calculate wait range before entering the loop.
>>>
>>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
>>> ---
>>>   example/generator/odp_generator.c | 10 ++++------
>>>   1 file changed, 4 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/example/generator/odp_generator.c b/example/generator/odp_generator.c
>>> index 60e015b..dd30403 100644
>>> --- a/example/generator/odp_generator.c
>>> +++ b/example/generator/odp_generator.c
>>> @@ -586,7 +586,7 @@ static void *gen_recv_thread(void *arg)
>>>    */
>>>   static void print_global_stats(int num_workers)
>>>   {
>>> -    uint64_t start, now, diff;
>>> +    uint64_t start, wait, diff;
>>>       uint64_t pkts, pkts_prev = 0, pps, maximum_pps = 0;
>>>       int verbose_interval = 20;
>>>       odp_thrmask_t thrd_mask;
>>> @@ -594,6 +594,7 @@ static void print_global_stats(int num_workers)
>>>       while (odp_thrmask_worker(&thrd_mask) < num_workers)
>>>           continue;
>>> +    wait = odp_time_ns_to_cycles(verbose_interval * ODP_TIME_SEC);
>>>       start = odp_time_cycles();
>>>       while (odp_thrmask_worker(&thrd_mask) == num_workers) {
>>> @@ -603,12 +604,9 @@ static void print_global_stats(int num_workers)
>>>               break;
>>>           }
>>> -        now = odp_time_cycles();
>>> -        diff = odp_time_diff_cycles(start, now);
>>> -        if (odp_time_cycles_to_ns(diff) <
>>> -            verbose_interval * ODP_TIME_SEC) {
>>> +        diff = odp_time_diff_cycles(start, odp_time_cycles());
>>> +        if (diff < wait)
>>>               continue;
>>> -        }
>>>           start = odp_time_cycles();
>>
>> _______________________________________________
>> lng-odp mailing list
>> lng-odp@lists.linaro.org
>> https://lists.linaro.org/mailman/listinfo/lng-odp
>
diff mbox

Patch

diff --git a/example/generator/odp_generator.c b/example/generator/odp_generator.c
index 60e015b..dd30403 100644
--- a/example/generator/odp_generator.c
+++ b/example/generator/odp_generator.c
@@ -586,7 +586,7 @@  static void *gen_recv_thread(void *arg)
  */
 static void print_global_stats(int num_workers)
 {
-	uint64_t start, now, diff;
+	uint64_t start, wait, diff;
 	uint64_t pkts, pkts_prev = 0, pps, maximum_pps = 0;
 	int verbose_interval = 20;
 	odp_thrmask_t thrd_mask;
@@ -594,6 +594,7 @@  static void print_global_stats(int num_workers)
 	while (odp_thrmask_worker(&thrd_mask) < num_workers)
 		continue;
 
+	wait = odp_time_ns_to_cycles(verbose_interval * ODP_TIME_SEC);
 	start = odp_time_cycles();
 
 	while (odp_thrmask_worker(&thrd_mask) == num_workers) {
@@ -603,12 +604,9 @@  static void print_global_stats(int num_workers)
 			break;
 		}
 
-		now = odp_time_cycles();
-		diff = odp_time_diff_cycles(start, now);
-		if (odp_time_cycles_to_ns(diff) <
-		    verbose_interval * ODP_TIME_SEC) {
+		diff = odp_time_diff_cycles(start, odp_time_cycles());
+		if (diff < wait)
 			continue;
-		}
 
 		start = odp_time_cycles();