Skip to content

Conversation

rruuaanng
Copy link
Contributor

@rruuaanng rruuaanng commented Oct 25, 2024

Add 'k_thread_runtime_stats_is_enabled' to 'usage.c', which's used to check whether runtime statistics collection is enabled for a thread.

It can determine whether the thread has enabled runtime statistics collection when the program is running. It allows users to add conditional statements based on this function when using it.

for example:

void f()
{
    // Assume there is a thread id
    if (k_thread_runtime_stats_is_enabled(id)) {
        /* do something */
    }
    ...
}

And it can provide a complete support when similar operations are needed. for example enable, disable, is_enabled.
For example, its use in the implementation of CLOCK_THREAD_CPUTIME_ID actually lacks k_thread_runtime_stats_is_enabled, which makes the entire function impossible to implement safely. It is necessary.

..

@rruuaanng rruuaanng force-pushed the gh80363 branch 2 times, most recently from fc52903 to 075a033 Compare October 25, 2024 15:12
@rruuaanng rruuaanng changed the title kernel: Add k_thread_runtime_stats_enabled function kernel: Add k_thread_runtime_stats_is_enabled function Oct 25, 2024
@rruuaanng rruuaanng marked this pull request as ready for review October 25, 2024 15:28
@zephyrbot zephyrbot added the area: POSIX POSIX API Library label Oct 26, 2024
@zephyrbot zephyrbot requested a review from ycsin October 26, 2024 10:40
@rruuaanng rruuaanng force-pushed the gh80363 branch 2 times, most recently from 70b4aa0 to 04e6d25 Compare October 26, 2024 12:02
Copy link
Member

@cfriedt cfriedt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for all of the POSIX clock work.

Just a question / suggestion regarding runtime function call overhead.

Copy link
Member

@cfriedt cfriedt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some suggestions around CONFIG_POSIX_THREAD_CPUTIME

@rruuaanng rruuaanng force-pushed the gh80363 branch 2 times, most recently from 5302a73 to e9323cc Compare October 28, 2024 04:18
@rruuaanng rruuaanng requested a review from cfriedt October 28, 2024 04:26
@rruuaanng
Copy link
Contributor Author

I've made change, Please riewer again!

peter-mitsis
peter-mitsis previously approved these changes Oct 28, 2024
Copy link
Contributor

@peter-mitsis peter-mitsis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kernel side seems reasonable to me.

ycsin
ycsin previously approved these changes Oct 29, 2024
peter-mitsis
peter-mitsis previously approved these changes Apr 16, 2025
ycsin
ycsin previously approved these changes Apr 17, 2025
cfriedt
cfriedt previously approved these changes Apr 17, 2025
@ycsin
Copy link
Member

ycsin commented Apr 22, 2025

@rruuaanng this needs another rebase

@peter-mitsis
Copy link
Contributor

@rruuaanng - Looks like this has conflicts that need to get resolved.

andyross
andyross previously approved these changes Jul 1, 2025
Copy link
Contributor

@andyross andyross left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tiny nitpick. I'll let others weigh in on the posix changes, seems like that could be a separate PR? +1 for the meat of the PR.

* @param thread ID of thread
* @return true if usage statistics are enabled for the given thread, otherwise false
*/
bool k_thread_runtime_stats_is_enabled(k_tid_t thread);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this just depends on the thread struct, that header appears in the include paths already. You can make this inline without worry and save a few code bytes for users.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should stick to the existing API style? It looks like there are no static inline functions in kernel.h.

@rruuaanng rruuaanng marked this pull request as draft July 3, 2025 13:24
Add 'k_thread_runtime_stats_is_enabled' function, whichs
used to check whether runtime statistics collection is
enabled for a thread.

Signed-off-by: James Roy <[email protected]>
Add test for 'k_thread_runtime_stats_is_enabled(tid)'.

Signed-off-by: James Roy <[email protected]>
Add the `k_thread_runtime_stats_is_enabled` function
to check if runtime statistics are enabled.

Signed-off-by: James Roy <[email protected]>
@rruuaanng
Copy link
Contributor Author

I will open a new PR to finish the POSIX part, just to avoid any confusion.

@rruuaanng rruuaanng marked this pull request as ready for review July 15, 2025 08:14
Copy link

@rruuaanng rruuaanng modified the milestones: v4.2.0, v4.3.0 Jul 18, 2025
@rruuaanng rruuaanng requested review from andyross and removed request for andyross September 21, 2025 03:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: Kernel area: POSIX POSIX API Library Release Notes To be mentioned in the release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants