Bluetooth: HCI_UART: Update main stack and refactor conf files #23104

joerchan · 2020-02-26T12:44:47Z

For Nordic alternative HCI driver the main stack usage caused stack overflow issues.
Refactor conf files to recommended CONF_FILE usage as the current CONF_FILE mechanism does not work when providing extra CONF_FILE or CONF_OVERLAY. Configurations will be lost in this case.

joerchan · 2020-02-26T12:46:00Z

CC: @rugeGerritsen

zephyrbot · 2020-02-26T12:47:22Z

All checks passed.

Tip: The bot edits this comment instead of posting a new one, so you can check the comment's history to see earlier messages.

jhedberg · 2020-02-26T15:05:13Z

samples/bluetooth/hci_uart/src/main.c

This whole branch seems unnecessary to me. The condition is identical to the branch above it and you're not doing any long-duration work in that branch.

yeah, removed it.

cvinayak · 2020-03-01T01:41:01Z

We should address this at kernel level and not have these in individual samples. For some context, see: #15856 and #15848

@ceolin FYI.

andrewboie · 2020-03-02T03:35:13Z

samples/bluetooth/hci_uart/src/main.c

+		static u32_t prio_ts;
+
+		if (k_uptime_get_32() - prio_ts > K_SECONDS(5)) {
+			STACK_ANALYZE("main thread stack",


STACK_ANALYZE is deprecated, please do not use it.
You want a k_thread_foreach() loop and call k_thread_stack_space_get() on each thread.

~~Will replace STACK_ANALYZE usage. But need a way to do it for main stack. k_thread_foreach will not give me main stack.~~

Replaced with suggestion.

andrewboie · 2020-03-02T03:36:23Z

include/debug/stack.h

 #include <logging/log.h>
 #include <stdbool.h>

+extern K_THREAD_STACK_DEFINE(z_main_stack, CONFIG_MAIN_STACK_SIZE);


do not put these here, they are private to the kernel

~~I need an alternative so that I can debug the main stack.~~

andrewboie · 2020-03-02T03:38:15Z

you should really separate out the patches which are strictly bug fixes into a separate PR to get them in faster.

andrewboie

needs changes

joerchan · 2020-03-02T13:19:16Z

@andrewboie Your suggestion does not give me the stack usage of the threads mentioned, with the k_thread_foreach(_unlocked) I only get "BT RX" and "BT RX pri" threads. Not the main stack or any of the others.
EDIT: This appears to be an issue with the logging system. Only the two first threads is printed. But all threads are iterated.

Another thing I noticed is that k_thread_foreach does not iterate over the static threads either, but adding code for that did not give me main stack usage.
EDIT: Created #23196

~~The application sets the stack size of these threads and performs a lot of the code that runs it them. But don't have a way to analyze the stack usage for these threads.~~

~~What about adding struct k_thread_t *k_thread_get(const char *name) function or something similar, maybe one per thread, that would allow us to analyze these threads?~~

joerchan · 2020-03-02T14:13:49Z

@andrewboie Sorry for the noise, it appears to be a logging issue. I only get some of the LOG_INF statements. Using printk I see it all.
EDIT:
Silly mistake, too low LOG_LEVEL for LOG_INF to appear in main.c.

carlescufi · 2020-03-02T16:42:32Z

samples/bluetooth/hci_uart/debug.conf

Could you please update the sample's README.rst with info on how to use this debug.conf?

Update main stack size for nrf5 boards. This is to support an alternative hci_driver that has a higher stack size usage in hci driver open. Measured stack usage in this case to 808/1024. Signed-off-by: Joakim Andersson <joakim.andersson@nordicsemi.no>

Refactor conf files to use prj.conf + board/<board>.conf configuration. This allows us to have put common configurations into the prj.conf and have board specific configs in each board file. This also respects adding additional prj.conf files such as -DCONF_FILE='nrf5.conf debug.conf' to add debug configuration. Signed-off-by: Joakim Andersson <joakim.andersson@nordicsemi.no>

Remove BT_CTLR_DTM_HCI=y config which is not supported for this board. This produced the following warning: warning: BT_CTLR_DTM_HCI was assigned the value 'y' but got the value 'n'. Check these unsatisfied dependencies: BT_CTLR_DTM_HCI_SUPPORT (=n). Signed-off-by: Joakim Andersson <joakim.andersson@nordicsemi.no>

Remove data length and phy update set to 'n' on board that does not support these features anyway. BT_CTLR_DATA_LEN_SUPPORTED and BT_CTLR_PHY_UPDATE_SUPPORTED are both set to 'n'. Signed-off-by: Joakim Andersson <joakim.andersson@nordicsemi.no>

joerchan · 2020-03-04T15:12:33Z

Split the stack debug changes into a new PR.
This is now only the main stack usage update and refactor of the conf files.
The refactor of the conf files is needed to easily add additional configuration options from debug conf file.

carlescufi

LGTM, thanks!

joerchan · 2020-03-13T12:13:31Z

@andrewboie The things you objected to have been removed from the PR.

joerchan requested review from jhedberg, nashif and Vudentz as code owners February 26, 2020 12:44

joerchan requested review from andrewboie and ioannisg February 26, 2020 12:45

zephyrbot added area: Bluetooth area: API Changes to public APIs area: Samples Samples labels Feb 26, 2020

jhedberg reviewed Feb 26, 2020

View reviewed changes

joerchan force-pushed the main-stack-debug branch from 318fe66 to 5b360a0 Compare February 26, 2020 16:23

andrewboie reviewed Mar 2, 2020

View reviewed changes

andrewboie suggested changes Mar 2, 2020

View reviewed changes

joerchan requested a review from andrewboie March 2, 2020 13:19

joerchan force-pushed the main-stack-debug branch from 5b360a0 to 75c43c1 Compare March 2, 2020 15:49

carlescufi reviewed Mar 2, 2020

View reviewed changes

joerchan force-pushed the main-stack-debug branch from 75c43c1 to d8dd663 Compare March 2, 2020 17:16

zephyrbot added the area: Documentation label Mar 2, 2020

joerchan force-pushed the main-stack-debug branch from d8dd663 to 51302e8 Compare March 2, 2020 17:19

joerchan mentioned this pull request Mar 3, 2020

Stack analyzer nrfconnect/sdk-nrf#1529

Closed

joerchan added 4 commits March 4, 2020 15:09

joerchan force-pushed the main-stack-debug branch from 51302e8 to a935834 Compare March 4, 2020 15:08

joerchan changed the title ~~Bluetooth: HCI_UART: Update stack sizes and provide stack usage debug~~ Bluetooth: HCI_UART: Update main stack and refactor conf files Mar 4, 2020

joerchan mentioned this pull request Mar 4, 2020

debug: Thread analyzer #23258

Merged

joerchan requested review from jhedberg and carlescufi March 13, 2020 11:15

carlescufi approved these changes Mar 13, 2020

View reviewed changes

jhedberg approved these changes Mar 13, 2020

View reviewed changes

andrewboie approved these changes Mar 13, 2020

View reviewed changes

andrewboie merged commit 5726a26 into zephyrproject-rtos:master Mar 13, 2020

joerchan deleted the main-stack-debug branch March 13, 2020 17:07

Bluetooth: HCI_UART: Update main stack and refactor conf files #23104

Bluetooth: HCI_UART: Update main stack and refactor conf files #23104

Uh oh!

Conversation

joerchan commented Feb 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joerchan commented Feb 26, 2020

Uh oh!

zephyrbot commented Feb 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jhedberg Feb 26, 2020

Choose a reason for hiding this comment

Uh oh!

joerchan Feb 27, 2020

Choose a reason for hiding this comment

Uh oh!

cvinayak commented Mar 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

andrewboie Mar 2, 2020

Choose a reason for hiding this comment

Uh oh!

joerchan Mar 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joerchan Mar 2, 2020

Choose a reason for hiding this comment

Uh oh!

andrewboie Mar 2, 2020

Choose a reason for hiding this comment

Uh oh!

joerchan Mar 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joerchan Mar 2, 2020

Choose a reason for hiding this comment

Uh oh!

andrewboie commented Mar 2, 2020

Uh oh!

andrewboie left a comment

Choose a reason for hiding this comment

Uh oh!

joerchan commented Mar 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joerchan commented Mar 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

carlescufi Mar 2, 2020

Choose a reason for hiding this comment

Uh oh!

joerchan commented Mar 4, 2020

Uh oh!

carlescufi left a comment

Choose a reason for hiding this comment

Uh oh!

joerchan commented Mar 13, 2020

Uh oh!

Uh oh!

joerchan commented Feb 26, 2020 •

edited

Loading

zephyrbot commented Feb 26, 2020 •

edited

Loading

cvinayak commented Mar 1, 2020 •

edited

Loading

joerchan Mar 2, 2020 •

edited

Loading

joerchan Mar 2, 2020 •

edited

Loading

joerchan commented Mar 2, 2020 •

edited

Loading

joerchan commented Mar 2, 2020 •

edited

Loading