[PATCH V2] closes #3889 new defect Test needed for timer_create with CLOCK_MONOTONIC
Gedare Bloom
gedare at rtems.org
Thu Jul 15 15:29:24 UTC 2021
The first line of the commit should be shorter. Use a blank line, and
then close the ticket with the Closes #nnnn command.
https://devel.rtems.org/wiki/Developer/Git#GitCommits
We need to move this information into
https://docs.rtems.org/branches/master/eng/management.html
On Thu, Jul 8, 2021 at 1:50 PM Zacchaeus Leung <zakthertemsdev at gmail.com> wrote:
>
> ---
> ...or-timer_create-with-clock_monotonic.patch | 234 ++++++++++++++++++
> cpukit/include/rtems/posix/timer.h | 2 +-
> cpukit/posix/src/psxtimercreate.c | 86 +++----
> cpukit/posix/src/timergettime.c | 76 ++----
> 4 files changed, 300 insertions(+), 98 deletions(-)
> create mode 100644 0001-Addded-test-for-timer_create-with-clock_monotonic.patch
>
> diff --git a/0001-Addded-test-for-timer_create-with-clock_monotonic.patch b/0001-Addded-test-for-timer_create-with-clock_monotonic.patch
> new file mode 100644
> index 0000000000..33be528411
> --- /dev/null
> +++ b/0001-Addded-test-for-timer_create-with-clock_monotonic.patch
> @@ -0,0 +1,234 @@
> +From f1cac2a4b64e5076f0ceff014b78be41e5f00389 Mon Sep 17 00:00:00 2001
> +From: Zack <zakthertemsdev at outlook.com>
> +Date: Mon, 28 Jun 2021 14:23:42 -0400
> +Subject: [PATCH] Addded test for timer_create with clock_monotonic
> +
It seems that you have included a patch within your patch. Please
check your commit before you email it, to make sure only the changes
you want included are there. Since I'm not sure what changes you are
intending to send, I'm going to stop here.
A few general comments:
Avoid reformatting existing code. Keep your changes limited to
functional changes only, and send reformatting/documentation changes
separately. Make sure you write new code consistently to our style
guide. (https://docs.rtems.org/branches/master/eng/coding.html) If in
doubt, write your code consistent with code near it.
Provide all the patches for your commits that go from the current HEAD
(most recent commit on master). We can't review commits that are based
on other commits on your local repo only, unless you include those
commits also for review.
> +---
> + cpukit/include/rtems/posix/timer.h | 1 +
> + cpukit/posix/src/psxtimercreate.c | 5 +-
> + cpukit/posix/src/timergettime.c | 79 ++++++++++++++++-------
> + testsuites/psxtests/psxtimer02/psxtimer.c | 38 ++++++++++-
> + 4 files changed, 98 insertions(+), 25 deletions(-)
> +
> +diff --git a/cpukit/include/rtems/posix/timer.h b/cpukit/include/rtems/posix/timer.h
> +index bcbf07a65a..839fe3293c 100644
> +--- a/cpukit/include/rtems/posix/timer.h
> ++++ b/cpukit/include/rtems/posix/timer.h
> +@@ -48,6 +48,7 @@ typedef struct {
> + uint32_t ticks; /* Number of ticks of the initialization */
> + uint32_t overrun; /* Number of expirations of the timer */
> + struct timespec time; /* Time at which the timer was started */
> ++ clockid_t clock_type;
> + } POSIX_Timer_Control;
> +
> + /**
> +diff --git a/cpukit/posix/src/psxtimercreate.c b/cpukit/posix/src/psxtimercreate.c
> +index a63cf1d100..b60be3f229 100644
> +--- a/cpukit/posix/src/psxtimercreate.c
> ++++ b/cpukit/posix/src/psxtimercreate.c
> +@@ -40,7 +40,7 @@ int timer_create(
> + {
> + POSIX_Timer_Control *ptimer;
> +
> +- if ( clock_id != CLOCK_REALTIME )
> ++ if ( clock_id != CLOCK_REALTIME && clock_id != CLOCK_MONOTONIC )
> + rtems_set_errno_and_return_minus_one( EINVAL );
> +
> + if ( !timerid )
> +@@ -91,7 +91,8 @@ int timer_create(
> + ptimer->timer_data.it_value.tv_nsec = 0;
> + ptimer->timer_data.it_interval.tv_sec = 0;
> + ptimer->timer_data.it_interval.tv_nsec = 0;
> +-
> ++ ptimer->clock_type=clock_id;
> ++
> + _Watchdog_Preinitialize( &ptimer->Timer, _Per_CPU_Get_snapshot() );
> + _Watchdog_Initialize( &ptimer->Timer, _POSIX_Timer_TSR );
> + _Objects_Open_u32(&_POSIX_Timer_Information, &ptimer->Object, 0);
> +diff --git a/cpukit/posix/src/timergettime.c b/cpukit/posix/src/timergettime.c
> +index ee2a566f0e..57b0ab4918 100644
> +--- a/cpukit/posix/src/timergettime.c
> ++++ b/cpukit/posix/src/timergettime.c
> +@@ -6,6 +6,14 @@
> + * @brief Function Fetches State of POSIX Per-Process Timers
> + */
> +
> ++/**
> ++ * @file
> ++ *
> ++ * @ingroup POSIXAPI
> ++ *
> ++ * @brief Function Fetches State of POSIX Per-Process Timers
> ++ */
> ++
> + /*
> + * 14.2.4 Per-Process Timers, P1003.1b-1993, p. 267
> + *
> +@@ -21,13 +29,13 @@
> + #include "config.h"
> + #endif
> +
> +-#include <time.h>
> + #include <errno.h>
> +-
> + #include <rtems/posix/timerimpl.h>
> + #include <rtems/score/todimpl.h>
> + #include <rtems/score/watchdogimpl.h>
> + #include <rtems/seterr.h>
> ++#include <rtems/timespec.h>
> ++#include <time.h>
> +
> + /*
> + * - When a timer is initialized, the value of the time in
> +@@ -36,38 +44,65 @@
> + * between the current time and the initialization time.
> + */
> +
> +-int timer_gettime(
> +- timer_t timerid,
> +- struct itimerspec *value
> +-)
> +-{
> ++int timer_gettime(timer_t timerid, struct itimerspec *value) {
> + POSIX_Timer_Control *ptimer;
> +- ISR_lock_Context lock_context;
> +- uint64_t now;
> +- uint32_t remaining;
> ++ ISR_lock_Context lock_context;
> ++ uint32_t remaining;
> ++ Per_CPU_Control *cpu;
> ++ struct timespec *now; // get time now either with
> ++ struct timespec *expire; // expire
> ++
> ++ struct timespec *result;
> ++
> ++ if (!value)
> ++ rtems_set_errno_and_return_minus_one(EINVAL);
> +
> +- if ( !value )
> +- rtems_set_errno_and_return_minus_one( EINVAL );
> ++ ptimer = _POSIX_Timer_Get(timerid, &lock_context);
> ++ if (ptimer == NULL) {
> ++ rtems_set_errno_and_return_minus_one(EINVAL);
> ++ }
> ++
> ++ if (ptimer->clock_type == CLOCK_REALTIME) {
> +
> +- ptimer = _POSIX_Timer_Get( timerid, &lock_context );
> +- if ( ptimer != NULL ) {
> +- Per_CPU_Control *cpu;
> ++ cpu = _POSIX_Timer_Acquire_critical(ptimer, &lock_context);
> +
> +- cpu = _POSIX_Timer_Acquire_critical( ptimer, &lock_context );
> +- now = cpu->Watchdog.ticks;
> ++ _TOD_Get(now); // get current time
> ++ rtems_timespec_from_ticks(ptimer->Timer.expire,
> ++ expire);
> +
> +- if ( now < ptimer->Timer.expire ) {
> +- remaining = (uint32_t) ( ptimer->Timer.expire - now );
> ++ if (now->tv_nsec + now->tv_sec >
> ++ expire->tv_nsec + expire->tv_sec) {
> ++ rtems_timespec_subtract(now, expire, result);
> ++
> ++ remaining = (uint32_t)result->tv_nsec + result->tv_sec;
> + } else {
> + remaining = 0;
> + }
> +
> +- _Timespec_From_ticks( remaining, &value->it_value );
> ++ _Timespec_From_ticks(remaining, &value->it_value);
> + value->it_interval = ptimer->timer_data.it_interval;
> ++ _POSIX_Timer_Release(cpu, &lock_context);
> ++ return 0;
> ++ }
> ++
> ++ if (ptimer->clock_type == CLOCK_MONOTONIC) {
> ++
> ++ cpu = _POSIX_Timer_Acquire_critical(ptimer, &lock_context);
> ++ rtems_timespec_from_ticks(ptimer->Timer.expire, expire);
> +
> +- _POSIX_Timer_Release( cpu, &lock_context );
> ++ if (now->tv_nsec + now->tv_sec > expire->tv_nsec + expire->tv_sec) {
> ++
> ++ rtems_timespec_subtract(now, expire, result);
> ++ remaining = (uint32_t)result->tv_nsec + result->tv_sec;
> ++ } else {
> ++ remaining = 0;
> ++ }
> ++
> ++ _Timespec_From_ticks(remaining, &value->it_value);
> ++ value->it_interval = ptimer->timer_data.it_interval;
> ++ _POSIX_Timer_Release(cpu, &lock_context);
> + return 0;
> + }
> +
> +- rtems_set_errno_and_return_minus_one( EINVAL );
> ++ rtems_set_errno_and_return_minus_one(EINVAL);
> + }
> +diff --git a/testsuites/psxtests/psxtimer02/psxtimer.c b/testsuites/psxtests/psxtimer02/psxtimer.c
> +index 9f79d33c42..6aea61c498 100644
> +--- a/testsuites/psxtests/psxtimer02/psxtimer.c
> ++++ b/testsuites/psxtests/psxtimer02/psxtimer.c
> +@@ -20,6 +20,7 @@
> + * http://www.rtems.org/license/LICENSE.
> + */
> +
> ++
> + #ifdef HAVE_CONFIG_H
> + #include "config.h"
> + #endif
> +@@ -63,7 +64,7 @@ void *POSIX_Init (
> + fatal_posix_service_status_errno( status, EINVAL, "bad timer id" );
> +
> + puts( "timer_create - OK" );
> +- status = timer_create( CLOCK_REALTIME, NULL, &timer );
> ++ status = timer_create( CLOCK_REALTIME , NULL, &timer );
> + posix_service_failed( status, "timer_create OK" );
> +
> + puts( "timer_create - too many - EAGAIN" );
> +@@ -127,6 +128,41 @@ void *POSIX_Init (
> + status = timer_delete( timer );
> + fatal_posix_service_status_errno( status, EINVAL, "bad id" );
> +
> ++
> ++ /*
> ++ * If these are not filled in correctly, we don't pass its error checking.
> ++ */
> ++
> ++
> ++puts( "timer_create - bad timer id pointer - EINVAL" );
> ++ status = timer_create( CLOCK_MONOTONIC, &event, NULL );
> ++ fatal_posix_service_status_errno( status, EINVAL, "bad timer id" );
> ++
> ++ puts( "timer_create - OK" );
> ++ status = timer_create( CLOCK_MONOTONIC, NULL, &timer );
> ++ posix_service_failed( status, "timer_create OK" );
> ++
> ++ puts( "timer_create - too many - EAGAIN" );
> ++ status = timer_create( CLOCK_MONOTONIC, NULL, &timer1 );
> ++ fatal_posix_service_status_errno( status, EAGAIN, "too many" );
> ++
> ++ clock_gettime( CLOCK_MONOTONIC, &now );
> ++ itimer.it_value = now;
> ++ itimer.it_value.tv_sec = itimer.it_value.tv_sec - 1;
> ++ puts( "timer_settime - bad itimer value - previous time - EINVAL" );
> ++ status = timer_settime( timer, TIMER_ABSTIME, &itimer, NULL );
> ++ fatal_posix_service_status_errno( status, EINVAL, "bad itimer value #3" );
> ++
> ++ clock_gettime( CLOCK_MONOTONIC, &now );
> ++ itimer.it_value = now;
> ++ itimer.it_value.tv_sec = itimer.it_value.tv_sec + 1;
> ++ puts( "timer_settime - bad id - EINVAL" );
> ++ status = timer_settime( timer1, TIMER_ABSTIME, &itimer, NULL );
> ++ fatal_posix_service_status_errno( status, EINVAL, "bad id" );
> ++
> ++
> ++
> ++
> + TEST_END();
> + rtems_test_exit (0);
> + }
> +--
> +2.31.1
> +
> diff --git a/cpukit/include/rtems/posix/timer.h b/cpukit/include/rtems/posix/timer.h
> index 839fe3293c..fdfa1e0b2f 100644
> --- a/cpukit/include/rtems/posix/timer.h
> +++ b/cpukit/include/rtems/posix/timer.h
> @@ -48,7 +48,7 @@ typedef struct {
> uint32_t ticks; /* Number of ticks of the initialization */
> uint32_t overrun; /* Number of expirations of the timer */
> struct timespec time; /* Time at which the timer was started */
> - clockid_t clock_type;
> + clockid_t clock_type; /* The type of timer */
> } POSIX_Timer_Control;
>
> /**
> diff --git a/cpukit/posix/src/psxtimercreate.c b/cpukit/posix/src/psxtimercreate.c
> index b60be3f229..18af8a6582 100644
> --- a/cpukit/posix/src/psxtimercreate.c
> +++ b/cpukit/posix/src/psxtimercreate.c
> @@ -21,9 +21,9 @@
> #include "config.h"
> #endif
>
> -#include <time.h>
> #include <errno.h>
> #include <signal.h>
> +#include <time.h>
>
> #include <rtems/posix/sigset.h>
> #include <rtems/posix/timerimpl.h>
> @@ -32,83 +32,73 @@
> #include <rtems/seterr.h>
> #include <rtems/sysinit.h>
>
> -int timer_create(
> - clockid_t clock_id,
> - struct sigevent *__restrict evp,
> - timer_t *__restrict timerid
> -)
> -{
> +int timer_create(clockid_t clock_id, struct sigevent *__restrict evp,
> + timer_t *__restrict timerid) {
> POSIX_Timer_Control *ptimer;
>
> - if ( clock_id != CLOCK_REALTIME && clock_id != CLOCK_MONOTONIC )
> - rtems_set_errno_and_return_minus_one( EINVAL );
> + if (clock_id != CLOCK_REALTIME && clock_id != CLOCK_MONOTONIC)
> + rtems_set_errno_and_return_minus_one(EINVAL);
>
> - if ( !timerid )
> - rtems_set_errno_and_return_minus_one( EINVAL );
> + if (!timerid)
> + rtems_set_errno_and_return_minus_one(EINVAL);
>
> - /*
> - * The data of the structure evp are checked in order to verify if they
> - * are coherent.
> - */
> + /*
> + * The data of the structure evp are checked in order to verify if they
> + * are coherent.
> + */
>
> if (evp != NULL) {
> /* The structure has data */
> - if ( ( evp->sigev_notify != SIGEV_NONE ) &&
> - ( evp->sigev_notify != SIGEV_SIGNAL ) ) {
> - /* The value of the field sigev_notify is not valid */
> - rtems_set_errno_and_return_minus_one( EINVAL );
> - }
> + if ((evp->sigev_notify != SIGEV_NONE) &&
> + (evp->sigev_notify != SIGEV_SIGNAL)) {
> + /* The value of the field sigev_notify is not valid */
> + rtems_set_errno_and_return_minus_one(EINVAL);
> + }
>
> - if ( !evp->sigev_signo )
> - rtems_set_errno_and_return_minus_one( EINVAL );
> + if (!evp->sigev_signo)
> + rtems_set_errno_and_return_minus_one(EINVAL);
>
> - if ( !is_valid_signo(evp->sigev_signo) )
> - rtems_set_errno_and_return_minus_one( EINVAL );
> + if (!is_valid_signo(evp->sigev_signo))
> + rtems_set_errno_and_return_minus_one(EINVAL);
> }
>
> /*
> * Allocate a timer
> */
> ptimer = _POSIX_Timer_Allocate();
> - if ( !ptimer ) {
> + if (!ptimer) {
> _Objects_Allocator_unlock();
> - rtems_set_errno_and_return_minus_one( EAGAIN );
> + rtems_set_errno_and_return_minus_one(EAGAIN);
> }
>
> /* The data of the created timer are stored to use them later */
>
> - ptimer->state = POSIX_TIMER_STATE_CREATE_NEW;
> + ptimer->state = POSIX_TIMER_STATE_CREATE_NEW;
> ptimer->thread_id = _Thread_Get_executing()->Object.id;
>
> - if ( evp != NULL ) {
> + if (evp != NULL) {
> ptimer->inf.sigev_notify = evp->sigev_notify;
> - ptimer->inf.sigev_signo = evp->sigev_signo;
> - ptimer->inf.sigev_value = evp->sigev_value;
> + ptimer->inf.sigev_signo = evp->sigev_signo;
> + ptimer->inf.sigev_value = evp->sigev_value;
> }
>
> - ptimer->overrun = 0;
> - ptimer->timer_data.it_value.tv_sec = 0;
> - ptimer->timer_data.it_value.tv_nsec = 0;
> - ptimer->timer_data.it_interval.tv_sec = 0;
> + ptimer->overrun = 0;
> + ptimer->timer_data.it_value.tv_sec = 0;
> + ptimer->timer_data.it_value.tv_nsec = 0;
> + ptimer->timer_data.it_interval.tv_sec = 0;
> ptimer->timer_data.it_interval.tv_nsec = 0;
> - ptimer->clock_type=clock_id;
> -
> - _Watchdog_Preinitialize( &ptimer->Timer, _Per_CPU_Get_snapshot() );
> - _Watchdog_Initialize( &ptimer->Timer, _POSIX_Timer_TSR );
> + ptimer->clock_type = clock_id;
> + _Watchdog_Preinitialize(&ptimer->Timer, _Per_CPU_Get_snapshot());
> + _Watchdog_Initialize(&ptimer->Timer, _POSIX_Timer_TSR);
> _Objects_Open_u32(&_POSIX_Timer_Information, &ptimer->Object, 0);
> -
> - *timerid = ptimer->Object.id;
> + *timerid = ptimer->Object.id;
> _Objects_Allocator_unlock();
> return 0;
> }
>
> -static void _POSIX_Timer_Manager_initialization( void )
> -{
> - _Objects_Initialize_information( &_POSIX_Timer_Information );
> +static void _POSIX_Timer_Manager_initialization(void) {
> + _Objects_Initialize_information(&_POSIX_Timer_Information);
> }
>
> -RTEMS_SYSINIT_ITEM(
> - _POSIX_Timer_Manager_initialization,
> - RTEMS_SYSINIT_POSIX_TIMER,
> - RTEMS_SYSINIT_ORDER_MIDDLE
> -);
> +RTEMS_SYSINIT_ITEM(_POSIX_Timer_Manager_initialization,
> + RTEMS_SYSINIT_POSIX_TIMER, RTEMS_SYSINIT_ORDER_MIDDLE);
> diff --git a/cpukit/posix/src/timergettime.c b/cpukit/posix/src/timergettime.c
> index 57b0ab4918..ee9d37ccdd 100644
> --- a/cpukit/posix/src/timergettime.c
> +++ b/cpukit/posix/src/timergettime.c
> @@ -1,10 +1,3 @@
> -/**
> - * @file
> - *
> - * @ingroup POSIXAPI
> - *
> - * @brief Function Fetches State of POSIX Per-Process Timers
> - */
>
> /**
> * @file
> @@ -30,12 +23,13 @@
> #endif
>
> #include <errno.h>
> +#include <time.h>
> +
> #include <rtems/posix/timerimpl.h>
> #include <rtems/score/todimpl.h>
> #include <rtems/score/watchdogimpl.h>
> #include <rtems/seterr.h>
> #include <rtems/timespec.h>
> -#include <time.h>
>
> /*
> * - When a timer is initialized, the value of the time in
> @@ -44,15 +38,18 @@
> * between the current time and the initialization time.
> */
>
> -int timer_gettime(timer_t timerid, struct itimerspec *value) {
> +int timer_gettime(
> + timer_t timerid,
> + struct itimerspec *value
> +)
> +{
> POSIX_Timer_Control *ptimer;
> ISR_lock_Context lock_context;
> uint32_t remaining;
> Per_CPU_Control *cpu;
> - struct timespec *now; // get time now either with
> - struct timespec *expire; // expire
> -
> - struct timespec *result;
> + struct timespec *now;
> + struct timespec *expire;
> + struct timespec *result;
>
> if (!value)
> rtems_set_errno_and_return_minus_one(EINVAL);
> @@ -62,47 +59,28 @@ int timer_gettime(timer_t timerid, struct itimerspec *value) {
> rtems_set_errno_and_return_minus_one(EINVAL);
> }
>
> - if (ptimer->clock_type == CLOCK_REALTIME) {
> -
> - cpu = _POSIX_Timer_Acquire_critical(ptimer, &lock_context);
> -
> - _TOD_Get(now); // get current time
> - rtems_timespec_from_ticks(ptimer->Timer.expire,
> - expire);
> -
> - if (now->tv_nsec + now->tv_sec >
> - expire->tv_nsec + expire->tv_sec) {
> - rtems_timespec_subtract(now, expire, result);
> -
> - remaining = (uint32_t)result->tv_nsec + result->tv_sec;
> - } else {
> - remaining = 0;
> - }
> -
> - _Timespec_From_ticks(remaining, &value->it_value);
> - value->it_interval = ptimer->timer_data.it_interval;
> - _POSIX_Timer_Release(cpu, &lock_context);
> - return 0;
> - }
> + cpu = _POSIX_Timer_Acquire_critical(ptimer, &lock_context);
> + rtems_timespec_from_ticks(ptimer->Timer.expire, expire);
>
> if (ptimer->clock_type == CLOCK_MONOTONIC) {
> + _Timecounter_Nanouptime(now);
> +
> + }
> + if (ptimer->clock_type == CLOCK_REALTIME) {
> + _TOD_Get(now);
> + }
>
> - cpu = _POSIX_Timer_Acquire_critical(ptimer, &lock_context);
> - rtems_timespec_from_ticks(ptimer->Timer.expire, expire);
> -
> - if (now->tv_nsec + now->tv_sec > expire->tv_nsec + expire->tv_sec) {
>
> + if( rtems_timespec_less_than( now, expire )) {
> rtems_timespec_subtract(now, expire, result);
> - remaining = (uint32_t)result->tv_nsec + result->tv_sec;
> } else {
> - remaining = 0;
> + result->tv_nsec=0;
> + result->tv_sec=0;
> }
> -
> - _Timespec_From_ticks(remaining, &value->it_value);
> - value->it_interval = ptimer->timer_data.it_interval;
> - _POSIX_Timer_Release(cpu, &lock_context);
> - return 0;
> - }
> -
> - rtems_set_errno_and_return_minus_one(EINVAL);
> +
> + (*value).it_value=*result;
> + value->it_interval = ptimer->timer_data.it_interval;
> +
> + _POSIX_Timer_Release(cpu, &lock_context);
> + return 0;
> }
> --
> 2.32.0
>
> _______________________________________________
> devel mailing list
> devel at rtems.org
> http://lists.rtems.org/mailman/listinfo/devel
More information about the devel
mailing list