RTEMS | CPU Usage api (!864)

[Gedare Bloom (@gedare)]

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.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/bugs/attachments/20260210/4b07e692/attachment-0001.htm>