[PATCH 2/2] rtems: Add more clock tick functions

Pavel Pisa ppisa4lists at pikron.com
Fri Aug 22 20:14:50 UTC 2014


On Friday 22 of August 2014 17:24:41 Gedare Bloom wrote:
> On Fri, Aug 22, 2014 at 11:12 AM, Sebastian Huber
>
> <sebastian.huber at embedded-brains.de> wrote:
> > Add rtems_clock_ticks_later(), rtems_clock_ticks_later_us() and
> > rtems_clock_ticks_later_us().
> >
> > FIXME: Patch is incomplete.  Documentation and tests are missing.  Just
> > for API review.
> > ---
> >  cpukit/rtems/include/rtems/rtems/clock.h | 66
> > ++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+)
> >
> > diff --git a/cpukit/rtems/include/rtems/rtems/clock.h
> > b/cpukit/rtems/include/rtems/rtems/clock.h index ff71665..cee930e 100644
> > --- a/cpukit/rtems/include/rtems/rtems/clock.h
> > +++ b/cpukit/rtems/include/rtems/rtems/clock.h
> > @@ -34,6 +34,7 @@
> >  #include <rtems/score/tod.h>
> >  #include <rtems/rtems/status.h>
> >  #include <rtems/rtems/types.h>
> > +#include <rtems/config.h>
> >
> >  #include <sys/time.h> /* struct timeval */
> >
> > @@ -160,6 +161,71 @@ RTEMS_INLINE_ROUTINE rtems_interval
> > rtems_clock_get_ticks_since_boot(void) }
> >
> >  /**
> > + * @brief Returns the ticks counter value delta ticks in the future.
> > + *
> > + * @param[in] delta The ticks delta value.
> > + *
> > + * @return The tick counter value delta ticks in the future.
> > + */
> > +RTEMS_INLINE_ROUTINE rtems_interval rtems_clock_ticks_later(
> > +  rtems_interval delta
> > +)
> > +{
> > +  return _Watchdog_Ticks_since_boot + delta;
> > +}
> > +
> > +/**
> > + * @brief Returns the ticks counter value at least delta microseconds in
> > the + * future.
> > + *
> > + * @param[in] delta The delta value in microseconds.
> > + *
> > + * @return The tick counter value at least delta microseconds in the
> > future. + */
> > +RTEMS_INLINE_ROUTINE rtems_interval rtems_clock_ticks_later_us(
> > +  rtems_interval delta
> > +)
> > +{
> > +  rtems_interval us_per_tick =
> > rtems_configuration_get_microseconds_per_tick(); +
> > +  return _Watchdog_Ticks_since_boot + (delta + us_per_tick - 1) /
> > us_per_tick; +}
> > +
> > +/**
> > + * @brief Returns true if the current ticks counter value indicates a
> > time + * before the time specified by the ticks value and false
> > otherwise. + *
> > + * @param[in] ticks The ticks value.
> > + *
> > + * This can be used to write busy loops with a timeout.
> > + *
> > + * @code
> > + * status busy( void )
> > + * {
> > + *   rtems_interval timeout = rtems_clock_ticks_later_us( 10000 );
> > + *
> > + *   do {
> > + *     if ( ok() ) {
> > + *       return success;
> > + *     }
> > + *   } while ( rtems_clock_ticks_before( timeout ) );
> > + *
> > + *   return timeout;
> > + * }
> > + * @endcode
> > + *
> > + * @retval true The current ticks counter value indicates a time before
> > the + * time specified by the ticks value.
> > + * @retval false Otherwise.
> > + */
> > +RTEMS_INLINE_ROUTINE bool rtems_clock_ticks_before(
> > +  rtems_interval ticks
> > +)
> > +{
> > +  return ( (int32_t) ticks - (int32_t) _Watchdog_Ticks_since_boot ) > 0;
>
> Why not just return _Watchdog_Ticks_since_boot < ticks;

For sure not, to have correctly working overflow arithmetics
it is required to use subtraction of unsigned types and then
to limit result to signed type of same size.

Overflow of subtraction of signed types is undefined according
to the C standard. The implementation causing exception is correct
but seldom used (can be enabled for MIPS on GCC). But comparison
is often optimized. So for example next code

int fnc(int32_t x)
{
  if ((x + 0x7fffffff) < -0x10000000) {
    return 1;
  } else {
    return 0;
  }
}

can be legally optimized to

int fnc(int32_t x)
{
  return 0;
}

because for any valid x number <-0x80000000;0x7fffffff>
arithmetic value of the sum cannot be smaller than -1.
And this kind of optimization is seen in reality.

So even Sebastian's above code which tries to prevent
overflow case can be misoptimized and broken.

Correct is

  return ( (int32_t) ( (uint32_t) ticks - (uint32_t) _Watchdog_Ticks_since_boot ) ) > 0;

Because I have worry that I can often write code incorrectly
or compare types of different sizes I use special macros to declare
overflow arithmetics in my code

http://sourceforge.net/p/ulan/ulut/ci/master/tree/ulut/ul_utdefs.h#l73

May it be that my macros are real overkill but definition of

  #define ul_cyclic_gt_32(x,y) \
    ((int32_t)((uint32_t)(x)-(uint32_t)(y))>0)

  #define ul_cyclic_gt_64(x,y) \
    ((int64_t)((uint64_t)(x)-(uint64_t)(y))>0)

would worth to be included in RTEMS.

If you want to reuse som code from uLUt library directly
then its license tries to be specially friendly to RTEMS.
It lists MPL (Mozilla Public License) option which should
be OK and I declare there that GPL with linking exception
is OK for my code which declares MPL+GPL as well.

The lengthy discussion of above arithmetic operations did happen
on Linux kernel lists two or three years ago. Many more years ago,
I have discussed the same problem when IMX1 timers been revwitten
for high resolution support. And I have even experienced that time
misfunction on real hardware in some situations.
This kind of bugs are very hard to find. Be carefull there.
Eve cross-check with other sources than me. I believe I understand
the model right, but multiple independent analysis worth to
prevent troubles.

Best wishes,

              Pavel



More information about the devel mailing list