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

net: mgmt: Use proper coop thread priority value #32384

Conversation

jukkar
Copy link
Member

@jukkar jukkar commented Feb 17, 2021

If user sets CONFIG_NUM_PREEMPT_PRIORITIES=0, then the priority
of the net_mgmt thread will be -1 which is the same as idle thread.
This is will assert in kernel as then the minimum coop priority is -2
in this case. Remove the net_mgmt thread priority setting from
Kconfig file as it is low value and set the coop thread priority
the same way as other network threads are doing it.

Fixes #32375

Signed-off-by: Jukka Rissanen jukka.rissanen@linux.intel.com

If user sets CONFIG_NUM_PREEMPT_PRIORITIES=0, then the priority
of the net_mgmt thread will be -1 which is the same as idle thread.
This will trigger assert in kernel as then the minimum coop priority
is -2 in this case. Remove the net_mgmt thread priority setting from
Kconfig file as it is low value and set the coop thread priority
the same way as other network threads are doing it.

Fixes zephyrproject-rtos#32375

Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>
@jukkar jukkar force-pushed the bug-32375-net-mgmt-thread-wrong-coop-priority branch from f4c2112 to 18ecdce Compare February 17, 2021 09:43
@github-actions github-actions bot added the area: Tests Issues related to a particular existing or missing test label Feb 17, 2021
@carlescufi carlescufi requested a review from rlubos February 17, 2021 10:21
k_thread_create(&mgmt_thread_data, mgmt_stack,
K_KERNEL_STACK_SIZEOF(mgmt_stack),
(k_thread_entry_t)mgmt_thread, NULL, NULL, NULL,
CONFIG_NET_MGMT_EVENT_THREAD_PRIO, 0, K_NO_WAIT);
THREAD_PRIORITY, 0, K_NO_WAIT);
Copy link
Member

@mnkp mnkp Feb 17, 2021

Choose a reason for hiding this comment

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

There is a predefined K_LOWEST_APPLICATION_THREAD_PRIO in include/kernel.h which could be used here. The define is used by e.g. logger subsystem.

Copy link
Member Author

Choose a reason for hiding this comment

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

I did this change same way as the other networking threads are done so not using this define. The name of the define talks about application and we have no applications here but a thread which is part of a subsystem. But it is certainly possible to use that define if needed, but I would do that change in a separate PR if we really want to do it.

Copy link
Member

Choose a reason for hiding this comment

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

From the kernel perspective subsystem is an application. There is no thread priority range that is reserved for subsystems. So the name K_LOWEST_APPLICATION_THREAD_PRIO carries in fact the right meaning. Though I agree that seeing such a define in a networking stack might be confusing. From the user perspective subsystem is certainly not an application. Using the define was just a suggestion.

@jukkar jukkar merged commit 7699563 into zephyrproject-rtos:master Feb 17, 2021
@jukkar jukkar deleted the bug-32375-net-mgmt-thread-wrong-coop-priority branch February 17, 2021 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Networking area: Tests Issues related to a particular existing or missing test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Networking stack crashes when run in cooperative scheduling mode only
4 participants