Richard Weinberger via Xenomai <xenomai@xenomai.org> writes:

> Florian,
>
> ----- Ursprüngliche Mail -----
>> Von: "Florian Bezdeka" <florian.bezd...@siemens.com>
>> An: "xenomai" <xenomai@xenomai.org>, "richard" <rich...@nod.at>
>> Gesendet: Freitag, 4. März 2022 13:01:08
>> Betreff: Re: Questiion on commit: demo/cyclictest: fix time delta calculation
>
>> Hi Richard,
>> 
>> On Fri, 2022-03-04 at 12:35 +0100, Richard Weinberger via Xenomai
>> wrote:
>>> Hello,
>>> 
>>> I'm investigating into a cyclictest issue where it reports negative values 
>>> just
>>> like this commit states:
>>> https://source.denx.de/Xenomai/xenomai/-/commit/3069a7bbc48559f4d2a6184d6159b771f193a4e9
>> 
>> Ipipe or Dovetail? Does running "autotune" help?
>
> I'm on ipipe on an arm i.mx6 system. Let me try autotune. :)
>
>>> 
>>> Since I'm on Xenomai 3.1.2 the said commit is already included.
>>> But I have a hard time to understand what problem the commit actually fixes.
>>> Why is the old calculation wrong?
>> 
>> old code: In case the second wrapped we have
>> ts1.nsec = <small> (we consider t1 to be the later point in time)
>> ts2.nsec = <big>
>> 
>> diff += t1 - t2 is now negative. Right?
>
> It did:
>> diff = NSEC_PER_SEC * (int64_t)(t1.tv_sec - t2.tv_sec);
>
> At this point diff is always a multiple of NSEC_PER_SEC or 0.
> It is only 0 if t1.tv_sec are the same t2.tv_sec value.
>
>> diff += (t1.tv_nsec - t2.tv_nsec);
>
> Here is where we seem to disagree.
> t1.tv_nsec - t2.tv_nsec can be negative, yes.
> But it can get only negative if t1.tv_sec - t2.tv_sec is non-zero.
> So subtracting the negative delta from diff (which is a multiple of 
> NSEC_PER_SEC)
> cannot get negative.
>
> Of course the whole calculation works only of the clock does not go backwards.
>
> Thanks,
> //richard

Florian pointed in the right direction when mentioning autotune: if the
core timer is not calibrated properly, Cobalt might try to compensate
for the basic latency of the platform too much, leading to early timer
shots. If such an early shot arrives and the test loop wakes up too
early, say in the code implementing clock_nanosleep (-n) case, 'next'
might actually be later in the timeline than 'now', leading to negative
differences in calcdiff().

The change in calcdiff() is most likely a plain workaround for this
case, assuming the normalized delta is very unlikely to go over the top
and won't be used as a worst-case figure. The commit log is definitely
misleading.

-- 
Philippe.

Reply via email to