On Mon, May 16, 2016 at 9:27 AM, Ivan Khoronzhuk <[email protected] > wrote:
> Hi Matias, > > The odp_time_local and others functions are time sensitive functions, > that's why it was decided to avoid copping as more as possible. > > The timespec is not simple "long type". Its type is arch dependent but is > always 64bit. > In case of 32 bit system it's defined as long long. > The same for odp_time_t struct. So, at least for now it seems to be the > same for > both 32 and 64 bit systems. And I think Bill Fischofer knew about this > while adding this > ,at first glance, strange union, right Bill? > Yes. The original purpose of that union was not efficiency (though that's a happy side-effect) but rather to remove the need for ODP applications to be dependent on the variable expansion of the Linux timespec struct, which is dependent on POSIX level. That was an earlier bug that caused much grief. > Yes, it's not the best decision from style point of view, but it's fast > and in case of an error > is supposed to be caught by time validation tests. > > > On 04.05.16 16:01, Matias Elo wrote: > >> The size of 'struct timespec' may vary on different host >> architectures as it includes type long members. This breaks >> time functions on a 32-bit x86 host. Fix this by >> individually copying struct timespec members to odp_time_t. >> >> Signed-off-by: Matias Elo <[email protected]> >> --- >> platform/linux-generic/odp_time.c | 26 +++++++++++++++----------- >> 1 file changed, 15 insertions(+), 11 deletions(-) >> >> diff --git a/platform/linux-generic/odp_time.c >> b/platform/linux-generic/odp_time.c >> index 040f754..81e0522 100644 >> --- a/platform/linux-generic/odp_time.c >> +++ b/platform/linux-generic/odp_time.c >> @@ -11,11 +11,6 @@ >> #include <odp/api/hints.h> >> #include <odp_debug_internal.h> >> >> -typedef union { >> - odp_time_t ex; >> - struct timespec in; >> -} _odp_time_t; >> - >> static odp_time_t start_time; >> >> static inline >> @@ -47,13 +42,17 @@ static inline odp_time_t time_diff(odp_time_t t2, >> odp_time_t t1) >> static inline odp_time_t time_local(void) >> { >> int ret; >> - _odp_time_t time; >> + odp_time_t time; >> + struct timespec sys_time; >> >> - ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time.in); >> + ret = clock_gettime(CLOCK_MONOTONIC_RAW, &sys_time); >> if (odp_unlikely(ret != 0)) >> ODP_ABORT("clock_gettime failed\n"); >> >> - return time_diff(time.ex, start_time); >> + time.tv_sec = sys_time.tv_sec; >> + time.tv_nsec = sys_time.tv_nsec; >> + >> + return time_diff(time, start_time); >> } >> >> static inline int time_cmp(odp_time_t t2, odp_time_t t1) >> @@ -195,10 +194,15 @@ uint64_t odp_time_to_u64(odp_time_t time) >> int odp_time_init_global(void) >> { >> int ret; >> - _odp_time_t time; >> + struct timespec time; >> >> - ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time.in); >> - start_time = ret ? ODP_TIME_NULL : time.ex; >> + ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time); >> + if (ret) { >> + start_time = ODP_TIME_NULL; >> + } else { >> + start_time.tv_sec = time.tv_sec; >> + start_time.tv_nsec = time.tv_nsec; >> + } >> >> return ret; >> } >> >> > -- > Regards, > Ivan Khoronzhuk > > _______________________________________________ > lng-odp mailing list > [email protected] > https://lists.linaro.org/mailman/listinfo/lng-odp > _______________________________________________ lng-odp mailing list [email protected] https://lists.linaro.org/mailman/listinfo/lng-odp
