diff mbox

[2/3,v5] linux-generic: odp_timer: warn if tick is late

Message ID 1447169760-15258-3-git-send-email-ivan.khoronzhuk@linaro.org
State Superseded
Headers show

Commit Message

Ivan Khoronzhuk Nov. 10, 2015, 3:35 p.m. UTC
If tick is late then application should be warned about this.
It means that actual timer resolution is worse than expected.
The default timer resolution is set 10ms, that is equal to jiffy
on most systems. The default resolution is set bearing in mind
that on a CPU runs maximum two threads that ideally fits in 10ms.
But user can change it to be smaller, in case if CPU0 is isolated
and it handles only the timer ticks. This patch helps user to set
correct timer resolution.

Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
---
 platform/linux-generic/odp_timer.c | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Ola Liljedahl Nov. 18, 2015, 5:37 p.m. UTC | #1
On 10 November 2015 at 15:35, Ivan Khoronzhuk
<ivan.khoronzhuk@linaro.org> wrote:
> If tick is late then application should be warned about this.
> It means that actual timer resolution is worse than expected.
> The default timer resolution is set 10ms, that is equal to jiffy
> on most systems. The default resolution is set bearing in mind
> that on a CPU runs maximum two threads that ideally fits in 10ms.
> But user can change it to be smaller, in case if CPU0 is isolated
> and it handles only the timer ticks. This patch helps user to set
> correct timer resolution.
>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> ---
>  platform/linux-generic/odp_timer.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/platform/linux-generic/odp_timer.c b/platform/linux-generic/odp_timer.c
> index e8f0267..407ccc1 100644
> --- a/platform/linux-generic/odp_timer.c
> +++ b/platform/linux-generic/odp_timer.c
> @@ -632,7 +632,14 @@ static unsigned odp_timer_pool_expire(odp_timer_pool_t tpid, uint64_t tick)
>
>  static void timer_notify(sigval_t sigval)
>  {
> +       int overrun;
>         odp_timer_pool *tp = (odp_timer_pool *)sigval.sival_ptr;
> +
> +       overrun = timer_getoverrun(tp->timerid);
The Linux man page isn't completely clear to me but I assume that this
overrun counter doesn't accumulate over time (then we would get the
message every tick as soon as it occurred once) but that it only count
and returns the numbers of overruns (expiration notifications lost)
since this specific timer expired and the notification was enqueued.

> +       if (overrun)
> +               ODP_ERR("\n\t%d ticks overrun on timer pool \"%s\", timer resolution too high\n",
The timer overruns but ticks are lost. I think you are conflating the
concepts here by writing "ticks overrun". What was wrong with my
"ticks lost on timer pool %s"? Or "POSIX timer overrun(s) on timer
pool %s"?

> +                       overrun, tp->name);
> +
>  #ifdef __ARM_ARCH
>         odp_timer *array = &tp->timers[0];
>         uint32_t i;
> --
> 1.9.1
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
Ivan Khoronzhuk Nov. 18, 2015, 6:07 p.m. UTC | #2
On 18.11.15 19:37, Ola Liljedahl wrote:
> On 10 November 2015 at 15:35, Ivan Khoronzhuk
> <ivan.khoronzhuk@linaro.org> wrote:
>> If tick is late then application should be warned about this.
>> It means that actual timer resolution is worse than expected.
>> The default timer resolution is set 10ms, that is equal to jiffy
>> on most systems. The default resolution is set bearing in mind
>> that on a CPU runs maximum two threads that ideally fits in 10ms.
>> But user can change it to be smaller, in case if CPU0 is isolated
>> and it handles only the timer ticks. This patch helps user to set
>> correct timer resolution.
>>
>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
>> ---
>>   platform/linux-generic/odp_timer.c | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/platform/linux-generic/odp_timer.c b/platform/linux-generic/odp_timer.c
>> index e8f0267..407ccc1 100644
>> --- a/platform/linux-generic/odp_timer.c
>> +++ b/platform/linux-generic/odp_timer.c
>> @@ -632,7 +632,14 @@ static unsigned odp_timer_pool_expire(odp_timer_pool_t tpid, uint64_t tick)
>>
>>   static void timer_notify(sigval_t sigval)
>>   {
>> +       int overrun;
>>          odp_timer_pool *tp = (odp_timer_pool *)sigval.sival_ptr;
>> +
>> +       overrun = timer_getoverrun(tp->timerid);
> The Linux man page isn't completely clear to me but I assume that this
> overrun counter doesn't accumulate over time (then we would get the
I assume that it can accumulate but handle them one by one. Thus when
3 tick where not delivered in time, it will handle it 3 times one by one equaling it to 0.
It can lead to burst of ticks. Even in this case they cannot be accumulated as the main reason
why they were late is process yielding and after process is awaken it handles all late
ticks in a queue. Even in case the ticks are not in the queue we have requested timer
resolution more than system can allow and it shows in how much it should be tuned and after read it = 0.
I've tested it.

> message every tick as soon as it occurred once) but that it only count
> and returns the numbers of overruns (expiration notifications lost)
> since this specific timer expired and the notification was enqueued.
>
>> +       if (overrun)
>> +               ODP_ERR("\n\t%d ticks overrun on timer pool \"%s\", timer resolution too high\n",
> The timer overruns but ticks are lost. I think you are conflating the
yes

> concepts here by writing "ticks overrun". What was wrong with my
> "ticks lost on timer pool %s"? Or "POSIX timer overrun(s) on timer
> pool %s"?

There is no description they are lost or not. Thus in some case they can be in queue.
Or some amount can be in the queue and other can be lost (due to limitations).
So I cannot say that they are all lost or all late, and here no need in it.
Here it's used only as indicator that resolution is incorrect and print evaluation number
in how much it's should be tuned.

>
>> +                       overrun, tp->name);
>> +
>>   #ifdef __ARM_ARCH
>>          odp_timer *array = &tp->timers[0];
>>          uint32_t i;
>> --
>> 1.9.1
>>
>> _______________________________________________
>> lng-odp mailing list
>> lng-odp@lists.linaro.org
>> https://lists.linaro.org/mailman/listinfo/lng-odp
diff mbox

Patch

diff --git a/platform/linux-generic/odp_timer.c b/platform/linux-generic/odp_timer.c
index e8f0267..407ccc1 100644
--- a/platform/linux-generic/odp_timer.c
+++ b/platform/linux-generic/odp_timer.c
@@ -632,7 +632,14 @@  static unsigned odp_timer_pool_expire(odp_timer_pool_t tpid, uint64_t tick)
 
 static void timer_notify(sigval_t sigval)
 {
+	int overrun;
 	odp_timer_pool *tp = (odp_timer_pool *)sigval.sival_ptr;
+
+	overrun = timer_getoverrun(tp->timerid);
+	if (overrun)
+		ODP_ERR("\n\t%d ticks overrun on timer pool \"%s\", timer resolution too high\n",
+			overrun, tp->name);
+
 #ifdef __ARM_ARCH
 	odp_timer *array = &tp->timers[0];
 	uint32_t i;