<div dir="ltr"><div><div><div><div>Hello Gedare,<br><br></div>I take the latest version of RTEMS eae4541d7b8001aa18c6fc2d62b323<wbr>85b0310125 to integrate my patch this time.<br></div></div><div>All you mentioned coding-convention issues are solved accordingly I think.<br>I will hold the copyright by myself.<br></div><div><br>rtems_rate_monotonic_<wbr>postponed_num() this function is prepared for run-time monitoring. <br>The user can call this function to check the number of postponed jobs in run-time. <br>Therefore I name it with rtems prefix as the feature for users. <br>Should I add the description of this function in the documentation? <br><br></div>Best,<br></div>Kuan-Hsun <br></div><div class="gmail_extra"><br><div class="gmail_quote">2016-12-20 23:10 GMT+01:00 Gedare Bloom <span dir="ltr"><<a href="mailto:gedare@rtems.org" target="_blank">gedare@rtems.org</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Fri, Dec 9, 2016 at 11:21 AM, Kuan-Hsun Chen <<a href="mailto:c0066c@gmail.com">c0066c@gmail.com</a>> wrote:<br>
> Three additional functions:<br>
> RM_Postponed_num, RM_Renew_deadline, and RM_Release_postponedjob.<br>
><br>
> Four refined functions:<br>
> RM_Activate, RM_Block_while_expired, rtems_rate_monotonic_period, RM_Timeout.<br>
><br>
> Rate_monotonic_Control contains one counter for counting the postponed jobs and one for recording the recent deadline.<br>
> ---<br>
> cpukit/rtems/include/rtems/<wbr>rtems/ratemon.h | 42 ++++++--<br>
> cpukit/rtems/include/rtems/<wbr>rtems/ratemonimpl.h | 25 +++--<br>
> cpukit/rtems/src/<wbr>ratemonperiod.c | 144 +++++++++++++++++++++----<br>
> cpukit/rtems/src/<wbr>ratemontimeout.c | 13 ++-<br>
> 4 files changed, 183 insertions(+), 41 deletions(-)<br>
><br>
> diff --git a/cpukit/rtems/include/rtems/<wbr>rtems/ratemon.h b/cpukit/rtems/include/rtems/<wbr>rtems/ratemon.h<br>
> index 50b8478..71a99dc 100644<br>
> --- a/cpukit/rtems/include/rtems/<wbr>rtems/ratemon.h<br>
> +++ b/cpukit/rtems/include/rtems/<wbr>rtems/ratemon.h<br>
> @@ -22,6 +22,7 @@<br>
><br>
> /* COPYRIGHT (c) 1989-2009, 2016.<br>
> * On-Line Applications Research Corporation (OAR).<br>
> + * COPYRIGHT (c) 2016 Kuan-Hsun Chen, TU Dortmund University (TUDo).<br>
> *<br>
> * The license and distribution terms for this file may be<br>
> * found in the file LICENSE in this distribution or at<br>
> @@ -194,11 +195,6 @@ typedef struct {<br>
> /** This field is the object management portion of a Period instance. */<br>
> Objects_Control Object;<br>
><br>
> - /**<br>
> - * @brief Protects the rate monotonic period state.<br>
> - */<br>
> - ISR_LOCK_MEMBER( Lock )<br>
> -<br>
</div></div>Why are these removed?<br>
<span class=""><br>
> /** This is the timer used to provide the unblocking mechanism. */<br>
> Watchdog_Control Timer;<br>
><br>
> @@ -206,12 +202,6 @@ typedef struct {<br>
> rtems_rate_monotonic_period_<wbr>states state;<br>
><br>
> /**<br>
> - * @brief A priority node for use by the scheduler job release and cancel<br>
> - * operations.<br>
> - */<br>
> - Priority_Node Priority;<br>
> -<br>
> - /**<br>
</span>Ditto.<br>
<div><div class="h5"><br>
> * This field contains the length of the next period to be<br>
> * executed.<br>
> */<br>
> @@ -240,6 +230,19 @@ typedef struct {<br>
> * This field contains the statistics maintained for the period.<br>
> */<br>
> Rate_monotonic_Statistics Statistics;<br>
> +<br>
> + /**<br>
> + * This field contains the number of postponed jobs. When the watchdog timeout,<br>
> + * this variable will be increased immediately.<br>
> + */<br>
> + uint32_t postponed_jobs;<br>
> +<br>
> + /**<br>
> + * This field contains the tick of the latest deadline decided by the period<br>
> + * watchdog.<br>
> + */<br>
> + uint64_t latest_deadline;<br>
> +<br>
> } Rate_monotonic_Control;<br>
><br>
> /**<br>
> @@ -386,6 +389,23 @@ void rtems_rate_monotonic_report_<wbr>statistics_with_plugin(<br>
> void rtems_rate_monotonic_report_<wbr>statistics( void );<br>
><br>
> /**<br>
> + * @brief RTEMS Return the number of postponed jobs<br>
</div></div>remove "RTEMS".<br>
<span class=""><br>
> + *<br>
> + * This is a helper function to return the number of postponed jobs by this<br>
</span>by -> in<br>
<span class=""><br>
> + * given period. This number is only increased by the corresponding watchdog,<br>
</span>Is it only in a given period, or is it postponed jobs generally, i.e.<br>
you could miss multiple periods hence jobs in multiple periods would<br>
be counted here?<br>
<span class=""><br>
> + * and is decreased by RMS manager with the postponed job releasing.<br>
> + *<br>
> + * @param[in] id is the period id<br>
> + *<br>
> + * @retval This helper function returns the number of postponed<br>
> + * jobs with given period_id.<br>
> + *<br>
> + */<br>
> +uint32_t rtems_rate_monotonic_<wbr>Postponed_num(<br>
</span>See coding conventions for naming rules. this is better<br>
'rtems_rate_monotonic_<wbr>postponed_jobs(). Is this function needed in the<br>
public-facing API?<br>
<div><div class="h5"><br>
> + rtems_id period_id<br>
> +);<br>
> +<br>
> +/**<br>
> * @brief RTEMS Rate Monotonic Period<br>
> *<br>
> * This routine implements the rtems_rate_monotonic_period directive. When<br>
> diff --git a/cpukit/rtems/include/rtems/<wbr>rtems/ratemonimpl.h b/cpukit/rtems/include/rtems/<wbr>rtems/ratemonimpl.h<br>
> index b6b3ffd..6cdaaeb 100644<br>
> --- a/cpukit/rtems/include/rtems/<wbr>rtems/ratemonimpl.h<br>
> +++ b/cpukit/rtems/include/rtems/<wbr>rtems/ratemonimpl.h<br>
> @@ -9,6 +9,7 @@<br>
> /* COPYRIGHT (c) 1989-2008.<br>
> * On-Line Applications Research Corporation (OAR).<br>
> * Copyright (c) 2016 embedded brains GmbH.<br>
> + * COPYRIGHT (c) 2016 Kuan-Hsun Chen, TU Dortmund University (TUDo).<br>
> *<br>
> * The license and distribution terms for this file may be<br>
> * found in the file LICENSE in this distribution or at<br>
> @@ -69,19 +70,19 @@ RTEMS_INLINE_ROUTINE Rate_monotonic_Control *_Rate_monotonic_Allocate( void )<br>
> }<br>
><br>
> RTEMS_INLINE_ROUTINE void _Rate_monotonic_Acquire_<wbr>critical(<br>
> - Rate_monotonic_Control *the_period,<br>
> - ISR_lock_Context *lock_context<br>
> + Thread_Control *the_thread,<br>
> + ISR_lock_Context *lock_context<br>
> )<br>
> {<br>
> - _ISR_lock_Acquire( &the_period->Lock, lock_context );<br>
> + _Thread_Wait_acquire_default_<wbr>critical( the_thread, lock_context );<br>
> }<br>
</div></div>At what version did you make your patch? These lock changes are weird.<br>
<div><div class="h5"><br>
><br>
> RTEMS_INLINE_ROUTINE void _Rate_monotonic_Release(<br>
> - Rate_monotonic_Control *the_period,<br>
> - ISR_lock_Context *lock_context<br>
> + Thread_Control *the_thread,<br>
> + ISR_lock_Context *lock_context<br>
> )<br>
> {<br>
> - _ISR_lock_Release_and_ISR_<wbr>enable( &the_period->Lock, lock_context );<br>
> + _Thread_Wait_release_default( the_thread, lock_context );<br>
> }<br>
><br>
> RTEMS_INLINE_ROUTINE Rate_monotonic_Control *_Rate_monotonic_Get(<br>
> @@ -116,6 +117,18 @@ bool _Rate_monotonic_Get_status(<br>
> Timestamp_Control *cpu_since_last_period<br>
> );<br>
><br>
> +/**<br>
> + * @brief Renew the watchdog deadline<br>
> + *<br>
> + * This routine is prepared for the watchdog timeout to renew its deadline<br>
> + * without releasing jobs.<br>
> + */<br>
> +void _Rate_monotonic_Renew_<wbr>deadline(<br>
> + Rate_monotonic_Control *the_period,<br>
> + Thread_Control *owner,<br>
> + ISR_lock_Context *lock_context<br>
> +);<br>
> +<br>
> void _Rate_monotonic_Restart(<br>
> Rate_monotonic_Control *the_period,<br>
> Thread_Control *owner,<br>
> diff --git a/cpukit/rtems/src/<wbr>ratemonperiod.c b/cpukit/rtems/src/<wbr>ratemonperiod.c<br>
> index 77bd996..26cee58 100644<br>
> --- a/cpukit/rtems/src/<wbr>ratemonperiod.c<br>
> +++ b/cpukit/rtems/src/<wbr>ratemonperiod.c<br>
> @@ -9,6 +9,7 @@<br>
> * COPYRIGHT (c) 1989-2010.<br>
> * On-Line Applications Research Corporation (OAR).<br>
> * Copyright (c) 2016 embedded brains GmbH.<br>
> + * COPYRIGHT (c) 2016 Kuan-Hsun Chen, TU Dortmund University (TUDo).<br>
> *<br>
> * The license and distribution terms for this file may be<br>
> * found in the file LICENSE in this distribution or at<br>
> @@ -63,6 +64,24 @@ bool _Rate_monotonic_Get_status(<br>
> return true;<br>
> }<br>
><br>
> +static void _Rate_monotonic_Release_<wbr>postponedjob(<br>
</div></div>better; Release_postponed_job<br>
<div><div class="h5"><br>
> + Rate_monotonic_Control *the_period,<br>
> + Thread_Control *owner,<br>
> + rtems_interval next_length,<br>
> + ISR_lock_Context *lock_context<br>
> +)<br>
> +{<br>
> + /* This function only releases the postponed jobs. */<br>
> + Per_CPU_Control *cpu_self;<br>
> + cpu_self = _Thread_Dispatch_disable_<wbr>critical( lock_context );<br>
> + _Rate_monotonic_Release( owner, lock_context );<br>
> +<br>
> + the_period->postponed_jobs -=1;<br>
> + _Scheduler_Release_job( owner, the_period->latest_deadline );<br>
> +<br>
> + _Thread_Dispatch_enable( cpu_self );<br>
> +}<br>
> +<br>
> static void _Rate_monotonic_Release_job(<br>
> Rate_monotonic_Control *the_period,<br>
> Thread_Control *owner,<br>
> @@ -70,29 +89,49 @@ static void _Rate_monotonic_Release_job(<br>
> ISR_lock_Context *lock_context<br>
> )<br>
> {<br>
> - Per_CPU_Control *cpu_self;<br>
> - Thread_queue_Context queue_context;<br>
> - uint64_t deadline;<br>
> + Per_CPU_Control *cpu_self;<br>
> + uint64_t deadline;<br>
><br>
> cpu_self = _Thread_Dispatch_disable_<wbr>critical( lock_context );<br>
> + _Rate_monotonic_Release( owner, lock_context );<br>
><br>
> + _ISR_lock_ISR_disable( lock_context );<br>
> deadline = _Watchdog_Per_CPU_insert_<wbr>relative(<br>
> &the_period->Timer,<br>
> cpu_self,<br>
> next_length<br>
> );<br>
> - _Scheduler_Release_job(<br>
> - owner,<br>
> - &the_period->Priority,<br>
> - deadline,<br>
> - &queue_context<br>
> - );<br>
> + _ISR_lock_ISR_enable( lock_context );<br>
> +<br>
> + _Scheduler_Release_job( owner, deadline );<br>
><br>
> - _Rate_monotonic_Release( the_period, lock_context );<br>
> - _Thread_Priority_update( &queue_context );<br>
> _Thread_Dispatch_enable( cpu_self );<br>
> }<br>
><br>
> +void _Rate_monotonic_Renew_<wbr>deadline(<br>
> + Rate_monotonic_Control *the_period,<br>
> + Thread_Control *owner,<br>
> + ISR_lock_Context *lock_context<br>
> +)<br>
> +{<br>
> + Per_CPU_Control *cpu_self;<br>
> + uint64_t deadline;<br>
> +<br>
> + cpu_self = _Thread_Dispatch_disable_<wbr>critical( lock_context );<br>
> + _Rate_monotonic_Release( owner, lock_context );<br>
> +<br>
> + _ISR_lock_ISR_disable( lock_context );<br>
> + deadline = _Watchdog_Per_CPU_insert_<wbr>relative(<br>
> + &the_period->Timer,<br>
> + cpu_self,<br>
> + the_period->next_length<br>
> + );<br>
> + the_period->latest_deadline = deadline;<br>
> + _ISR_lock_ISR_enable( lock_context );<br>
> + _Thread_Dispatch_enable( cpu_self );<br>
> +<br>
> +}<br>
> +<br>
> void _Rate_monotonic_Restart(<br>
> Rate_monotonic_Control *the_period,<br>
> Thread_Control *owner,<br>
> @@ -190,6 +229,10 @@ static rtems_status_code _Rate_monotonic_Activate(<br>
> ISR_lock_Context *lock_context<br>
> )<br>
> {<br>
> +<br>
> + /* Initialize the number of postponed job variable */<br>
</div></div>don't need this comment or the extra blank newlines around this. Just<br>
init the variable.<br>
<div><div class="h5">> + the_period->postponed_jobs = 0;<br>
> +<br>
> the_period->state = RATE_MONOTONIC_ACTIVE;<br>
> the_period->next_length = length;<br>
> _Rate_monotonic_Restart( the_period, executing, lock_context );<br>
> @@ -221,7 +264,7 @@ static rtems_status_code _Rate_monotonic_Block_while_<wbr>active(<br>
> _Thread_Wait_flags_set( executing, RATE_MONOTONIC_INTEND_TO_BLOCK );<br>
><br>
> cpu_self = _Thread_Dispatch_disable_<wbr>critical( lock_context );<br>
> - _Rate_monotonic_Release( the_period, lock_context );<br>
> + _Rate_monotonic_Release( executing, lock_context );<br>
><br>
> _Thread_Set_state( executing, STATES_WAITING_FOR_PERIOD );<br>
><br>
> @@ -241,6 +284,11 @@ static rtems_status_code _Rate_monotonic_Block_while_<wbr>active(<br>
> return RTEMS_SUCCESSFUL;<br>
> }<br>
><br>
> +/*<br>
> + * There are two possible cases: one is that the previous deadline is missed,<br>
> + * The other is that the number of postponed jobs is not 0, but the current<br>
> + * deadline is still not expired, i.e., state = RATE_MONOTONIC_ACTIVE.<br>
> + */<br>
> static rtems_status_code _Rate_monotonic_Block_while_<wbr>expired(<br>
> Rate_monotonic_Control *the_period,<br>
> rtems_interval length,<br>
> @@ -248,6 +296,12 @@ static rtems_status_code _Rate_monotonic_Block_while_<wbr>expired(<br>
> ISR_lock_Context *lock_context<br>
> )<br>
> {<br>
> + /*<br>
> + * No matter the just finished jobs in time or not,<br>
> + * they are actually missing their deadlines already.<br>
> + */<br>
> + the_period->state = RATE_MONOTONIC_EXPIRED;<br>
> +<br>
> /*<br>
> * Update statistics from the concluding period<br>
> */<br>
> @@ -255,11 +309,27 @@ static rtems_status_code _Rate_monotonic_Block_while_<wbr>expired(<br>
><br>
> the_period->state = RATE_MONOTONIC_ACTIVE;<br>
> the_period->next_length = length;<br>
> -<br>
> - _Rate_monotonic_Release_job( the_period, executing, length, lock_context );<br>
> +<br>
> + _Rate_monotonic_Release_<wbr>postponedjob( the_period, executing, length, lock_context );<br>
> return RTEMS_TIMEOUT;<br>
> }<br>
><br>
> +uint32_t rtems_rate_monotonic_<wbr>Postponed_num(<br>
</div></div>Is this function used/needed to be used?<br>
<div><div class="h5"><br>
> + rtems_id period_id<br>
> +)<br>
> +{<br>
> + Rate_monotonic_Control *the_period;<br>
> + ISR_lock_Context lock_context;<br>
> + Thread_Control *owner;<br>
> +<br>
> + the_period = _Rate_monotonic_Get( period_id, &lock_context );<br>
> + _Assert(the_period != NULL);<br>
> + uint32_t jobs = the_period->postponed_jobs;<br>
> + owner = the_period->owner;<br>
> + _Rate_monotonic_Release( owner, &lock_context );<br>
> + return jobs;<br>
> +}<br>
> +<br>
> rtems_status_code rtems_rate_monotonic_period(<br>
> rtems_id id,<br>
> rtems_interval length<br>
> @@ -282,22 +352,44 @@ rtems_status_code rtems_rate_monotonic_period(<br>
> return RTEMS_NOT_OWNER_OF_RESOURCE;<br>
> }<br>
><br>
> - _Rate_monotonic_Acquire_<wbr>critical( the_period, &lock_context );<br>
> + _Rate_monotonic_Acquire_<wbr>critical( executing, &lock_context );<br>
><br>
> state = the_period->state;<br>
><br>
> if ( length == RTEMS_PERIOD_STATUS ) {<br>
> status = _Rate_monotonic_Get_status_<wbr>for_state( state );<br>
> - _Rate_monotonic_Release( the_period, &lock_context );<br>
> + _Rate_monotonic_Release( executing, &lock_context );<br>
> } else {<br>
> switch ( state ) {<br>
> case RATE_MONOTONIC_ACTIVE:<br>
> - status = _Rate_monotonic_Block_while_<wbr>active(<br>
> - the_period,<br>
> - length,<br>
> - executing,<br>
> - &lock_context<br>
> - );<br>
> +<br>
> + if(the_period->postponed_jobs > 0){<br>
</div></div>fix whitespace<br>
<span class=""><br>
> + /*<br>
> + * If the number of postponed jobs is not 0, it means the<br>
> + * previous postponed instance is finished without exceeding<br>
> + * the current period deadline.<br>
> + *<br>
</span>align asterisks.<br>
<span class=""><br>
> + * Do nothing on the watchdog deadline assignment but release the next<br>
> + * remaining postponed job.<br>
> + */<br>
> + status = _Rate_monotonic_Block_while_<wbr>expired(<br>
> + the_period,<br>
> + length,<br>
> + executing,<br>
> + &lock_context<br>
> + );<br>
> + }else{<br>
</span>fix ws<br>
<span class=""><br>
> + /*<br>
> + * Normal case that no postponed jobs and no expiration, so wait for the period<br>
> + * and update the deadline of watchdog accordingly.<br>
> + */<br>
> + status = _Rate_monotonic_Block_while_<wbr>active(<br>
> + the_period,<br>
> + length,<br>
> + executing,<br>
> + &lock_context<br>
> + );<br>
> + }<br>
> break;<br>
> case RATE_MONOTONIC_INACTIVE:<br>
> status = _Rate_monotonic_Activate(<br>
> @@ -308,6 +400,14 @@ rtems_status_code rtems_rate_monotonic_period(<br>
> );<br>
> break;<br>
> default:<br>
> + /*<br>
> + * As now this period was already TIMEOUT, there must be at least one<br>
> + * postponed job recorded by the watchdog. The one which exceeded<br>
> + * the previous deadline"s" was just finished.<br>
</span>remove those double-quotes.<br>
<div><div class="h5"><br>
> + *<br>
> + * Maybe there is more than one job postponed due to the preemption or<br>
> + * the previous finished job.<br>
> + */<br>
> _Assert( state == RATE_MONOTONIC_EXPIRED );<br>
> status = _Rate_monotonic_Block_while_<wbr>expired(<br>
> the_period,<br>
> diff --git a/cpukit/rtems/src/<wbr>ratemontimeout.c b/cpukit/rtems/src/<wbr>ratemontimeout.c<br>
> index e514a31..be0a770 100644<br>
> --- a/cpukit/rtems/src/<wbr>ratemontimeout.c<br>
> +++ b/cpukit/rtems/src/<wbr>ratemontimeout.c<br>
> @@ -9,6 +9,8 @@<br>
> * COPYRIGHT (c) 1989-2009.<br>
> * On-Line Applications Research Corporation (OAR).<br>
> *<br>
> + * COPYRIGHT (c) 2016 Kuan-Hsun Chen, TU Dortmund University (TUDo).<br>
> + *<br>
> * The license and distribution terms for this file may be<br>
> * found in the file LICENSE in this distribution or at<br>
> * <a href="http://www.rtems.org/license/LICENSE" rel="noreferrer" target="_blank">http://www.rtems.org/license/<wbr>LICENSE</a>.<br>
> @@ -31,7 +33,7 @@ void _Rate_monotonic_Timeout( Watchdog_Control *the_watchdog )<br>
> owner = the_period->owner;<br>
><br>
> _ISR_lock_ISR_disable( &lock_context );<br>
> - _Rate_monotonic_Acquire_<wbr>critical( the_period, &lock_context );<br>
> + _Rate_monotonic_Acquire_<wbr>critical( owner, &lock_context );<br>
> wait_flags = _Thread_Wait_flags_get( owner );<br>
><br>
> if (<br>
> @@ -62,7 +64,14 @@ void _Rate_monotonic_Timeout( Watchdog_Control *the_watchdog )<br>
> _Thread_Unblock( owner );<br>
> }<br>
> } else {<br>
> + /*<br>
> + * If the watchdog is timeout, it means there is an additional postponed<br>
> + * job in the next period but it is not available to release now:<br>
> + * Either the current task is still executed, or it is preemptive by the<br>
> + * other higher priority tasks.<br>
> + */<br>
> + the_period->postponed_jobs += 1;<br>
> the_period->state = RATE_MONOTONIC_EXPIRED;<br>
> - _Rate_monotonic_Release( the_period, &lock_context );<br>
> + _Rate_monotonic_Renew_<wbr>deadline( the_period, owner, &lock_context );<br>
> }<br>
> }<br>
> --<br>
> 1.9.1<br>
><br>
</div></div>> ______________________________<wbr>_________________<br>
> devel mailing list<br>
> <a href="mailto:devel@rtems.org">devel@rtems.org</a><br>
> <a href="http://lists.rtems.org/mailman/listinfo/devel" rel="noreferrer" target="_blank">http://lists.rtems.org/<wbr>mailman/listinfo/devel</a><br>
</blockquote></div><br></div>