From 0221da5f56353c9b238ef51d5a24802ba67b8c56 Mon Sep 17 00:00:00 2001 From: Sebastian Huber Date: Tue, 12 Oct 2021 08:26:39 +0200 Subject: rtems: Fix rate monotonic statistics The rate monotonic period statistics were affected by rtems_cpu_usage_reset(). The logic to detect and work around a CPU usage reset was broken. The Thread_Contol::cpu_time_used is changed to contain the processor time used throughout the entire lifetime of the thread. The new member Thread_Contol::cpu_time_used_at_last_reset is added to contain the processor time used at the time of the last reset through rtems_cpu_usage_reset(). This decouples the resets of the CPU usage and the rate monotonic period statistics. Update #4528. --- cpukit/include/rtems/rtems/ratemonimpl.h | 7 +-- cpukit/include/rtems/score/schedulerimpl.h | 4 +- cpukit/include/rtems/score/thread.h | 15 ++++- cpukit/include/rtems/score/threadimpl.h | 40 ++++++++++--- cpukit/libcsupport/src/__times.c | 3 +- cpukit/libmisc/cpuuse/cpuusagereport.c | 2 +- cpukit/libmisc/cpuuse/cpuusagereset.c | 3 +- cpukit/libmisc/cpuuse/cpuusagetop.c | 2 +- cpukit/libtest/testbusy.c | 4 +- cpukit/rtems/src/ratemongetstatus.c | 28 ++++----- cpukit/rtems/src/ratemonperiod.c | 21 ++----- cpukit/score/src/threadgetcputimeused.c | 32 +++++++--- cpukit/score/src/threadgetcputimeusedafterreset.c | 71 +++++++++++++++++++++++ spec/build/cpukit/librtemscpu.yml | 1 + 14 files changed, 166 insertions(+), 67 deletions(-) create mode 100644 cpukit/score/src/threadgetcputimeusedafterreset.c diff --git a/cpukit/include/rtems/rtems/ratemonimpl.h b/cpukit/include/rtems/rtems/ratemonimpl.h index 7e42a0437c..62327c5b09 100644 --- a/cpukit/include/rtems/rtems/ratemonimpl.h +++ b/cpukit/include/rtems/rtems/ratemonimpl.h @@ -92,7 +92,7 @@ RTEMS_INLINE_ROUTINE Rate_monotonic_Control *_Rate_monotonic_Get( void _Rate_monotonic_Timeout( Watchdog_Control *watchdog ); /** - * @brief _Rate_monotonic_Get_status( + * @brief Gets the rate monotonic CPU usage status. * * This routine is invoked to compute the elapsed wall time and cpu * time for a period. @@ -102,11 +102,8 @@ void _Rate_monotonic_Timeout( Watchdog_Control *watchdog ); * since the period was initiated. * @param[out] cpu_since_last_period is set to the cpu time used by the * owning thread since the period was initiated. - * - * @retval This routine returns true if the status can be determined - * and false otherwise. */ -bool _Rate_monotonic_Get_status( +void _Rate_monotonic_Get_status( const Rate_monotonic_Control *the_period, Timestamp_Control *wall_since_last_period, Timestamp_Control *cpu_since_last_period diff --git a/cpukit/include/rtems/score/schedulerimpl.h b/cpukit/include/rtems/score/schedulerimpl.h index 98f8e337fd..50110ea6e7 100644 --- a/cpukit/include/rtems/score/schedulerimpl.h +++ b/cpukit/include/rtems/score/schedulerimpl.h @@ -1270,8 +1270,8 @@ RTEMS_INLINE_ROUTINE void _Scheduler_Update_heir( if ( heir != new_heir && ( heir->is_preemptible || force_dispatch ) ) { #if defined(RTEMS_SMP) /* - * We need this state only for _Thread_Get_CPU_time_used(). Cannot use - * _Scheduler_Thread_change_state() since THREAD_SCHEDULER_BLOCKED to + * We need this state only for _Thread_Get_CPU_time_used_locked(). Cannot + * use _Scheduler_Thread_change_state() since THREAD_SCHEDULER_BLOCKED to * THREAD_SCHEDULER_BLOCKED state changes are illegal for the real SMP * schedulers. */ diff --git a/cpukit/include/rtems/score/thread.h b/cpukit/include/rtems/score/thread.h index e23261701a..aff2f58d77 100644 --- a/cpukit/include/rtems/score/thread.h +++ b/cpukit/include/rtems/score/thread.h @@ -853,10 +853,19 @@ struct _Thread_Control { Thread_CPU_budget_algorithms budget_algorithm; /** This field is the method invoked with the budgeted time is consumed. */ Thread_CPU_budget_algorithm_callout budget_callout; - /** This field is the amount of CPU time consumed by this thread - * since it was created. + + /** + * @brief This member contains the amount of CPU time consumed by this thread + * since it was created. + */ + Timestamp_Control cpu_time_used; + + /** + * @brief This member contains the amount of CPU time consumed by this thread + * at the time of the last reset of the CPU usage by + * rtems_cpu_usage_reset(). */ - Timestamp_Control cpu_time_used; + Timestamp_Control cpu_time_used_at_last_reset; /** This field contains information about the starting state of * this thread. diff --git a/cpukit/include/rtems/score/threadimpl.h b/cpukit/include/rtems/score/threadimpl.h index ace42d8023..a983975568 100644 --- a/cpukit/include/rtems/score/threadimpl.h +++ b/cpukit/include/rtems/score/threadimpl.h @@ -1242,15 +1242,41 @@ RTEMS_INLINE_ROUTINE void _Thread_Dispatch_update_heir( #endif /** - * @brief Gets the used cpu time of the thread and stores it in the given - * Timestamp_Control. + * @brief Gets the used processor time of the thread throughout its entire + * lifetime. * - * @param the_thread The thread to get the used cpu time of. - * @param[out] cpu_time_used Stores the used cpu time of @a the_thread. + * @param[in, out] the_thread is the thread. + * + * @return Returns the used processor time of the thread throughout its entire + * lifetime. */ -void _Thread_Get_CPU_time_used( - Thread_Control *the_thread, - Timestamp_Control *cpu_time_used +Timestamp_Control _Thread_Get_CPU_time_used( Thread_Control *the_thread ); + +/** + * @brief Gets the used processor time of the thread throughout its entire + * lifetime if the caller already acquired the thread state and home + * scheduler locks. + * + * @param[in, out] the_thread is the thread. + * + * @return Returns the used processor time of the thread throughout its entire + * lifetime. + */ +Timestamp_Control _Thread_Get_CPU_time_used_locked( + Thread_Control *the_thread +); + +/** + * @brief Gets the used processor time of the thread after the last CPU usage + * reset. + * + * @param[in, out] the_thread is the thread. + * + * @return Returns the used processor time of the thread after the last CPU usage + * reset. + */ +Timestamp_Control _Thread_Get_CPU_time_used_after_last_reset( + Thread_Control *the_thread ); /** diff --git a/cpukit/libcsupport/src/__times.c b/cpukit/libcsupport/src/__times.c index 7bb7e0e9ca..a37c662654 100644 --- a/cpukit/libcsupport/src/__times.c +++ b/cpukit/libcsupport/src/__times.c @@ -65,7 +65,8 @@ clock_t _times( * of ticks since boot and the number of ticks executed by this * this thread. */ - _Thread_Get_CPU_time_used( _Thread_Get_executing(), &cpu_time_used ); + cpu_time_used = + _Thread_Get_CPU_time_used_after_last_reset( _Thread_Get_executing() ); ptms->tms_utime = ((clock_t) cpu_time_used) / tick_interval; return ptms->tms_stime; diff --git a/cpukit/libmisc/cpuuse/cpuusagereport.c b/cpukit/libmisc/cpuuse/cpuusagereport.c index 08bc5bb541..ea21e73dc1 100644 --- a/cpukit/libmisc/cpuuse/cpuusagereport.c +++ b/cpukit/libmisc/cpuuse/cpuusagereport.c @@ -52,7 +52,7 @@ static bool cpu_usage_visitor( Thread_Control *the_thread, void *arg ) ctx = arg; _Thread_Get_name( the_thread, name, sizeof( name ) ); - _Thread_Get_CPU_time_used( the_thread, &used ); + used = _Thread_Get_CPU_time_used_after_last_reset( the_thread ); _TOD_Get_uptime( &uptime ); _Timestamp_Subtract( &ctx->uptime_at_last_reset, &uptime, &ctx->total ); _Timestamp_Divide( &used, &ctx->total, &ival, &fval ); diff --git a/cpukit/libmisc/cpuuse/cpuusagereset.c b/cpukit/libmisc/cpuuse/cpuusagereset.c index 10d8ebec48..d1f0e65180 100644 --- a/cpukit/libmisc/cpuuse/cpuusagereset.c +++ b/cpukit/libmisc/cpuuse/cpuusagereset.c @@ -40,7 +40,8 @@ static bool CPU_usage_Per_thread_handler( scheduler = _Thread_Scheduler_get_home( the_thread ); _Scheduler_Acquire_critical( scheduler, &scheduler_lock_context ); - _Timestamp_Set_to_zero( &the_thread->cpu_time_used ); + the_thread->cpu_time_used_at_last_reset = + _Thread_Get_CPU_time_used_locked( the_thread ); _Scheduler_Release_critical( scheduler, &scheduler_lock_context ); _Thread_State_release( the_thread, &state_lock_context ); diff --git a/cpukit/libmisc/cpuuse/cpuusagetop.c b/cpukit/libmisc/cpuuse/cpuusagetop.c index dad11ad748..51d049257a 100644 --- a/cpukit/libmisc/cpuuse/cpuusagetop.c +++ b/cpukit/libmisc/cpuuse/cpuusagetop.c @@ -178,7 +178,7 @@ task_usage(Thread_Control* thread, void* arg) data->stack_size += thread->Start.Initial_stack.size; - _Thread_Get_CPU_time_used(thread, &usage); + usage = _Thread_Get_CPU_time_used_after_last_reset(thread); for (j = 0; j < data->last_task_count; j++) { diff --git a/cpukit/libtest/testbusy.c b/cpukit/libtest/testbusy.c index c1d44278be..51c6a71810 100644 --- a/cpukit/libtest/testbusy.c +++ b/cpukit/libtest/testbusy.c @@ -28,10 +28,10 @@ void rtems_test_busy_cpu_usage( time_t seconds, long nanoseconds ) Timestamp_Control now; executing = _Thread_Get_executing(); - _Thread_Get_CPU_time_used( executing, &start ); + start = _Thread_Get_CPU_time_used( executing ); _Timestamp_Set( &busy, seconds, nanoseconds ); do { - _Thread_Get_CPU_time_used( executing, &now ); + now = _Thread_Get_CPU_time_used( executing ); } while ( now - start < busy ); } diff --git a/cpukit/rtems/src/ratemongetstatus.c b/cpukit/rtems/src/ratemongetstatus.c index 745b52f026..5b46a7a435 100644 --- a/cpukit/rtems/src/ratemongetstatus.c +++ b/cpukit/rtems/src/ratemongetstatus.c @@ -31,7 +31,6 @@ rtems_status_code rtems_rate_monotonic_get_status( { Rate_monotonic_Control *the_period; ISR_lock_Context lock_context; - rtems_status_code status; if ( period_status == NULL ) { return RTEMS_INVALID_ADDRESS; @@ -54,35 +53,28 @@ rtems_status_code rtems_rate_monotonic_get_status( */ _Timespec_Set_to_zero( &period_status->since_last_period ); _Timespec_Set_to_zero( &period_status->executed_since_last_period ); - status = RTEMS_SUCCESSFUL; } else { Timestamp_Control wall_since_last_period; Timestamp_Control cpu_since_last_period; - bool valid_status; /* * Grab the current status. */ - valid_status = _Rate_monotonic_Get_status( + _Rate_monotonic_Get_status( the_period, &wall_since_last_period, &cpu_since_last_period ); - if ( valid_status ) { - _Timestamp_To_timespec( - &wall_since_last_period, - &period_status->since_last_period - ); - _Timestamp_To_timespec( - &cpu_since_last_period, - &period_status->executed_since_last_period - ); - status = RTEMS_SUCCESSFUL; - } else { - status = RTEMS_NOT_DEFINED; - } + _Timestamp_To_timespec( + &wall_since_last_period, + &period_status->since_last_period + ); + _Timestamp_To_timespec( + &cpu_since_last_period, + &period_status->executed_since_last_period + ); } _Rate_monotonic_Release( the_period, &lock_context ); - return status; + return RTEMS_SUCCESSFUL; } diff --git a/cpukit/rtems/src/ratemonperiod.c b/cpukit/rtems/src/ratemonperiod.c index 7f0d302583..32ac688a7f 100644 --- a/cpukit/rtems/src/ratemonperiod.c +++ b/cpukit/rtems/src/ratemonperiod.c @@ -26,7 +26,7 @@ #include #include -bool _Rate_monotonic_Get_status( +void _Rate_monotonic_Get_status( const Rate_monotonic_Control *the_period, Timestamp_Control *wall_since_last_period, Timestamp_Control *cpu_since_last_period @@ -47,14 +47,7 @@ bool _Rate_monotonic_Get_status( /* * Determine cpu usage since period initiated. */ - _Thread_Get_CPU_time_used( owning_thread, &used ); - - /* - * The cpu usage info was reset while executing. Can't - * determine a status. - */ - if ( _Timestamp_Less_than( &used, &the_period->cpu_usage_period_initiated ) ) - return false; + used = _Thread_Get_CPU_time_used( owning_thread ); /* used = current cpu usage - cpu usage at start of period */ _Timestamp_Subtract( @@ -62,8 +55,6 @@ bool _Rate_monotonic_Get_status( &used, cpu_since_last_period ); - - return true; } static void _Rate_monotonic_Release_postponed_job( @@ -130,7 +121,7 @@ void _Rate_monotonic_Restart( * Set the starting point and the CPU time used for the statistics. */ _TOD_Get_uptime( &the_period->time_period_initiated ); - _Thread_Get_CPU_time_used( owner, &the_period->cpu_usage_period_initiated ); + the_period->cpu_usage_period_initiated = _Thread_Get_CPU_time_used( owner ); _Rate_monotonic_Release_job( the_period, @@ -147,7 +138,6 @@ static void _Rate_monotonic_Update_statistics( Timestamp_Control executed; Timestamp_Control since_last_period; Rate_monotonic_Statistics *stats; - bool valid_status; /* * Assume we are only called in states where it is appropriate @@ -167,10 +157,7 @@ static void _Rate_monotonic_Update_statistics( /* * Grab status for time statistics. */ - valid_status = - _Rate_monotonic_Get_status( the_period, &since_last_period, &executed ); - if (!valid_status) - return; + _Rate_monotonic_Get_status( the_period, &since_last_period, &executed ); /* * Update CPU time diff --git a/cpukit/score/src/threadgetcputimeused.c b/cpukit/score/src/threadgetcputimeused.c index 7406da0bf3..f23f606411 100644 --- a/cpukit/score/src/threadgetcputimeused.c +++ b/cpukit/score/src/threadgetcputimeused.c @@ -4,7 +4,7 @@ * @ingroup RTEMSScoreThread * * @brief This source file contains the implementation of - * _Thread_Get_CPU_time_used(). + * _Thread_Get_CPU_time_used() and _Thread_Get_CPU_time_used_locked(). */ /* @@ -37,25 +37,39 @@ static bool _Thread_Is_scheduled( const Thread_Control *the_thread ) #endif } -void _Thread_Get_CPU_time_used( - Thread_Control *the_thread, - Timestamp_Control *cpu_time_used +Timestamp_Control _Thread_Get_CPU_time_used_locked( + Thread_Control *the_thread ) +{ + _Assert( _Thread_State_is_owner( the_thread ) ); + _Assert( + _ISR_lock_Is_owner( + &_Scheduler_Get_context( _Thread_Scheduler_get_home( the_thread ) )->Lock + ) + ); + + if ( _Thread_Is_scheduled( the_thread ) ) { + _Thread_Update_CPU_time_used( the_thread, _Thread_Get_CPU( the_thread ) ); + } + + return the_thread->cpu_time_used; +} + +Timestamp_Control _Thread_Get_CPU_time_used( Thread_Control *the_thread ) { const Scheduler_Control *scheduler; ISR_lock_Context state_lock_context; ISR_lock_Context scheduler_lock_context; + Timestamp_Control cpu_time_used; _Thread_State_acquire( the_thread, &state_lock_context ); scheduler = _Thread_Scheduler_get_home( the_thread ); _Scheduler_Acquire_critical( scheduler, &scheduler_lock_context ); - if ( _Thread_Is_scheduled( the_thread ) ) { - _Thread_Update_CPU_time_used( the_thread, _Thread_Get_CPU( the_thread ) ); - } - - *cpu_time_used = the_thread->cpu_time_used; + cpu_time_used = _Thread_Get_CPU_time_used_locked( the_thread ); _Scheduler_Release_critical( scheduler, &scheduler_lock_context ); _Thread_State_release( the_thread, &state_lock_context ); + + return cpu_time_used; } diff --git a/cpukit/score/src/threadgetcputimeusedafterreset.c b/cpukit/score/src/threadgetcputimeusedafterreset.c new file mode 100644 index 0000000000..1ab4aa16fb --- /dev/null +++ b/cpukit/score/src/threadgetcputimeusedafterreset.c @@ -0,0 +1,71 @@ +/* SPDX-License-Identifier: BSD-2-Clause */ + +/** + * @file + * + * @ingroup RTEMSScoreThread + * + * @brief This source file contains the implementation of + * _Thread_Get_CPU_time_used_after_last_reset(). + */ + +/* + * Copyright (C) 2021 embedded brains GmbH (http://www.embedded-brains.de) + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE + * POSSIBILITY OF SUCH DAMAGE. + */ + +#ifdef HAVE_CONFIG_H +#include "config.h" +#endif + +#include +#include + +Timestamp_Control _Thread_Get_CPU_time_used_after_last_reset( + Thread_Control *the_thread +) +{ + const Scheduler_Control *scheduler; + ISR_lock_Context state_lock_context; + ISR_lock_Context scheduler_lock_context; + Timestamp_Control cpu_time_used; + Timestamp_Control cpu_time_used_at_last_reset; + + _Thread_State_acquire( the_thread, &state_lock_context ); + scheduler = _Thread_Scheduler_get_home( the_thread ); + _Scheduler_Acquire_critical( scheduler, &scheduler_lock_context ); + + cpu_time_used = _Thread_Get_CPU_time_used_locked( the_thread ); + cpu_time_used_at_last_reset = the_thread->cpu_time_used_at_last_reset; + + _Scheduler_Release_critical( scheduler, &scheduler_lock_context ); + _Thread_State_release( the_thread, &state_lock_context ); + + _Timestamp_Subtract( + &cpu_time_used_at_last_reset, + &cpu_time_used, + &cpu_time_used + ); + + return cpu_time_used; +} diff --git a/spec/build/cpukit/librtemscpu.yml b/spec/build/cpukit/librtemscpu.yml index 070bff0d57..2017ab058c 100644 --- a/spec/build/cpukit/librtemscpu.yml +++ b/spec/build/cpukit/librtemscpu.yml @@ -1543,6 +1543,7 @@ source: - cpukit/score/src/threadentryadaptorpointer.c - cpukit/score/src/threadget.c - cpukit/score/src/threadgetcputimeused.c +- cpukit/score/src/threadgetcputimeusedafterreset.c - cpukit/score/src/threadhandler.c - cpukit/score/src/threadidledefault.c - cpukit/score/src/threadinitialize.c -- cgit v1.2.3