| Message ID | 1446245775-31359-1-git-send-email-ivan.khoronzhuk@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 31.10.15 00:56, Ivan Khoronzhuk wrote: > No need to continue test example if tick is lost. 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 handles the timer > ticks. This patch saves user from incorrect choice by aborting > the application. > > 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..3728ac3 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) > { > + uint64_t overrun; > odp_timer_pool *tp = (odp_timer_pool *)sigval.sival_ptr; > + > + overrun = timer_getoverrun(tp->timerid); > + if (overrun) > + ODP_ABORT("Error: need to increase resolution, lost tickets: %" > + PRIu64"\n", overrun); Maybe here should be warn instead of abort? > + > #ifdef __ARM_ARCH > odp_timer *array = &tp->timers[0]; > uint32_t i; >
A reportable statistic for this situation would be appropriate, but this certainly should not result in an abort. Ticks can be lost for all sorts of reasons and should be considered unusual but hardly fatal conditions. Aborts are reserved for serious internal errors or detected inconsistencies that compromise the integrity of the system. On Fri, Nov 6, 2015 at 8:05 AM, Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote: > On 31.10.15 00:56, Ivan Khoronzhuk wrote: > >> No need to continue test example if tick is lost. 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 handles the timer >> ticks. This patch saves user from incorrect choice by aborting >> the application. >> >> 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..3728ac3 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) >> { >> + uint64_t overrun; >> odp_timer_pool *tp = (odp_timer_pool *)sigval.sival_ptr; >> + >> + overrun = timer_getoverrun(tp->timerid); >> + if (overrun) >> + ODP_ABORT("Error: need to increase resolution, lost >> tickets: %" >> + PRIu64"\n", overrun); >> > > Maybe here should be warn instead of abort? > > + >> #ifdef __ARM_ARCH >> odp_timer *array = &tp->timers[0]; >> uint32_t i; >> >> > -- > Regards, > Ivan Khoronzhuk > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > https://lists.linaro.org/mailman/listinfo/lng-odp >
On 07.11.15 06:10, Bill Fischofer wrote: > A reportable statistic for this situation would be appropriate, but this certainly should not result in an abort. Ticks can be lost for all sorts of reasons and should be considered unusual but hardly fatal conditions. Aborts are reserved for serious internal errors or detected inconsistencies that compromise the integrity of the system. It's not simple statistic, it's indicator that actual resolution in the system is worse than set in the timer by the application and allows to set appropriate ... later. It depends on load on system, jiffy, size of the handler and is only way to draw attention on it. But abort is not correct, I agree. I'll send new patch with warn instead of abort. > > On Fri, Nov 6, 2015 at 8:05 AM, Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org <mailto:ivan.khoronzhuk@linaro.org>> wrote: > > On 31.10.15 00:56, Ivan Khoronzhuk wrote: > > No need to continue test example if tick is lost. 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 handles the timer > ticks. This patch saves user from incorrect choice by aborting > the application. > > Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org <mailto: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..3728ac3 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) > { > + uint64_t overrun; > odp_timer_pool *tp = (odp_timer_pool *)sigval.sival_ptr; > + > + overrun = timer_getoverrun(tp->timerid); > + if (overrun) > + ODP_ABORT("Error: need to increase resolution, lost tickets: %" > + PRIu64"\n", overrun); > > > Maybe here should be warn instead of abort? > > + > #ifdef __ARM_ARCH > odp_timer *array = &tp->timers[0]; > uint32_t i; > > > -- > Regards, > Ivan Khoronzhuk > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> > https://lists.linaro.org/mailman/listinfo/lng-odp > >
diff --git a/platform/linux-generic/odp_timer.c b/platform/linux-generic/odp_timer.c index e8f0267..3728ac3 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) { + uint64_t overrun; odp_timer_pool *tp = (odp_timer_pool *)sigval.sival_ptr; + + overrun = timer_getoverrun(tp->timerid); + if (overrun) + ODP_ABORT("Error: need to increase resolution, lost tickets: %" + PRIu64"\n", overrun); + #ifdef __ARM_ARCH odp_timer *array = &tp->timers[0]; uint32_t i;
No need to continue test example if tick is lost. 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 handles the timer ticks. This patch saves user from incorrect choice by aborting the application. Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> --- platform/linux-generic/odp_timer.c | 7 +++++++ 1 file changed, 7 insertions(+)