Merge request https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/843 was reviewed by Gedare Bloom
-- Gedare Bloom started a new discussion on cpukit/libmisc/cpuuse/cpuuse_context_pc_aarch64.c: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/843#note_142013 > +#include <rtems/cpuuseimpl.h> > + > +uintptr_t rtems_cpuuse_context_to_pc( const Context_Control *context ) I would suggest making this part of each CPU port definitions in `score/cpu/${arch}/include/rtems/score/cpu.h` and provide this as an score interface to the `Context_Control` or the `CPU_Exception_frame`. If it needs to be exported to users, then you could add an `rtems_...` interface to it somewhere suitable, but I think this could be kept internal. -- Gedare Bloom started a new discussion on cpukit/include/rtems/cpuuse.h: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/843#note_142014 > + uint32_t fval; > + > + char task_name[38]; why `38`? -- Gedare Bloom started a new discussion on cpukit/include/rtems/cpuuse.h: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/843#note_142015 > + * @brief Fractional part of the percentage. > + */ > + uint32_t fval; this could be an `rtems_task_cpu_usage_info`? This appears to be a kind of inheritance. -- Gedare Bloom started a new discussion on cpukit/include/rtems/cpuuse.h: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/843#note_142016 > +rtems_status_code rtems_cpu_max_usage_extended( > + rtems_max_task_cpu_usage_info *usage, > + struct _Thread_Control **thread should not be leaking internal data types. use an opaque type (e.g., `rtems_id`) -- Gedare Bloom started a new discussion on cpukit/include/rtems/cpuuse_backtrace.h: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/843#note_142017 > + * @retval RTEMS_INTERNAL_ERROR Stack walking encountered an error. > + */ > +rtems_status_code rtems_cpu_usage_backtrace( This seems to be not related to `cpuuse`, I think the API should be placed somewhere else. -- Gedare Bloom started a new discussion on cpukit/include/rtems/cpuuse_backtrace.h: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/843#note_142018 > + */ > +rtems_status_code rtems_cpu_usage_backtrace( > + Thread_Control *thread, use opaque types -- Gedare Bloom started a new discussion on cpukit/include/rtems/cpuuse_backtrace.h: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/843#note_142019 > + */ > + uintptr_t stack_pointer; > +} rtems_backtrace_frame_t; avoid defining typedefs with `_t`. This is reserved by POSIX. -- Gedare Bloom started a new discussion on cpukit/include/rtems/cpuuse_backtrace.h: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/843#note_142020 > + const rtems_printer *printer, > + const rtems_backtrace_t *backtrace, > + bool show_details same here, this is a configuration-time decision. -- Gedare Bloom started a new discussion on cpukit/include/rtems/cpuuse_backtrace.h: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/843#note_142021 > + * @retval false Backtrace is not supported on this architecture. > + */ > +bool rtems_backtrace_supported(void); same here, this is a configuration-time decision. -- Gedare Bloom started a new discussion on cpukit/include/rtems/cpuuse_backtrace.h: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/843#note_142022 > + Thread_Control *thread, > + rtems_backtrace_t *backtrace, > + int max_frames I'm guessing this is a compile-time decision. I would consider making it part of the configuration. -- Gedare Bloom started a new discussion on cpukit/include/rtems/cpuuse_backtrace.h: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/843#note_142023 > + * @brief Status of backtrace generation. > + */ > + rtems_status_code status; we usually return the status from functions, so it can be checked directly. -- Gedare Bloom started a new discussion on cpukit/include/rtems/cpuuse_backtrace.h: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/843#note_142024 > + > + /** > + * @brief RTL object containing the symbol. RTL? -- Gedare Bloom started a new discussion on cpukit/include/rtems/cpuuseimpl.h: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/843#note_142025 > + * held. Callers should copy any required string data under the lock. > + */ > +bool rtems_cpuuse_find_nearest_symbol( If these faciltiies exist in the RTL, maybe it should be relied on for the backtrace functionality also? -- View it on GitLab: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/843 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
