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

posix: SCHED_RR valid thread priorities #56729

Closed
ambroise-arm opened this issue Apr 11, 2023 · 0 comments · Fixed by #57161
Closed

posix: SCHED_RR valid thread priorities #56729

ambroise-arm opened this issue Apr 11, 2023 · 0 comments · Fixed by #57161
Assignees
Labels
area: POSIX POSIX API Library bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug

Comments

@ambroise-arm
Copy link
Contributor

Describe the bug
Zephyr does not correctly advertize the minimum acceptable priority value that can be used with its posix threads when using SCHED_RR.

Calling sched_get_priority_min returns 0:

But looking at posix_to_zephyr_priority (used by pthread_create), 0 is not an acceptable value as it translates to a Zephyr thread priority of CONFIG_NUM_PREEMPT_PRIORITIES:

prio = (CONFIG_NUM_PREEMPT_PRIORITIES - priority);
which fails on the following assert when using the Zephyr test framework:
Z_ASSERT_VALID_PRIO(prio, entry);

Note that the priority set by pthread_attr_init is 1, so it isn't an issue when leaving the default thread priority. But this value looks arbitrary:

#define LOWEST_POSIX_THREAD_PRIORITY 1

To Reproduce
Steps to reproduce the behavior:

  1. Use CONFIG_PREEMPT_ENABLED=y in order to get SCHED_RR.
  2. Use CONFIG_ZTEST=y in order to get the kernel asserts.
  3. Have something like the following:
pthread_attr_t attr;
struct sched_param sched_param;
sched_param.sched_priority = sched_get_priority_min();
pthread_attr_init(&attr);
pthread_attr_setschedparam(&attr, &sched_param);
pthread_create(..., &attr, ...);

Expected behavior
No assert failed when using a priority advertized by Zephyr as being valid.

Impact
Annoyance

Logs and console output
With CONFIG_NUM_PREEMPT_PRIORITIES=15:

ASSERTION FAIL [((((prio)) == 15 && z_is_idle_thread_entry((entry))) || (((15 - 1) >= ((-16))) && ((prio)) >= ((-16)) && ((prio)) <= (15 - 1)))] @ WEST_TOPDIR/zephyr/kernel/thread.c:536
	invalid priority (15); allowed range: 14 to -16

Environment (please complete the following information):
Tested on tag zephyr-v3.3.0, looks still relevant on tip of main as of the time of writing.

Additional context
N/A

cc @PatrickM-ZS

@ambroise-arm ambroise-arm added the bug The issue is a bug, or the PR is fixing a bug label Apr 11, 2023
@henrikbrixandersen henrikbrixandersen added the area: POSIX POSIX API Library label Apr 11, 2023
@jgl-meta jgl-meta added the priority: low Low impact/importance bug label Apr 11, 2023
cfriedt added a commit to cfriedt/zephyr that referenced this issue Apr 25, 2023
Previously, there was an off-by-one error for SCHED_RR.

Fixes zephyrproject-rtos#56729

Signed-off-by: Chris Friedt <[email protected]>
cfriedt added a commit to cfriedt/zephyr that referenced this issue Apr 29, 2023
Verify that threads are actually schedulable for min and max
scheduler priority for both `SCHED_RR` (preemptive) and
`SCHED_FIFO` (cooperative).

Fixes zephyrproject-rtos#56729

Signed-off-by: Chris Friedt <[email protected]>
jgl-meta pushed a commit that referenced this issue Apr 29, 2023
Previously, there was an off-by-one error for SCHED_RR.

Fixes #56729

Signed-off-by: Chris Friedt <[email protected]>
jgl-meta pushed a commit that referenced this issue Apr 29, 2023
Verify that threads are actually schedulable for min and max
scheduler priority for both `SCHED_RR` (preemptive) and
`SCHED_FIFO` (cooperative).

Fixes #56729

Signed-off-by: Chris Friedt <[email protected]>
cfriedt added a commit that referenced this issue Apr 30, 2023
Previously, there was an off-by-one error for SCHED_RR.

Fixes #56729

Signed-off-by: Chris Friedt <[email protected]>
(cherry picked from commit 2b2cbf8)
cfriedt added a commit that referenced this issue Apr 30, 2023
Verify that threads are actually schedulable for min and max
scheduler priority for both `SCHED_RR` (preemptive) and
`SCHED_FIFO` (cooperative).

Fixes #56729

Signed-off-by: Chris Friedt <[email protected]>
(cherry picked from commit ad71b78)
cfriedt added a commit that referenced this issue May 2, 2023
Previously, there was an off-by-one error for SCHED_RR.

Fixes #56729

Signed-off-by: Chris Friedt <[email protected]>
(cherry picked from commit 2b2cbf8)
cfriedt added a commit that referenced this issue May 2, 2023
Verify that threads are actually schedulable for min and max
scheduler priority for both `SCHED_RR` (preemptive) and
`SCHED_FIFO` (cooperative).

Fixes #56729

Signed-off-by: Chris Friedt <[email protected]>
(cherry picked from commit ad71b78)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: POSIX POSIX API Library bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants