On 16.09.15 14:39, Savolainen, Petri (Nokia - FI/Espoo) wrote:


-----Original Message-----
From: EXT Ivan Khoronzhuk [mailto:[email protected]]
Sent: Wednesday, September 16, 2015 1:47 PM
To: [email protected]; Savolainen, Petri (Nokia - FI/Espoo)
Subject: Re: [lng-odp] [PATCH v2 3/5] linux-generic: odp_time: reutrn 0
if t2 = t1 instead of MAX for diff

On 16.09.15 13:23, Ivan Khoronzhuk wrote:
Petri,

What about this fix? It's similar to to CPU API.

On 11.09.15 13:04, Ivan Khoronzhuk wrote:
It's better to describe by example:

cur = 15;
start = 15;
diff = 2;
while (odp_time_cycles_diff(start, cur) < diff) {
     cur = odp_time_cycles();
}

This example has to work. It's possible only when t2 - t1 = 0 if t2
= t1.
The validation test on it will be added later.

Signed-off-by: Ivan Khoronzhuk <[email protected]>
---
   platform/linux-generic/odp_time.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/platform/linux-generic/odp_time.c b/platform/linux-
generic/odp_time.c
index a08833d..a007d69 100644
--- a/platform/linux-generic/odp_time.c
+++ b/platform/linux-generic/odp_time.c
@@ -14,7 +14,7 @@

   uint64_t odp_time_diff_cycles(uint64_t t1, uint64_t t2)
   {
-    if (odp_likely(t2 > t1))
+    if (odp_likely(t2 >= t1))
           return t2 - t1;

       return t2 + (UINT64_MAX - t1);


But I have additional proposition. Maybe I'm wrong, but
one cycle can be lost here (equal as in CPU api, I'm ready to fix it
also)

For instance:

start = MAX - 2;
cur = 1

res = MAX - MAX + 2 + 1 = 3;
It's correct. But in real it will be:

(MAX - 2)
            -> 1 cycle
(MAX - 1)
            -> 2 cycle
MAX
            -> 3 cycle
0
            -> 4 cycle
1

The function returns 3 cycles difference,
but due to 0, physically, timer counts 4 cycles.

Not sure, but I should send +1 patch that corrects it to:

return t2 + (UINT64_MAX - t1) + 1;

due to counter in continuous mode is reset to 0, then continues
counting.

Can we apply this on cycle counter? (then I need correct CPU API
implementation also)
Is it reseted to zero or wraps to 1 for all arches?



For instance, from here
http://download.intel.com/design/intelxscale/27347302.pdf,
(Intel XScale(r) Core) CCNT behaves like:

"When CCNT reaches its maximum value 0xFFFF,FFFF, the next clock
cycle will cause it to roll over to zero"

But I'm not sure about other arches hidden under linux-generic.
I tend to believe that it's applicable for all cases.

--
Regards,
Ivan Khoronzhuk


uint64_t odp_time_diff_cycles(uint64_t t1, uint64_t t2)
{
        if (odp_unlikely(t2 == t1))
                return 0;

        if (odp_likely(t2 > t1))
                return t2 - t1;

Sorry, but why not
if (odp_likely(t2 >= t1))
        return t2 - t1;

as in patch.


        return t2 + (UINT64_MAX - t1) + 1;
}


t1 = MAX

t2 = MAX - 1, diff = MAX - 1 + MAX - MAX + 1 = MAX
True, as we count +1 cycle in real.
Ok.

t2 = MAX,     diff = 0
True.

t2 = 0,       diff = 0 + MAX - MAX + 1 = 1
Also true. As for count from MAX to 0 we need 1 cycle.
Ok.



t1 = MAX - 1
t2 = MAX - 2, diff = MAX - 2 + MAX - MAX + 1 + 1 = MAX
True and OK.

t2 = MAX - 1, diff = 0
True and OK.

t2 = MAX,     diff = MAX - MAX + 1 = 1
True and OK.

t2 = 0,       diff = 0 + MAX - MAX + 1 + 1 = 2
True, as we need 2 cycles to reach 0.
Ok.

t2 = 1,       diff = 1 + MAX - MAX + 1 + 1 = 3
True, as we need 3 cycles to reach 1.
Ok.



It's very likely that when t1 == t2, the correct result is 0.
Yep, I'm not proposing to change this patch. I propose to send additional patch 
later.
It will be rather series, as I want to correct cpu API also.

 Let's catch that first. Wrap around case can have then the extra +1.
Right, that is I worry about. But seems on current arches it's applicable.
Also we can limit it to arches we are sure, as -1 cycle it's obvious error now 
and
it adds additional error in accuracy +-2 cycles instead of 1. In case of bad 
cycle counter
resolution, say 64, it can have valuable impact, especially if it's multiplied 
on some value.



-Petri





--
Regards,
Ivan Khoronzhuk
_______________________________________________
lng-odp mailing list
[email protected]
https://lists.linaro.org/mailman/listinfo/lng-odp

Reply via email to