| Message ID | 1449840155-5550-1-git-send-email-bill.fischofer@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
On 12/11/2015 16:22, Bill Fischofer wrote: > The linux-generic implementation of odp_time_t makes use of POSIX > APIs that are sensitive to the _POSIX_C_SOURCE level. Use an indirection > mechanism so that these dependencies do not "bleed through" the ODP API. > This means that ODP applications can be independent of _POSIX_C_SOURCE > level. > > Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org> > --- > .../linux-generic/include/odp/plat/time_types.h | 9 ++++--- > platform/linux-generic/odp_time.c | 30 +++++++++++----------- > 2 files changed, 20 insertions(+), 19 deletions(-) > > diff --git a/platform/linux-generic/include/odp/plat/time_types.h b/platform/linux-generic/include/odp/plat/time_types.h > index e5765ec..e999beb 100644 > --- a/platform/linux-generic/include/odp/plat/time_types.h > +++ b/platform/linux-generic/include/odp/plat/time_types.h > @@ -21,11 +21,12 @@ extern "C" { > * @{ > **/ > > -typedef struct timespec odp_time_t; > +typedef struct { > + int64_t tv_sec; time_t is not reachable? > + int64_t tv_nsec; > +} odp_time_t; > > -odp_time_t odp_time_null(void); > - > -#define ODP_TIME_NULL odp_time_null() > +#define ODP_TIME_NULL ((odp_time_t){0, 0}) > > /** > * @} > diff --git a/platform/linux-generic/odp_time.c b/platform/linux-generic/odp_time.c > index 1c7c214..9b64b37 100644 > --- a/platform/linux-generic/odp_time.c > +++ b/platform/linux-generic/odp_time.c > @@ -11,7 +11,12 @@ > #include <odp/hints.h> > #include <odp_debug_internal.h> > > -static struct timespec start_time; > +typedef union { > + odp_time_t ex; > + struct timespec in; > +} _odp_time_t; > + > +static odp_time_t start_time; > > static inline > uint64_t time_to_ns(odp_time_t time) > @@ -27,7 +32,7 @@ uint64_t time_to_ns(odp_time_t time) > static inline > odp_time_t time_diff(odp_time_t t2, odp_time_t t1) > { > - struct timespec time; > + odp_time_t time; > > time.tv_sec = t2.tv_sec - t1.tv_sec; > time.tv_nsec = t2.tv_nsec - t1.tv_nsec; > @@ -43,13 +48,13 @@ odp_time_t time_diff(odp_time_t t2, odp_time_t t1) > odp_time_t odp_time_local(void) > { > int ret; > - struct timespec time; > + _odp_time_t time; > > - ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time); > + ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time.in); > if (odp_unlikely(ret != 0)) > ODP_ABORT("clock_gettime failed\n"); > > - return time_diff(time, start_time); > + return time_diff(time.ex, start_time); > } > > odp_time_t odp_time_diff(odp_time_t t2, odp_time_t t1) > @@ -64,7 +69,7 @@ uint64_t odp_time_to_ns(odp_time_t time) > > odp_time_t odp_time_local_from_ns(uint64_t ns) > { > - struct timespec time; > + odp_time_t time; > > time.tv_sec = ns / ODP_TIME_SEC_IN_NS; > time.tv_nsec = ns - time.tv_sec * ODP_TIME_SEC_IN_NS; > @@ -85,7 +90,7 @@ int odp_time_cmp(odp_time_t t2, odp_time_t t1) > > odp_time_t odp_time_sum(odp_time_t t1, odp_time_t t2) > { > - struct timespec time; > + odp_time_t time; > > time.tv_sec = t2.tv_sec + t1.tv_sec; > time.tv_nsec = t2.tv_nsec + t1.tv_nsec; > @@ -113,18 +118,13 @@ uint64_t odp_time_to_u64(odp_time_t time) > return time_to_ns(time) / resolution; > } > > -odp_time_t odp_time_null(void) > -{ > - return (struct timespec) {0, 0}; > -} > - > int odp_time_global_init(void) > { > int ret; > - struct timespec time; > + _odp_time_t time; > > - ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time); > - start_time = ret ? odp_time_null() : time; > + ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time.in); > + start_time = ret ? ODP_TIME_NULL : time.ex; > > return ret; > }
On Fri, Dec 11, 2015 at 7:27 AM, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > On 12/11/2015 16:22, Bill Fischofer wrote: > >> The linux-generic implementation of odp_time_t makes use of POSIX >> APIs that are sensitive to the _POSIX_C_SOURCE level. Use an indirection >> mechanism so that these dependencies do not "bleed through" the ODP API. >> This means that ODP applications can be independent of _POSIX_C_SOURCE >> level. >> >> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org> >> --- >> .../linux-generic/include/odp/plat/time_types.h | 9 ++++--- >> platform/linux-generic/odp_time.c | 30 >> +++++++++++----------- >> 2 files changed, 20 insertions(+), 19 deletions(-) >> >> diff --git a/platform/linux-generic/include/odp/plat/time_types.h >> b/platform/linux-generic/include/odp/plat/time_types.h >> index e5765ec..e999beb 100644 >> --- a/platform/linux-generic/include/odp/plat/time_types.h >> +++ b/platform/linux-generic/include/odp/plat/time_types.h >> @@ -21,11 +21,12 @@ extern "C" { >> * @{ >> **/ >> -typedef struct timespec odp_time_t; >> +typedef struct { >> + int64_t tv_sec; >> > time_t is not reachable? > > Not sure I understand this comment. Can you elaborate? > + int64_t tv_nsec; >> +} odp_time_t; >> -odp_time_t odp_time_null(void); >> - >> -#define ODP_TIME_NULL odp_time_null() >> +#define ODP_TIME_NULL ((odp_time_t){0, 0}) >> /** >> * @} >> diff --git a/platform/linux-generic/odp_time.c >> b/platform/linux-generic/odp_time.c >> index 1c7c214..9b64b37 100644 >> --- a/platform/linux-generic/odp_time.c >> +++ b/platform/linux-generic/odp_time.c >> @@ -11,7 +11,12 @@ >> #include <odp/hints.h> >> #include <odp_debug_internal.h> >> -static struct timespec start_time; >> +typedef union { >> + odp_time_t ex; >> + struct timespec in; >> +} _odp_time_t; >> + >> +static odp_time_t start_time; >> static inline >> uint64_t time_to_ns(odp_time_t time) >> @@ -27,7 +32,7 @@ uint64_t time_to_ns(odp_time_t time) >> static inline >> odp_time_t time_diff(odp_time_t t2, odp_time_t t1) >> { >> - struct timespec time; >> + odp_time_t time; >> time.tv_sec = t2.tv_sec - t1.tv_sec; >> time.tv_nsec = t2.tv_nsec - t1.tv_nsec; >> @@ -43,13 +48,13 @@ odp_time_t time_diff(odp_time_t t2, odp_time_t t1) >> odp_time_t odp_time_local(void) >> { >> int ret; >> - struct timespec time; >> + _odp_time_t time; >> - ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time); >> + ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time.in); >> if (odp_unlikely(ret != 0)) >> ODP_ABORT("clock_gettime failed\n"); >> - return time_diff(time, start_time); >> + return time_diff(time.ex, start_time); >> } >> odp_time_t odp_time_diff(odp_time_t t2, odp_time_t t1) >> @@ -64,7 +69,7 @@ uint64_t odp_time_to_ns(odp_time_t time) >> odp_time_t odp_time_local_from_ns(uint64_t ns) >> { >> - struct timespec time; >> + odp_time_t time; >> time.tv_sec = ns / ODP_TIME_SEC_IN_NS; >> time.tv_nsec = ns - time.tv_sec * ODP_TIME_SEC_IN_NS; >> @@ -85,7 +90,7 @@ int odp_time_cmp(odp_time_t t2, odp_time_t t1) >> odp_time_t odp_time_sum(odp_time_t t1, odp_time_t t2) >> { >> - struct timespec time; >> + odp_time_t time; >> time.tv_sec = t2.tv_sec + t1.tv_sec; >> time.tv_nsec = t2.tv_nsec + t1.tv_nsec; >> @@ -113,18 +118,13 @@ uint64_t odp_time_to_u64(odp_time_t time) >> return time_to_ns(time) / resolution; >> } >> -odp_time_t odp_time_null(void) >> -{ >> - return (struct timespec) {0, 0}; >> -} >> - >> int odp_time_global_init(void) >> { >> int ret; >> - struct timespec time; >> + _odp_time_t time; >> - ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time); >> - start_time = ret ? odp_time_null() : time; >> + ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time.in); >> + start_time = ret ? ODP_TIME_NULL : time.ex; >> return ret; >> } >> > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > https://lists.linaro.org/mailman/listinfo/lng-odp >
On 12/11/2015 16:30, Bill Fischofer wrote: > > > On Fri, Dec 11, 2015 at 7:27 AM, Maxim Uvarov <maxim.uvarov@linaro.org > <mailto:maxim.uvarov@linaro.org>> wrote: > > On 12/11/2015 16:22, Bill Fischofer wrote: > > The linux-generic implementation of odp_time_t makes use of POSIX > APIs that are sensitive to the _POSIX_C_SOURCE level. Use an > indirection > mechanism so that these dependencies do not "bleed through" > the ODP API. > This means that ODP applications can be independent of > _POSIX_C_SOURCE > level. > > Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org > <mailto:bill.fischofer@linaro.org>> > --- > .../linux-generic/include/odp/plat/time_types.h | 9 ++++--- > platform/linux-generic/odp_time.c | 30 > +++++++++++----------- > 2 files changed, 20 insertions(+), 19 deletions(-) > > diff --git > a/platform/linux-generic/include/odp/plat/time_types.h > b/platform/linux-generic/include/odp/plat/time_types.h > index e5765ec..e999beb 100644 > --- a/platform/linux-generic/include/odp/plat/time_types.h > +++ b/platform/linux-generic/include/odp/plat/time_types.h > @@ -21,11 +21,12 @@ extern "C" { > * @{ > **/ > -typedef struct timespec odp_time_t; > +typedef struct { > + int64_t tv_sec; > > time_t is not reachable? > > Not sure I understand this comment. Can you elaborate? I think this patch is correct. First argument might be time_t, but looks like it's the same size as everywhere. Will hold patch for some small time to see of somebody have any objections. Maxim. > > + int64_t tv_nsec; > +} odp_time_t; > -odp_time_t odp_time_null(void); > - > -#define ODP_TIME_NULL odp_time_null() > +#define ODP_TIME_NULL ((odp_time_t){0, 0}) > /** > * @} > diff --git a/platform/linux-generic/odp_time.c > b/platform/linux-generic/odp_time.c > index 1c7c214..9b64b37 100644 > --- a/platform/linux-generic/odp_time.c > +++ b/platform/linux-generic/odp_time.c > @@ -11,7 +11,12 @@ > #include <odp/hints.h> > #include <odp_debug_internal.h> > -static struct timespec start_time; > +typedef union { > + odp_time_t ex; > + struct timespec in; > +} _odp_time_t; > + > +static odp_time_t start_time; > static inline > uint64_t time_to_ns(odp_time_t time) > @@ -27,7 +32,7 @@ uint64_t time_to_ns(odp_time_t time) > static inline > odp_time_t time_diff(odp_time_t t2, odp_time_t t1) > { > - struct timespec time; > + odp_time_t time; > time.tv_sec = t2.tv_sec - t1.tv_sec; > time.tv_nsec = t2.tv_nsec - t1.tv_nsec; > @@ -43,13 +48,13 @@ odp_time_t time_diff(odp_time_t t2, > odp_time_t t1) > odp_time_t odp_time_local(void) > { > int ret; > - struct timespec time; > + _odp_time_t time; > - ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time); > + ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time.in > <http://time.in>); > if (odp_unlikely(ret != 0)) > ODP_ABORT("clock_gettime failed\n"); > - return time_diff(time, start_time); > + return time_diff(time.ex, start_time); > } > odp_time_t odp_time_diff(odp_time_t t2, odp_time_t t1) > @@ -64,7 +69,7 @@ uint64_t odp_time_to_ns(odp_time_t time) > odp_time_t odp_time_local_from_ns(uint64_t ns) > { > - struct timespec time; > + odp_time_t time; > time.tv_sec = ns / ODP_TIME_SEC_IN_NS; > time.tv_nsec = ns - time.tv_sec * ODP_TIME_SEC_IN_NS; > @@ -85,7 +90,7 @@ int odp_time_cmp(odp_time_t t2, odp_time_t t1) > odp_time_t odp_time_sum(odp_time_t t1, odp_time_t t2) > { > - struct timespec time; > + odp_time_t time; > time.tv_sec = t2.tv_sec + t1.tv_sec; > time.tv_nsec = t2.tv_nsec + t1.tv_nsec; > @@ -113,18 +118,13 @@ uint64_t odp_time_to_u64(odp_time_t time) > return time_to_ns(time) / resolution; > } > -odp_time_t odp_time_null(void) > -{ > - return (struct timespec) {0, 0}; > -} > - > int odp_time_global_init(void) > { > int ret; > - struct timespec time; > + _odp_time_t time; > - ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time); > - start_time = ret ? odp_time_null() : time; > + ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time.in > <http://time.in>); > + start_time = ret ? ODP_TIME_NULL : time.ex; > return ret; > } > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> > https://lists.linaro.org/mailman/listinfo/lng-odp > >
No, that's what changed in _POSIX_C_SOURCE 200809L. If you wade through all the various levels of indirection in /usr/include/time.h in all cases it uses two signed 64-bit ints. It just calls them either long or long long depending on the environment. On Fri, Dec 11, 2015 at 10:33 AM, Savolainen, Petri (Nokia - FI/Espoo) < petri.savolainen@nokia.com> wrote: > > To me it seems that "ODP timespec" and Linux timespec have still different > field sizes when 'long' is 32 bits. Does this work in the 32 bit build? > Struct fields in the union do not point to the same locations when field > sizes do not match. > > -Petri > ------------------------------ > Lähettäjä: EXT Maxim Uvarov <maxim.uvarov@linaro.org> > Lähetetty: 11.12.2015 15:27 > Vastaanottaja: lng-odp@lists.linaro.org > Aihe: Re: [lng-odp] [API-NEXT PATCHv2] linux-generic: time: remove posix > bleed through on odp_time_t > > On 12/11/2015 16:22, Bill Fischofer wrote: > > The linux-generic implementation of odp_time_t makes use of POSIX > > APIs that are sensitive to the _POSIX_C_SOURCE level. Use an indirection > > mechanism so that these dependencies do not "bleed through" the ODP API. > > This means that ODP applications can be independent of _POSIX_C_SOURCE > > level. > > > > Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org> > > --- > > .../linux-generic/include/odp/plat/time_types.h | 9 ++++--- > > platform/linux-generic/odp_time.c | 30 > +++++++++++----------- > > 2 files changed, 20 insertions(+), 19 deletions(-) > > > > diff --git a/platform/linux-generic/include/odp/plat/time_types.h > b/platform/linux-generic/include/odp/plat/time_types.h > > index e5765ec..e999beb 100644 > > --- a/platform/linux-generic/include/odp/plat/time_types.h > > +++ b/platform/linux-generic/include/odp/plat/time_types.h > > @@ -21,11 +21,12 @@ extern "C" { > > * @{ > > **/ > > > > -typedef struct timespec odp_time_t; > > +typedef struct { > > + int64_t tv_sec; > time_t is not reachable? > > + int64_t tv_nsec; > > +} odp_time_t; > > > > -odp_time_t odp_time_null(void); > > - > > -#define ODP_TIME_NULL odp_time_null() > > +#define ODP_TIME_NULL ((odp_time_t){0, 0}) > > > > /** > > * @} > > diff --git a/platform/linux-generic/odp_time.c > b/platform/linux-generic/odp_time.c > > index 1c7c214..9b64b37 100644 > > --- a/platform/linux-generic/odp_time.c > > +++ b/platform/linux-generic/odp_time.c > > @@ -11,7 +11,12 @@ > > #include <odp/hints.h> > > #include <odp_debug_internal.h> > > > > -static struct timespec start_time; > > +typedef union { > > + odp_time_t ex; > > + struct timespec in; > > +} _odp_time_t; > > + > > +static odp_time_t start_time; > > > > static inline > > uint64_t time_to_ns(odp_time_t time) > > @@ -27,7 +32,7 @@ uint64_t time_to_ns(odp_time_t time) > > static inline > > odp_time_t time_diff(odp_time_t t2, odp_time_t t1) > > { > > - struct timespec time; > > + odp_time_t time; > > > > time.tv_sec = t2.tv_sec - t1.tv_sec; > > time.tv_nsec = t2.tv_nsec - t1.tv_nsec; > > @@ -43,13 +48,13 @@ odp_time_t time_diff(odp_time_t t2, odp_time_t t1) > > odp_time_t odp_time_local(void) > > { > > int ret; > > - struct timespec time; > > + _odp_time_t time; > > > > - ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time); > > + ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time.in); > > if (odp_unlikely(ret != 0)) > > ODP_ABORT("clock_gettime failed\n"); > > > > - return time_diff(time, start_time); > > + return time_diff(time.ex, start_time); > > } > > > > odp_time_t odp_time_diff(odp_time_t t2, odp_time_t t1) > > @@ -64,7 +69,7 @@ uint64_t odp_time_to_ns(odp_time_t time) > > > > odp_time_t odp_time_local_from_ns(uint64_t ns) > > { > > - struct timespec time; > > + odp_time_t time; > > > > time.tv_sec = ns / ODP_TIME_SEC_IN_NS; > > time.tv_nsec = ns - time.tv_sec * ODP_TIME_SEC_IN_NS; > > @@ -85,7 +90,7 @@ int odp_time_cmp(odp_time_t t2, odp_time_t t1) > > > > odp_time_t odp_time_sum(odp_time_t t1, odp_time_t t2) > > { > > - struct timespec time; > > + odp_time_t time; > > > > time.tv_sec = t2.tv_sec + t1.tv_sec; > > time.tv_nsec = t2.tv_nsec + t1.tv_nsec; > > @@ -113,18 +118,13 @@ uint64_t odp_time_to_u64(odp_time_t time) > > return time_to_ns(time) / resolution; > > } > > > > -odp_time_t odp_time_null(void) > > -{ > > - return (struct timespec) {0, 0}; > > -} > > - > > int odp_time_global_init(void) > > { > > int ret; > > - struct timespec time; > > + _odp_time_t time; > > > > - ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time); > > - start_time = ret ? odp_time_null() : time; > > + ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time.in); > > + start_time = ret ? ODP_TIME_NULL : time.ex; > > > > return ret; > > } > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > https://lists.linaro.org/mailman/listinfo/lng-odp > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > https://lists.linaro.org/mailman/listinfo/lng-odp > >
On 12/11/2015 16:22, Bill Fischofer wrote: > The linux-generic implementation of odp_time_t makes use of POSIX > APIs that are sensitive to the _POSIX_C_SOURCE level. Use an indirection > mechanism so that these dependencies do not "bleed through" the ODP API. > This means that ODP applications can be independent of _POSIX_C_SOURCE > level. > > Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org> > --- > .../linux-generic/include/odp/plat/time_types.h | 9 ++++--- > platform/linux-generic/odp_time.c | 30 +++++++++++----------- > 2 files changed, 20 insertions(+), 19 deletions(-) > > diff --git a/platform/linux-generic/include/odp/plat/time_types.h b/platform/linux-generic/include/odp/plat/time_types.h > index e5765ec..e999beb 100644 > --- a/platform/linux-generic/include/odp/plat/time_types.h > +++ b/platform/linux-generic/include/odp/plat/time_types.h > @@ -21,11 +21,12 @@ extern "C" { > * @{ > **/ > > -typedef struct timespec odp_time_t; > +typedef struct { > + int64_t tv_sec; > + int64_t tv_nsec; > +} odp_time_t; Looks like that struct should be moved to ./include/odp/api/time.h Overwise apps will relay on different time structures which platforms can redifine. Also doxygen notes are missing: /opt/Linaro/check-odp.git/build/odp-apply/include/odp/api/time.h:32: warning: documented symbol `odp_time_t' was not declared or defined. /opt/Linaro/check-odp.git/build/odp-apply/platform/linux-generic/include/odp/plat/time_types.h:25: warning: Member tv_sec (variable) of class odp_time_t is not documented. /opt/Linaro/check-odp.git/build/odp-apply/platform/linux-generic/include/odp/plat/time_types.h:26: warning: Member tv_nsec (variable) of class odp_time_t is not documented. Maxim. > > -odp_time_t odp_time_null(void); > - > -#define ODP_TIME_NULL odp_time_null() > +#define ODP_TIME_NULL ((odp_time_t){0, 0}) > > /** > * @} > diff --git a/platform/linux-generic/odp_time.c b/platform/linux-generic/odp_time.c > index 1c7c214..9b64b37 100644 > --- a/platform/linux-generic/odp_time.c > +++ b/platform/linux-generic/odp_time.c > @@ -11,7 +11,12 @@ > #include <odp/hints.h> > #include <odp_debug_internal.h> > > -static struct timespec start_time; > +typedef union { > + odp_time_t ex; > + struct timespec in; > +} _odp_time_t; > + > +static odp_time_t start_time; > > static inline > uint64_t time_to_ns(odp_time_t time) > @@ -27,7 +32,7 @@ uint64_t time_to_ns(odp_time_t time) > static inline > odp_time_t time_diff(odp_time_t t2, odp_time_t t1) > { > - struct timespec time; > + odp_time_t time; > > time.tv_sec = t2.tv_sec - t1.tv_sec; > time.tv_nsec = t2.tv_nsec - t1.tv_nsec; > @@ -43,13 +48,13 @@ odp_time_t time_diff(odp_time_t t2, odp_time_t t1) > odp_time_t odp_time_local(void) > { > int ret; > - struct timespec time; > + _odp_time_t time; > > - ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time); > + ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time.in); > if (odp_unlikely(ret != 0)) > ODP_ABORT("clock_gettime failed\n"); > > - return time_diff(time, start_time); > + return time_diff(time.ex, start_time); > } > > odp_time_t odp_time_diff(odp_time_t t2, odp_time_t t1) > @@ -64,7 +69,7 @@ uint64_t odp_time_to_ns(odp_time_t time) > > odp_time_t odp_time_local_from_ns(uint64_t ns) > { > - struct timespec time; > + odp_time_t time; > > time.tv_sec = ns / ODP_TIME_SEC_IN_NS; > time.tv_nsec = ns - time.tv_sec * ODP_TIME_SEC_IN_NS; > @@ -85,7 +90,7 @@ int odp_time_cmp(odp_time_t t2, odp_time_t t1) > > odp_time_t odp_time_sum(odp_time_t t1, odp_time_t t2) > { > - struct timespec time; > + odp_time_t time; > > time.tv_sec = t2.tv_sec + t1.tv_sec; > time.tv_nsec = t2.tv_nsec + t1.tv_nsec; > @@ -113,18 +118,13 @@ uint64_t odp_time_to_u64(odp_time_t time) > return time_to_ns(time) / resolution; > } > > -odp_time_t odp_time_null(void) > -{ > - return (struct timespec) {0, 0}; > -} > - > int odp_time_global_init(void) > { > int ret; > - struct timespec time; > + _odp_time_t time; > > - ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time); > - start_time = ret ? odp_time_null() : time; > + ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time.in); > + start_time = ret ? ODP_TIME_NULL : time.ex; > > return ret; > }
On Mon, Dec 14, 2015 at 3:24 AM, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > On 12/11/2015 16:22, Bill Fischofer wrote: > >> The linux-generic implementation of odp_time_t makes use of POSIX >> APIs that are sensitive to the _POSIX_C_SOURCE level. Use an indirection >> mechanism so that these dependencies do not "bleed through" the ODP API. >> This means that ODP applications can be independent of _POSIX_C_SOURCE >> level. >> >> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org> >> --- >> .../linux-generic/include/odp/plat/time_types.h | 9 ++++--- >> platform/linux-generic/odp_time.c | 30 >> +++++++++++----------- >> 2 files changed, 20 insertions(+), 19 deletions(-) >> >> diff --git a/platform/linux-generic/include/odp/plat/time_types.h >> b/platform/linux-generic/include/odp/plat/time_types.h >> index e5765ec..e999beb 100644 >> --- a/platform/linux-generic/include/odp/plat/time_types.h >> +++ b/platform/linux-generic/include/odp/plat/time_types.h >> @@ -21,11 +21,12 @@ extern "C" { >> * @{ >> **/ >> -typedef struct timespec odp_time_t; >> +typedef struct { >> + int64_t tv_sec; >> + int64_t tv_nsec; >> +} odp_time_t; >> > > Looks like that struct should be moved to ./include/odp/api/time.h > Overwise apps will relay on different time structures which platforms can > redifine. > odp_time_t, like other ODP types, expects a platform-dependent definition. There's no need to define the type as part of the ODP time APIs. I'll add this topic to today's arch call discussions. > > Also doxygen notes are missing: > /opt/Linaro/check-odp.git/build/odp-apply/include/odp/api/time.h:32: > warning: documented symbol `odp_time_t' was not declared or defined. > /opt/Linaro/check-odp.git/build/odp-apply/platform/linux-generic/include/odp/plat/time_types.h:25: > warning: Member tv_sec (variable) of class odp_time_t is not documented. > /opt/Linaro/check-odp.git/build/odp-apply/platform/linux-generic/include/odp/plat/time_types.h:26: > warning: Member tv_nsec (variable) of class odp_time_t is not documented. > > Maxim. > > > -odp_time_t odp_time_null(void); >> - >> -#define ODP_TIME_NULL odp_time_null() >> +#define ODP_TIME_NULL ((odp_time_t){0, 0}) >> /** >> * @} >> diff --git a/platform/linux-generic/odp_time.c >> b/platform/linux-generic/odp_time.c >> index 1c7c214..9b64b37 100644 >> --- a/platform/linux-generic/odp_time.c >> +++ b/platform/linux-generic/odp_time.c >> @@ -11,7 +11,12 @@ >> #include <odp/hints.h> >> #include <odp_debug_internal.h> >> -static struct timespec start_time; >> +typedef union { >> + odp_time_t ex; >> + struct timespec in; >> +} _odp_time_t; >> + >> +static odp_time_t start_time; >> static inline >> uint64_t time_to_ns(odp_time_t time) >> @@ -27,7 +32,7 @@ uint64_t time_to_ns(odp_time_t time) >> static inline >> odp_time_t time_diff(odp_time_t t2, odp_time_t t1) >> { >> - struct timespec time; >> + odp_time_t time; >> time.tv_sec = t2.tv_sec - t1.tv_sec; >> time.tv_nsec = t2.tv_nsec - t1.tv_nsec; >> @@ -43,13 +48,13 @@ odp_time_t time_diff(odp_time_t t2, odp_time_t t1) >> odp_time_t odp_time_local(void) >> { >> int ret; >> - struct timespec time; >> + _odp_time_t time; >> - ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time); >> + ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time.in); >> if (odp_unlikely(ret != 0)) >> ODP_ABORT("clock_gettime failed\n"); >> - return time_diff(time, start_time); >> + return time_diff(time.ex, start_time); >> } >> odp_time_t odp_time_diff(odp_time_t t2, odp_time_t t1) >> @@ -64,7 +69,7 @@ uint64_t odp_time_to_ns(odp_time_t time) >> odp_time_t odp_time_local_from_ns(uint64_t ns) >> { >> - struct timespec time; >> + odp_time_t time; >> time.tv_sec = ns / ODP_TIME_SEC_IN_NS; >> time.tv_nsec = ns - time.tv_sec * ODP_TIME_SEC_IN_NS; >> @@ -85,7 +90,7 @@ int odp_time_cmp(odp_time_t t2, odp_time_t t1) >> odp_time_t odp_time_sum(odp_time_t t1, odp_time_t t2) >> { >> - struct timespec time; >> + odp_time_t time; >> time.tv_sec = t2.tv_sec + t1.tv_sec; >> time.tv_nsec = t2.tv_nsec + t1.tv_nsec; >> @@ -113,18 +118,13 @@ uint64_t odp_time_to_u64(odp_time_t time) >> return time_to_ns(time) / resolution; >> } >> -odp_time_t odp_time_null(void) >> -{ >> - return (struct timespec) {0, 0}; >> -} >> - >> int odp_time_global_init(void) >> { >> int ret; >> - struct timespec time; >> + _odp_time_t time; >> - ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time); >> - start_time = ret ? odp_time_null() : time; >> + ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time.in); >> + start_time = ret ? ODP_TIME_NULL : time.ex; >> return ret; >> } >> > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > https://lists.linaro.org/mailman/listinfo/lng-odp >
On 12/14/2015 14:39, Bill Fischofer wrote: > > > On Mon, Dec 14, 2015 at 3:24 AM, Maxim Uvarov <maxim.uvarov@linaro.org > <mailto:maxim.uvarov@linaro.org>> wrote: > > On 12/11/2015 16:22, Bill Fischofer wrote: > > The linux-generic implementation of odp_time_t makes use of POSIX > APIs that are sensitive to the _POSIX_C_SOURCE level. Use an > indirection > mechanism so that these dependencies do not "bleed through" > the ODP API. > This means that ODP applications can be independent of > _POSIX_C_SOURCE > level. > > Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org > <mailto:bill.fischofer@linaro.org>> > --- > .../linux-generic/include/odp/plat/time_types.h | 9 ++++--- > platform/linux-generic/odp_time.c | 30 > +++++++++++----------- > 2 files changed, 20 insertions(+), 19 deletions(-) > > diff --git > a/platform/linux-generic/include/odp/plat/time_types.h > b/platform/linux-generic/include/odp/plat/time_types.h > index e5765ec..e999beb 100644 > --- a/platform/linux-generic/include/odp/plat/time_types.h > +++ b/platform/linux-generic/include/odp/plat/time_types.h > @@ -21,11 +21,12 @@ extern "C" { > * @{ > **/ > -typedef struct timespec odp_time_t; > +typedef struct { > + int64_t tv_sec; > + int64_t tv_nsec; > +} odp_time_t; > > > Looks like that struct should be moved to ./include/odp/api/time.h > Overwise apps will relay on different time structures which > platforms can redifine. > > > odp_time_t, like other ODP types, expects a platform-dependent > definition. There's no need to define the type as part of the ODP > time APIs. I'll add this topic to today's arch call discussions. In that case you need to add accessors and getters to time fields. I.e. apps can not directly relay on struct members. odp_time_sec(odp_time_t time) odp_time_sec_set(odp_time_t time) odp_time_nsec(odp_time_t time) odp_time_nsec_set(odp_time_t time) which looks ugly for me. It's better to define some common struct which apps can use directly. Maxim. > > Also doxygen notes are missing: > /opt/Linaro/check-odp.git/build/odp-apply/include/odp/api/time.h:32: > warning: documented symbol `odp_time_t' was not declared or defined. > /opt/Linaro/check-odp.git/build/odp-apply/platform/linux-generic/include/odp/plat/time_types.h:25: > warning: Member tv_sec (variable) of class odp_time_t is not > documented. > /opt/Linaro/check-odp.git/build/odp-apply/platform/linux-generic/include/odp/plat/time_types.h:26: > warning: Member tv_nsec (variable) of class odp_time_t is not > documented. > > Maxim. > > > -odp_time_t odp_time_null(void); > - > -#define ODP_TIME_NULL odp_time_null() > +#define ODP_TIME_NULL ((odp_time_t){0, 0}) > /** > * @} > diff --git a/platform/linux-generic/odp_time.c > b/platform/linux-generic/odp_time.c > index 1c7c214..9b64b37 100644 > --- a/platform/linux-generic/odp_time.c > +++ b/platform/linux-generic/odp_time.c > @@ -11,7 +11,12 @@ > #include <odp/hints.h> > #include <odp_debug_internal.h> > -static struct timespec start_time; > +typedef union { > + odp_time_t ex; > + struct timespec in; > +} _odp_time_t; > + > +static odp_time_t start_time; > static inline > uint64_t time_to_ns(odp_time_t time) > @@ -27,7 +32,7 @@ uint64_t time_to_ns(odp_time_t time) > static inline > odp_time_t time_diff(odp_time_t t2, odp_time_t t1) > { > - struct timespec time; > + odp_time_t time; > time.tv_sec = t2.tv_sec - t1.tv_sec; > time.tv_nsec = t2.tv_nsec - t1.tv_nsec; > @@ -43,13 +48,13 @@ odp_time_t time_diff(odp_time_t t2, > odp_time_t t1) > odp_time_t odp_time_local(void) > { > int ret; > - struct timespec time; > + _odp_time_t time; > - ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time); > + ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time.in > <http://time.in>); > if (odp_unlikely(ret != 0)) > ODP_ABORT("clock_gettime failed\n"); > - return time_diff(time, start_time); > + return time_diff(time.ex, start_time); > } > odp_time_t odp_time_diff(odp_time_t t2, odp_time_t t1) > @@ -64,7 +69,7 @@ uint64_t odp_time_to_ns(odp_time_t time) > odp_time_t odp_time_local_from_ns(uint64_t ns) > { > - struct timespec time; > + odp_time_t time; > time.tv_sec = ns / ODP_TIME_SEC_IN_NS; > time.tv_nsec = ns - time.tv_sec * ODP_TIME_SEC_IN_NS; > @@ -85,7 +90,7 @@ int odp_time_cmp(odp_time_t t2, odp_time_t t1) > odp_time_t odp_time_sum(odp_time_t t1, odp_time_t t2) > { > - struct timespec time; > + odp_time_t time; > time.tv_sec = t2.tv_sec + t1.tv_sec; > time.tv_nsec = t2.tv_nsec + t1.tv_nsec; > @@ -113,18 +118,13 @@ uint64_t odp_time_to_u64(odp_time_t time) > return time_to_ns(time) / resolution; > } > -odp_time_t odp_time_null(void) > -{ > - return (struct timespec) {0, 0}; > -} > - > int odp_time_global_init(void) > { > int ret; > - struct timespec time; > + _odp_time_t time; > - ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time); > - start_time = ret ? odp_time_null() : time; > + ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time.in > <http://time.in>); > + start_time = ret ? ODP_TIME_NULL : time.ex; > return ret; > } > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> > https://lists.linaro.org/mailman/listinfo/lng-odp > >
On Mon, Dec 14, 2015 at 6:40 AM, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > On 12/14/2015 14:39, Bill Fischofer wrote: > >> >> >> On Mon, Dec 14, 2015 at 3:24 AM, Maxim Uvarov <maxim.uvarov@linaro.org >> <mailto:maxim.uvarov@linaro.org>> wrote: >> >> On 12/11/2015 16:22, Bill Fischofer wrote: >> >> The linux-generic implementation of odp_time_t makes use of POSIX >> APIs that are sensitive to the _POSIX_C_SOURCE level. Use an >> indirection >> mechanism so that these dependencies do not "bleed through" >> the ODP API. >> This means that ODP applications can be independent of >> _POSIX_C_SOURCE >> level. >> >> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org >> <mailto:bill.fischofer@linaro.org>> >> --- >> .../linux-generic/include/odp/plat/time_types.h | 9 ++++--- >> platform/linux-generic/odp_time.c | 30 >> +++++++++++----------- >> 2 files changed, 20 insertions(+), 19 deletions(-) >> >> diff --git >> a/platform/linux-generic/include/odp/plat/time_types.h >> b/platform/linux-generic/include/odp/plat/time_types.h >> index e5765ec..e999beb 100644 >> --- a/platform/linux-generic/include/odp/plat/time_types.h >> +++ b/platform/linux-generic/include/odp/plat/time_types.h >> @@ -21,11 +21,12 @@ extern "C" { >> * @{ >> **/ >> -typedef struct timespec odp_time_t; >> +typedef struct { >> + int64_t tv_sec; >> + int64_t tv_nsec; >> +} odp_time_t; >> >> >> Looks like that struct should be moved to ./include/odp/api/time.h >> Overwise apps will relay on different time structures which >> platforms can redifine. >> >> >> odp_time_t, like other ODP types, expects a platform-dependent >> definition. There's no need to define the type as part of the ODP time >> APIs. I'll add this topic to today's arch call discussions. >> > > In that case you need to add accessors and getters to time fields. I.e. > apps can not directly relay on struct members. > odp_time_sec(odp_time_t time) > odp_time_sec_set(odp_time_t time) > odp_time_nsec(odp_time_t time) > odp_time_nsec_set(odp_time_t time) > > which looks ugly for me. It's better to define some common struct which > apps can use directly. > That might not be a bad extension, but they are not currently part of the API. The only manipulations on odp_time_t currently defined are the ability to convert an odp_time_t to and from nanoseconds, the ability to get the current time, and the ability to add, subtract, and compare two odp_time_t values. > > Maxim. > > >> Also doxygen notes are missing: >> /opt/Linaro/check-odp.git/build/odp-apply/include/odp/api/time.h:32: >> warning: documented symbol `odp_time_t' was not declared or defined. >> >> /opt/Linaro/check-odp.git/build/odp-apply/platform/linux-generic/include/odp/plat/time_types.h:25: >> warning: Member tv_sec (variable) of class odp_time_t is not >> documented. >> >> /opt/Linaro/check-odp.git/build/odp-apply/platform/linux-generic/include/odp/plat/time_types.h:26: >> warning: Member tv_nsec (variable) of class odp_time_t is not >> documented. >> >> Maxim. >> >> >> -odp_time_t odp_time_null(void); >> - >> -#define ODP_TIME_NULL odp_time_null() >> +#define ODP_TIME_NULL ((odp_time_t){0, 0}) >> /** >> * @} >> diff --git a/platform/linux-generic/odp_time.c >> b/platform/linux-generic/odp_time.c >> index 1c7c214..9b64b37 100644 >> --- a/platform/linux-generic/odp_time.c >> +++ b/platform/linux-generic/odp_time.c >> @@ -11,7 +11,12 @@ >> #include <odp/hints.h> >> #include <odp_debug_internal.h> >> -static struct timespec start_time; >> +typedef union { >> + odp_time_t ex; >> + struct timespec in; >> +} _odp_time_t; >> + >> +static odp_time_t start_time; >> static inline >> uint64_t time_to_ns(odp_time_t time) >> @@ -27,7 +32,7 @@ uint64_t time_to_ns(odp_time_t time) >> static inline >> odp_time_t time_diff(odp_time_t t2, odp_time_t t1) >> { >> - struct timespec time; >> + odp_time_t time; >> time.tv_sec = t2.tv_sec - t1.tv_sec; >> time.tv_nsec = t2.tv_nsec - t1.tv_nsec; >> @@ -43,13 +48,13 @@ odp_time_t time_diff(odp_time_t t2, >> odp_time_t t1) >> odp_time_t odp_time_local(void) >> { >> int ret; >> - struct timespec time; >> + _odp_time_t time; >> - ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time); >> + ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time.in >> <http://time.in>); >> >> if (odp_unlikely(ret != 0)) >> ODP_ABORT("clock_gettime failed\n"); >> - return time_diff(time, start_time); >> + return time_diff(time.ex, start_time); >> } >> odp_time_t odp_time_diff(odp_time_t t2, odp_time_t t1) >> @@ -64,7 +69,7 @@ uint64_t odp_time_to_ns(odp_time_t time) >> odp_time_t odp_time_local_from_ns(uint64_t ns) >> { >> - struct timespec time; >> + odp_time_t time; >> time.tv_sec = ns / ODP_TIME_SEC_IN_NS; >> time.tv_nsec = ns - time.tv_sec * ODP_TIME_SEC_IN_NS; >> @@ -85,7 +90,7 @@ int odp_time_cmp(odp_time_t t2, odp_time_t t1) >> odp_time_t odp_time_sum(odp_time_t t1, odp_time_t t2) >> { >> - struct timespec time; >> + odp_time_t time; >> time.tv_sec = t2.tv_sec + t1.tv_sec; >> time.tv_nsec = t2.tv_nsec + t1.tv_nsec; >> @@ -113,18 +118,13 @@ uint64_t odp_time_to_u64(odp_time_t time) >> return time_to_ns(time) / resolution; >> } >> -odp_time_t odp_time_null(void) >> -{ >> - return (struct timespec) {0, 0}; >> -} >> - >> int odp_time_global_init(void) >> { >> int ret; >> - struct timespec time; >> + _odp_time_t time; >> - ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time); >> - start_time = ret ? odp_time_null() : time; >> + ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time.in >> <http://time.in>); >> + start_time = ret ? ODP_TIME_NULL : time.ex; >> return ret; >> } >> >> >> _______________________________________________ >> 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/include/odp/plat/time_types.h b/platform/linux-generic/include/odp/plat/time_types.h index e5765ec..e999beb 100644 --- a/platform/linux-generic/include/odp/plat/time_types.h +++ b/platform/linux-generic/include/odp/plat/time_types.h @@ -21,11 +21,12 @@ extern "C" { * @{ **/ -typedef struct timespec odp_time_t; +typedef struct { + int64_t tv_sec; + int64_t tv_nsec; +} odp_time_t; -odp_time_t odp_time_null(void); - -#define ODP_TIME_NULL odp_time_null() +#define ODP_TIME_NULL ((odp_time_t){0, 0}) /** * @} diff --git a/platform/linux-generic/odp_time.c b/platform/linux-generic/odp_time.c index 1c7c214..9b64b37 100644 --- a/platform/linux-generic/odp_time.c +++ b/platform/linux-generic/odp_time.c @@ -11,7 +11,12 @@ #include <odp/hints.h> #include <odp_debug_internal.h> -static struct timespec start_time; +typedef union { + odp_time_t ex; + struct timespec in; +} _odp_time_t; + +static odp_time_t start_time; static inline uint64_t time_to_ns(odp_time_t time) @@ -27,7 +32,7 @@ uint64_t time_to_ns(odp_time_t time) static inline odp_time_t time_diff(odp_time_t t2, odp_time_t t1) { - struct timespec time; + odp_time_t time; time.tv_sec = t2.tv_sec - t1.tv_sec; time.tv_nsec = t2.tv_nsec - t1.tv_nsec; @@ -43,13 +48,13 @@ odp_time_t time_diff(odp_time_t t2, odp_time_t t1) odp_time_t odp_time_local(void) { int ret; - struct timespec time; + _odp_time_t time; - ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time); + ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time.in); if (odp_unlikely(ret != 0)) ODP_ABORT("clock_gettime failed\n"); - return time_diff(time, start_time); + return time_diff(time.ex, start_time); } odp_time_t odp_time_diff(odp_time_t t2, odp_time_t t1) @@ -64,7 +69,7 @@ uint64_t odp_time_to_ns(odp_time_t time) odp_time_t odp_time_local_from_ns(uint64_t ns) { - struct timespec time; + odp_time_t time; time.tv_sec = ns / ODP_TIME_SEC_IN_NS; time.tv_nsec = ns - time.tv_sec * ODP_TIME_SEC_IN_NS; @@ -85,7 +90,7 @@ int odp_time_cmp(odp_time_t t2, odp_time_t t1) odp_time_t odp_time_sum(odp_time_t t1, odp_time_t t2) { - struct timespec time; + odp_time_t time; time.tv_sec = t2.tv_sec + t1.tv_sec; time.tv_nsec = t2.tv_nsec + t1.tv_nsec; @@ -113,18 +118,13 @@ uint64_t odp_time_to_u64(odp_time_t time) return time_to_ns(time) / resolution; } -odp_time_t odp_time_null(void) -{ - return (struct timespec) {0, 0}; -} - int odp_time_global_init(void) { int ret; - struct timespec time; + _odp_time_t time; - ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time); - start_time = ret ? odp_time_null() : time; + ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time.in); + start_time = ret ? ODP_TIME_NULL : time.ex; return ret; }
The linux-generic implementation of odp_time_t makes use of POSIX APIs that are sensitive to the _POSIX_C_SOURCE level. Use an indirection mechanism so that these dependencies do not "bleed through" the ODP API. This means that ODP applications can be independent of _POSIX_C_SOURCE level. Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org> --- .../linux-generic/include/odp/plat/time_types.h | 9 ++++--- platform/linux-generic/odp_time.c | 30 +++++++++++----------- 2 files changed, 20 insertions(+), 19 deletions(-)