Merge request https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/864 was reviewed by Gedare Bloom
-- Gedare Bloom started a new discussion on cpukit/include/rtems/cpuuse.h: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/864#note_142314 > + rtems_id id; > + /** > + * CPU time this thread has consumed since the last reset. Use together > with What are the units? -- Gedare Bloom started a new discussion on cpukit/include/rtems/cpuuse.h: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/864#note_142315 > > +/** > + * @brief CPU usage snapshot. add "Task" in the brief somewhere. -- Gedare Bloom started a new discussion on cpukit/include/rtems/cpuuse.h: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/864#note_142316 > + */ > + Timestamp_Control cpu_total; > +} rtems_cpu_usage_info; add `per_task` or similar in the struct name? -- Gedare Bloom started a new discussion on cpukit/include/rtems/cpuuse.h: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/864#note_142317 > + * @brief Populate CPU usage information for a specific thread. > + * > + * @retval ::RTEMS_SUCCESSFUL The CPU usage information was stored in @a > info. why are there `::`? -- Gedare Bloom started a new discussion on cpukit/include/rtems/cpuuse.h: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/864#note_142318 > + * @retval ::RTEMS_ILLEGAL_ON_REMOTE_OBJECT The thread resided on a remote > node. > + */ > +rtems_status_code rtems_cpu_usage_for_thread( Call them tasks at the API level. -- Gedare Bloom started a new discussion on cpukit/include/rtems/cpuuse.h: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/864#note_142319 > + */ > +rtems_status_code rtems_cpu_usage_for_thread( > + rtems_id id, use descriptive var name, `task_id`. -- Gedare Bloom started a new discussion on cpukit/include/rtems/cpuuse.h: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/864#note_142320 > + * @retval ::RTEMS_UNSATISFIED No thread usage information was available. > + */ > +rtems_status_code rtems_cpu_usage_maximum( rtems_cpu_usage_info *info ); should this also return the task id? -- Gedare Bloom started a new discussion on cpukit/libmisc/cpuuse/cpuusagereport.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/864#note_142321 > + const Timestamp_Control *total, > + rtems_cpu_usage_info *info > +); I would prefer to define all the static functions here, rather than forward declare them. I think that is more typical. -- Gedare Bloom started a new discussion on cpukit/libmisc/cpuuse/cpuusagereport.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/864#note_142322 > + _Thread_Get_name( the_thread, name, sizeof( name ) ); > + seconds_used = _Timestamp_Get_seconds( &info.task_total ); > + microseconds_used = _Timestamp_Get_nanoseconds( why refactoring/changing existing code? -- Gedare Bloom started a new discussion on cpukit/include/rtems/cpuuse.h: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/864#note_142323 > + * @ref cpu_total to derive a percent. > + */ > + Timestamp_Control task_total; i think `task_CPU_time` is more descriptive. -- Gedare Bloom started a new discussion on cpukit/include/rtems/cpuuse.h: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/864#note_142324 > + * denominator for percent calculations). > + */ > + Timestamp_Control cpu_total; and then `total_CPU_time` -- Gedare Bloom started a new discussion on cpukit/libmisc/cpuuse/cpuusagereport.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/864#note_142325 > + > + info->task_total = used; > + info->cpu_total = *total; this seems unnecessary, you could just do this outside of the function -- Gedare Bloom started a new discussion on cpukit/libmisc/cpuuse/cpuusagereport.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/864#note_142326 > +} > + > +Timestamp_Control cpu_usage_populate( this should be static? -- Gedare Bloom started a new discussion on cpukit/libmisc/cpuuse/cpuusagereport.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/864#note_142327 > + * totals represent a consistent point in time (avoid a tick between > reads). > + */ > + Timestamp_Control total = cpu_usage_total_cpu_time(); declare variables at the start of the block -- Gedare Bloom started a new discussion on cpukit/libmisc/cpuuse/cpuusagereport.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/864#note_142328 > + Timestamp_Control max_used; > + bool have_max; > +} cpu_usage_max_context; struct should be declared in header if exported, or the top of the file for local use only. -- Gedare Bloom started a new discussion on cpukit/libmisc/cpuuse/cpuusagereport.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/864#note_142329 > +} > + > +rtems_status_code rtems_cpu_usage_maximum( rtems_cpu_usage_info *info ) This should go in its own separate c source file, e.g., `cpuuse/cpuusagemaximum.c` -- Gedare Bloom started a new discussion on cpukit/libmisc/cpuuse/cpuusagereport.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/864#note_142330 > + Timestamp_Control total; > + /* Will be populated by rtems_cpu_usage_maximum */ > + _Timestamp_Set_to_zero( &total ); seems unnecessary. -- View it on GitLab: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/864 You're receiving this email because of your account on gitlab.rtems.org.
_______________________________________________ bugs mailing list [email protected] http://lists.rtems.org/mailman/listinfo/bugs
