Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

k_call_stacks_analyze needs to be reimplemented and optimized #6902

Closed
1 task
nashif opened this issue Apr 3, 2018 · 8 comments
Closed
1 task

k_call_stacks_analyze needs to be reimplemented and optimized #6902

nashif opened this issue Apr 3, 2018 · 8 comments
Assignees
Labels
area: Profiling Enhancement Changes/Updates/Additions to existing features

Comments

@nashif
Copy link
Member

nashif commented Apr 3, 2018

k_call_stacks_analyze being a 1st class kernel API currently prints stack data for system threads by directly invoking printk. It is implemented as part of the init.c source code for weird reasons and to access to stack data of threads defined in this file.

Ideally we should rely on CONFIG_THREAD_STACK_INFO instead and provide stack data/information for all threads configured in the system, for example we could provide macros to iterated over stack data and have the application decide how to use show this data and print it, for all defined threads, not only the system threads, i.e.:

FOREACH_THREAD_STACK_INFO(thread_list) {
   printk(....)
} 

now sure what k_call_stacks_analyze will do in this case and if we need to keep it.

  • deprecate k_call_stacks_analyze() and use new iteration function
@nashif nashif added Enhancement Changes/Updates/Additions to existing features area: Profiling labels Apr 3, 2018
@jukkar
Copy link
Member

jukkar commented Apr 4, 2018

Ideally we should rely on CONFIG_THREAD_STACK_INFO instead and provide stack data/information for all threads configured in the system, for example we could provide macros to iterated over stack data and have the application decide how to use show this data and print it, for all defined threads, not only the system threads

This is actually how the "net stacks" net-shell command is doing things. It collects stack information for network related threads and prints the values itself. It would be great if we could have this kind of service provided by the kernel directly so I could actually remove this support from network subsystem.

@ramakrishnapallala
Copy link

@nashif do we need to dump all threads stack info in an iterative manner?

@nashif
Copy link
Member Author

nashif commented Apr 21, 2018

@nashif do we need to dump all threads stack info in an iterative manner?

yes, the 'dumping' should not be part of the API though...

@ramakrishnapallala
Copy link

ramakrishnapallala commented Apr 25, 2018

@nashif
I am planning to modify the API as below:

typedef void (*stack_analyze_fn)(k_tid_t thread, const char *stack, size_t size);
void k_call_stacks_analyze(k_tid_t thread, stack_analyze_fn analyze);

One challenge I see in iterating over all the threads is, they have not tied any list. For static threads, we are doing build time magic to get the start and end of the thread (tid's) but we don't have anything for tracking the dynamic threads at the moment. So, we would have check/iterate over ready_q[n] list and wait_q list to get the thread tid's.

Let me know your opinion.

@jukkar
Copy link
Member

jukkar commented Apr 25, 2018

Why do you have thread id as a parameter to k_call_stacks_analyze(), if that is omitted then the caller could just traverse through the stacks configured in the system and this would work with all the threads.

IMHO a better API would be something like this:

/**
 * @brief Go through all the stacks and call callback for each of them.
 *
 * @param cb User-supplied callback function to call
 * @param user_data User specified data
 */
void k_stacks_foreach(stack_analyze_fn cb, void *user_data);

@ramakrishnapallala
Copy link

ramakrishnapallala commented Apr 25, 2018

ok, what would user_data is needed for? do we need to pass that to the stack_analyze_fn?

@nashif
Copy link
Member Author

nashif commented Apr 25, 2018

One challenge I see in iterating over all the threads is, they have not tied any list.

the THREAD_MONITOR would give you this.

ok, what would user_data is needed for? do we need to pass that to the stack_analyze_fn?

the callback function will be more flexible this way, this would be an optional argument in this case.

Maybe:
k_stacks_foreach -> k_thread_stack_foreach

and then also create a generic API for iterating over threads in general, i.e. k_thread_foreach.

@ramakrishnapallala
Copy link

We can close this issue #7225 merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Profiling Enhancement Changes/Updates/Additions to existing features
Projects
None yet
Development

No branches or pull requests

3 participants