I missed reviewing this patch earlier, but there are some style problems with it, please fix them on the head.
On Tue, Oct 28, 2014 at 11:17 AM, Jennifer Averett <jenni...@rtems.org> wrote: > Module: rtems > Branch: master > Commit: 6031da438d219c6ec5d9d48f1df2aef91710cce3 > Changeset: > http://git.rtems.org/rtems/commit/?id=6031da438d219c6ec5d9d48f1df2aef91710cce3 > > Author: Jennifer Averett <jennifer.aver...@oarcorp.com> > Date: Mon Sep 29 10:20:27 2014 -0500 > > libmisc: Add top to cpuusage. > > --- > > cpukit/libmisc/Makefile.am | 2 +- > cpukit/libmisc/cpuuse/cpuusagetop.c | 337 > +++++++++++++++++++++++++++++++++++ > cpukit/libmisc/cpuuse/cpuuse.h | 19 ++ > 3 files changed, 357 insertions(+), 1 deletions(-) > > diff --git a/cpukit/libmisc/Makefile.am b/cpukit/libmisc/Makefile.am > index 2f41ffa..d26c484 100644 > --- a/cpukit/libmisc/Makefile.am > +++ b/cpukit/libmisc/Makefile.am > @@ -27,7 +27,7 @@ EXTRA_DIST += cpuuse/README > > noinst_LIBRARIES += libcpuuse.a > libcpuuse_a_SOURCES = cpuuse/cpuusagereport.c cpuuse/cpuusagereset.c \ > - cpuuse/cpuuse.h cpuuse/cpuusagedata.c > + cpuuse/cpuuse.h cpuuse/cpuusagedata.c cpuuse/cpuusagetop.c > > ## devnull > noinst_LIBRARIES += libdevnull.a > diff --git a/cpukit/libmisc/cpuuse/cpuusagetop.c > b/cpukit/libmisc/cpuuse/cpuusagetop.c > new file mode 100644 > index 0000000..7e7348a > --- /dev/null > +++ b/cpukit/libmisc/cpuuse/cpuusagetop.c > @@ -0,0 +1,337 @@ > +/** > + * @file > + * > + * @brief CPU Usage Top > + * @ingroup libmisc_cpuuse CPU Usage > + */ > + > +/* > + * COPYRIGHT (c) 2014. > + * On-Line Applications Research Corporation (OAR). > + * > + * The license and distribution terms for this file may be > + * found in the file LICENSE in this distribution or at > + * http://www.rtems.org/license/LICENSE. > + */ > + > +#ifdef HAVE_CONFIG_H > +#include "config.h" > +#endif > + > +#include <string.h> > +#include <stdlib.h> > +#include <stdio.h> > +#include <ctype.h> > +#include <inttypes.h> > + > +#include <rtems/cpuuse.h> > +#include <rtems/score/objectimpl.h> > +#include <rtems/score/threadimpl.h> > +#include <rtems/score/todimpl.h> > +#include <rtems/score/watchdogimpl.h> > + > + > +/* > + * Common variable to sync the load monitor task. > + */ > +static volatile int rtems_cpuusage_top_thread_active; > + > + remove multiple blank lines > +typedef struct { > + void *context; > + rtems_printk_plugin_t print; > +}rtems_cpu_usage_plugin_t; > + > +#define RTEMS_CPUUSAGE_TOP_MAX_LOAD_TASKS (20) > + > +/* > + * rtems_cpuusage_top_thread > + * > + * DESCRIPTION: > + * > + * This function displays the load of the tasks on an ANSI terminal. > + * Get rid of "DESCRIPTION", prefer to use minimalist comment style on static functions. > + */ > + > +static void > +rtems_cpuusage_top_thread (rtems_task_argument arg) > +{ > + uint32_t api_index; > + Thread_Control* the_thread; > + int i; > + int j; > + int k; > + Objects_Information* information; > + char name[13]; > + int task_count = 0; > + uint32_t seconds, nanoseconds; > + rtems_cpu_usage_plugin_t* plugin = (rtems_cpu_usage_plugin_t*)arg; > + Thread_Control* load_tasks[RTEMS_CPUUSAGE_TOP_MAX_LOAD_TASKS + > 1]; > + unsigned long long load[RTEMS_CPUUSAGE_TOP_MAX_LOAD_TASKS + 1]; > + > + while (true) > + { Opening brace should go on whille() line. > + #ifndef __RTEMS_USE_TICKS_FOR_STATISTICS__ > + Timestamp_Control uptime, total, ran, uptime_at_last_reset; > + #else > + uint32_t total_units = 0; > + #endif > + > + rtems_cpuusage_top_thread_active = 1; > + > + memset (load_tasks, 0, sizeof (load_tasks)); > + memset (load, 0, sizeof (load)); > + > + /* > + * Iterate over the tasks and sort the highest load tasks > + * into our local arrays. We only handle a limited number of > + * tasks. > + */ > + for ( api_index = 1 ; api_index <= OBJECTS_APIS_LAST ; api_index++ ) { > + #if !defined(RTEMS_POSIX_API) || defined(RTEMS_DEBUG) > + if ( !_Objects_Information_table[ api_index ] ) > + continue; > + #endif > + > + information = _Objects_Information_table[ api_index ][ 1 ]; > + if ( information ) { > + for ( k=1 ; k <= information->maximum ; k++ ) { I prefer that nested iterators should be used in increasing order, e.g. i first. > + the_thread = (Thread_Control *)information->local_table[ k ]; > + if ( the_thread ) { > + No blank space after the opening brace. > + Thread_CPU_usage_t l = the_thread->cpu_time_used; Avoid using l and I as variable names.They're hard to differentiate from each other and from the digit 1. This should be a more useful variable name anyway. A one-letter variable should really only be used for iterators. We ought to have a rule about this if we don't. > + > + /* > + * When not using nanosecond CPU usage resolution, we have to > count > + * the number of "ticks" we gave credit for to give the user a > rough > + * guideline as to what each number means proportionally. > + */ > + #ifdef __RTEMS_USE_TICKS_FOR_STATISTICS__ > + total_units += l; > + #endif > + > + /* Count the number of tasks and sort this load value */ > + task_count++; > + for (i = 0; i < RTEMS_CPUUSAGE_TOP_MAX_LOAD_TASKS; i++) { > + if (load_tasks[i]) { > + if ((l == 0) || (l < load[i])) > + continue; > + for (j = (RTEMS_CPUUSAGE_TOP_MAX_LOAD_TASKS - 1); j >= i; > j--){ > + load_tasks[j + 1] = load_tasks[j]; > + load[j + 1] = load[j]; > + } > + } > + load_tasks[i] = the_thread; > + load[i] = l; > + break; > + } > + } > + } > + } > + } > + > + #ifndef __RTEMS_USE_TICKS_FOR_STATISTICS__ > + _Timestamp_Set_to_zero( &total ); > + uptime_at_last_reset = CPU_usage_Uptime_at_last_reset; > + #endif > + > + _TOD_Get_uptime( &uptime ); > + seconds = _Timestamp_Get_seconds( &uptime ); > + nanoseconds = _Timestamp_Get_nanoseconds( &uptime ) / > + TOD_NANOSECONDS_PER_MICROSECOND; > + (*plugin->print)(plugin->context, "\x1b[H\x1b[J Press ENTER to > exit.\n\n"); > + (*plugin->print)(plugin->context, "uptime: "); > + (*plugin->print)(plugin->context, > + "%7" PRIu32 ".%06" PRIu32 "\n", seconds, nanoseconds > + ); > + > + (*plugin->print)( > + plugin->context, > + > "-------------------------------------------------------------------------------\n" > + " CPU USAGE BY THREAD\n" > + > "------------+---------------------+---------------+---------------+------------\n" > + #ifndef __RTEMS_USE_TICKS_FOR_STATISTICS__ > + " ID | NAME | RPRI | CPRI | SECONDS | > PERCENT\n" > + #else > + " ID | NAME | RPRI | CPRI | TICKS | > PERCENT\n" > + #endif > + > "------------+---------------------+---------------+---------------+------------\n" > + ); > + > + for (i = 0; i < RTEMS_CPUUSAGE_TOP_MAX_LOAD_TASKS; i++) > + { > + Fix style here. > + if (!load_tasks[i]) > + break; > + > + /* > + * If this is the currently executing thread, account for time > + * since the last context switch. > + */ > + the_thread = load_tasks[i]; > + > + rtems_object_get_name( the_thread->Object.id, sizeof(name), name ); > + (*plugin->print)( > + plugin->context, > + " 0x%08" PRIx32 " | %-19s | %3" PRId32 " | %3" PRId32 " |", > + the_thread->Object.id, > + name, > + the_thread->real_priority, > + the_thread->current_priority > + ); > + > + #ifndef __RTEMS_USE_TICKS_FOR_STATISTICS__ > + { I don't know how to feel about artifically adding scoped blocks. Anyone else weigh in? > + Timestamp_Control last; > + uint32_t ival, fval; > + > + /* > + * If this is the currently executing thread, account for time > + * since the last context switch. > + */ > + ran = load[i]; > + if ( _Thread_Get_time_of_last_context_switch( the_thread, &last ) ) { > + Timestamp_Control used; > + _TOD_Get_uptime( &uptime ); > + _Timestamp_Subtract( &last, &uptime, &used ); > + _Timestamp_Add_to( &ran, &used ); > + } else { > + _TOD_Get_uptime( &uptime ); > + } > + _Timestamp_Subtract( &uptime_at_last_reset, &uptime, &total ); > + _Timestamp_Divide( &ran, &total, &ival, &fval ); > + > + /* > + * Print the information > + */ > + > + seconds = _Timestamp_Get_seconds( &ran ); > + nanoseconds = _Timestamp_Get_nanoseconds( &ran ) / > + TOD_NANOSECONDS_PER_MICROSECOND; > + (*plugin->print)( plugin->context, > + "%7" PRIu32 ".%06" PRIu32 " |%4" PRIu32 ".%03" PRIu32 "\n", > + seconds, nanoseconds, > + ival, fval > + ); > + } > + #else > + if (total_units) { > + uint64_t ival_64; > + > + ival_64 = load[i]; > + ival_64 *= 100000; > + ival = ival_64 / total_units; > + } else { > + ival = 0; > + } > + > + fval = ival % 1000; > + ival /= 1000; > + (*plugin->print)( plugin->context, > + "%14" PRIu32 " |%4" PRIu32 ".%03" PRIu32 "\n", > + load[i], > + ival, > + fval > + ); > + #endif > + } > + > + if (task_count < RTEMS_CPUUSAGE_TOP_MAX_LOAD_TASKS) > + { > + j = RTEMS_CPUUSAGE_TOP_MAX_LOAD_TASKS - task_count; > + while (j > 0) > + { > + (*plugin->print)( plugin->context, "\x1b[K\n"); > + j--; > + } > + } > + > + rtems_cpuusage_top_thread_active = 0; > + > + rtems_task_wake_after (RTEMS_MICROSECONDS_TO_TICKS (5000000)); > + } > +} > + > +void rtems_cpu_usage_top_with_plugin( > + void *context, > + rtems_printk_plugin_t print > +) > +{ > + rtems_status_code sc; > + rtems_task_priority priority; > + rtems_name name; > + rtems_id id; > + rtems_cpu_usage_plugin_t plugin; > + > + if ( !print ) > + return; > + > + plugin.context = context; > + plugin.print = print; > + > + sc = rtems_task_set_priority (RTEMS_SELF, RTEMS_CURRENT_PRIORITY, > &priority); > + > + if (sc != RTEMS_SUCCESSFUL) > + { > + (*print)( > + context, > + "error: cannot obtain the current priority: %s\n", > + rtems_status_text (sc) > + ); > + return; > + } > + > + name = rtems_build_name('C', 'P', 'l', 't'); > + > + sc = rtems_task_create (name, priority, 4 * 1024, > + RTEMS_NO_FLOATING_POINT | RTEMS_LOCAL, > + RTEMS_PREEMPT | RTEMS_TIMESLICE | RTEMS_NO_ASR, > + &id); > + > + if (sc != RTEMS_SUCCESSFUL) > + { > + (*print)( > + context, > + "error: cannot create helper thread: %s\n", > + rtems_status_text (sc) > + ); > + return; > + } > + > + sc = rtems_task_start ( > + id, rtems_cpuusage_top_thread, (rtems_task_argument)&plugin > + ); > + if (sc != RTEMS_SUCCESSFUL) > + { > + (*print)( > + context, > + "error: cannot start helper thread: %s\n", > + rtems_status_text (sc) > + ); > + rtems_task_delete (id); > + return; > + } > + > + for (;;) > + { > + int c = getchar (); > + > + if ((c == '\r') || (c == '\n')) > + { > + int loops = 20; > + > + while (loops && rtems_cpuusage_top_thread_active) > + rtems_task_wake_after (RTEMS_MICROSECONDS_TO_TICKS (100000)); > + > + rtems_task_delete (id); > + > + (*print)(context, "load monitoring stopped.\n"); > + return; > + } > + } > +} > + > +void rtems_cpu_usage_top( void ) > +{ > + rtems_cpu_usage_top_with_plugin( NULL, printk_plugin ); > +} > diff --git a/cpukit/libmisc/cpuuse/cpuuse.h b/cpukit/libmisc/cpuuse/cpuuse.h > index 1aee275..662d905 100644 > --- a/cpukit/libmisc/cpuuse/cpuuse.h > +++ b/cpukit/libmisc/cpuuse/cpuuse.h > @@ -63,6 +63,25 @@ void rtems_cpu_usage_report_with_plugin( > void rtems_cpu_usage_report( void ); > > /** > + * @brief CPU usage Top plugin > + * > + * Report CPU Usage in top format to > + * to a print plugin. > + */ > +void rtems_cpu_usage_top_with_plugin( > + void *context, > + rtems_printk_plugin_t print > +); > + > +/** > + * @brief CPU usage top. > + * > + * CPU Usage top > + */ > + > +void rtems_cpu_usage_top( void ); > + > +/** > * @brief Reset CPU usage. > * > * CPU Usage Reporter > > _______________________________________________ > vc mailing list > v...@rtems.org > http://lists.rtems.org/mailman/listinfo/vc _______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel