On 09.09.15 15:00, Savolainen, Petri (Nokia - FI/Espoo) wrote:
-----Original Message-----
From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of
ext Ivan Khoronzhuk
Sent: Monday, September 07, 2015 11:51 AM
To: lng-odp@lists.linaro.org
Subject: [lng-odp] [Patch] linux-generic: odp_schedule: fix
odp_schdule_wait_time
The resolution of schedule time can be more than 1ns. As result
can happen that 1ns corresponds 0 ticks of timer counter, but if
passed 1ns it's obvious that user wanted to schedule at least once.
Currently it can lead to wait forever, as 0 corresponds to
ODP_SCHED_WAIT, it can block program flow at all.
Signed-off-by: Ivan Khoronzhuk <ivan.khoronz...@linaro.org>
---
Prerequisit for this patch was taken from:
"[lng-odp] [Patch] validation: scheduler: increase time check"
https://lists.linaro.org/pipermail/lng-odp/2015-August/014738.html
platform/linux-generic/odp_schedule.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/platform/linux-generic/odp_schedule.c b/platform/linux-
generic/odp_schedule.c
index c6619e5..05497de 100644
--- a/platform/linux-generic/odp_schedule.c
+++ b/platform/linux-generic/odp_schedule.c
@@ -646,10 +646,13 @@ void odp_schedule_resume(void)
uint64_t odp_schedule_wait_time(uint64_t ns)
{
- if (ns <= ODP_SCHED_NO_WAIT)
- ns = ODP_SCHED_NO_WAIT + 1;
+ uint64_t time;
- return odp_time_ns_to_cycles(ns);
+ time = odp_time_ns_to_cycles(ns);
+ if (!time)
+ time = ODP_SCHED_NO_WAIT;
+
+ return time;
}
I think it's better to change (implementation specific) WAIT/NO_WAIT values
like this.
plat/schedule_types.h
#define ODP_SCHED_WAIT UINT64_MAX
#define ODP_SCHED_NO_WAIT 0
... and avoid any conversion in the common case (WAIT/NO_WAIT). It will not
matter, if 1 ns is rounded to 0 == NO_WAIT.
uint64_t odp_schedule_wait_time(uint64_t ns)
{
if (ns == ODP_SCHED_WAIT || ns == ODP_SCHED_NO_WAIT)
We shouldn't compare sched time and ns, even it's under implementation.
And there is no direct mapping between ns and sched time.
In case of maximum for ns, it doesn't mean pool forever for scheduler.
Even if it has very low possibility. Assigning POOL forever it's rather an
application responsibility then conversion function.
This function can be reused by implementations, mostly on transition stage.
As you mentioned, WAIT/NO_WAIT it's implementation specific (define can be not
from linux-generic,
and function from linux-generic) and better to not rely on WAIT/NO_WAIT = 0.
My variant has no such drawback and is more clear in what's going on.
And your example should be revritten like:
if (!ns)
return ODP_SCHED_NO_WAIT;
return odp_time_ns_to_cycles(ns);
It doesn't differ a lot with proposed patch but still has mentioned drawback,
even
if it's very rarely case.
return ns;
return odp_time_ns_to_cycles(ns);
}
-Petri
--
Regards,
Ivan Khoronzhuk
_______________________________________________
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp
