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.

Sign up for GitHub

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

[Backport v2.7] posix: sched: ensure min and max priority are schedulable #57414

Merged
merged 2 commits into from
May 2, 2023
Merged

[Backport v2.7] posix: sched: ensure min and max priority are schedulable #57414

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
26ffb63
posix: sched: ensure min and max priority are schedulable
cfriedt Apr 24, 2023
347de78
tests: posix: ensure that min and max priority are schedulable
cfriedt Apr 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions lib/posix/pthread.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,10 @@
#define PTHREAD_INIT_FLAGS PTHREAD_CANCEL_ENABLE
#define PTHREAD_CANCELED ((void *) -1)

#define LOWEST_POSIX_THREAD_PRIORITY 1

PTHREAD_MUTEX_DEFINE(pthread_key_lock);

static const pthread_attr_t init_pthread_attrs = {
.priority = LOWEST_POSIX_THREAD_PRIORITY,
.priority = 0,
.stack = NULL,
.stacksize = 0,
.flags = PTHREAD_INIT_FLAGS,
Expand Down Expand Up @@ -54,9 +52,11 @@ static uint32_t zephyr_to_posix_priority(int32_t z_prio, int *policy)
if (z_prio < 0) {
*policy = SCHED_FIFO;
prio = -1 * (z_prio + 1);
__ASSERT_NO_MSG(prio < CONFIG_NUM_COOP_PRIORITIES);
} else {
*policy = SCHED_RR;
prio = (CONFIG_NUM_PREEMPT_PRIORITIES - z_prio);
prio = (CONFIG_NUM_PREEMPT_PRIORITIES - z_prio - 1);
__ASSERT_NO_MSG(prio < CONFIG_NUM_PREEMPT_PRIORITIES);
}

return prio;
Expand All @@ -68,9 +68,11 @@ static int32_t posix_to_zephyr_priority(uint32_t priority, int policy)

if (policy == SCHED_FIFO) {
/* Zephyr COOP priority starts from -1 */
__ASSERT_NO_MSG(priority < CONFIG_NUM_COOP_PRIORITIES);
prio = -1 * (priority + 1);
} else {
prio = (CONFIG_NUM_PREEMPT_PRIORITIES - priority);
__ASSERT_NO_MSG(priority < CONFIG_NUM_PREEMPT_PRIORITIES);
prio = (CONFIG_NUM_PREEMPT_PRIORITIES - priority - 1);
}

return prio;
Expand Down
48 changes: 8 additions & 40 deletions lib/posix/pthread_sched.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,9 @@
#include <kernel.h>
#include <posix/posix_sched.h>

static bool valid_posix_policy(int policy)
static inline bool valid_posix_policy(int policy)
{
if (policy != SCHED_FIFO && policy != SCHED_RR) {
return false;
}

return true;
return policy == SCHED_FIFO || policy == SCHED_RR;
}

/**
Expand All @@ -23,25 +19,12 @@ static bool valid_posix_policy(int policy)
*/
int sched_get_priority_min(int policy)
{
if (valid_posix_policy(policy) == false) {
if (!valid_posix_policy(policy)) {
errno = EINVAL;
return -1;
}

if (IS_ENABLED(CONFIG_COOP_ENABLED)) {
if (policy == SCHED_FIFO) {
return 0;
}
}

if (IS_ENABLED(CONFIG_PREEMPT_ENABLED)) {
if (policy == SCHED_RR) {
return 0;
}
}

errno = EINVAL;
return -1;
return 0;
}

/**
Expand All @@ -51,25 +34,10 @@ int sched_get_priority_min(int policy)
*/
int sched_get_priority_max(int policy)
{
if (valid_posix_policy(policy) == false) {
errno = EINVAL;
return -1;
}

if (IS_ENABLED(CONFIG_COOP_ENABLED)) {
if (policy == SCHED_FIFO) {
/* Posix COOP priority starts from 0
* whereas zephyr starts from -1
*/
return (CONFIG_NUM_COOP_PRIORITIES - 1);
}

}

if (IS_ENABLED(CONFIG_PREEMPT_ENABLED)) {
if (policy == SCHED_RR) {
return CONFIG_NUM_PREEMPT_PRIORITIES;
}
if (IS_ENABLED(CONFIG_COOP_ENABLED) && policy == SCHED_FIFO) {
return CONFIG_NUM_COOP_PRIORITIES - 1;
} else if (IS_ENABLED(CONFIG_PREEMPT_ENABLED) && policy == SCHED_RR) {
return CONFIG_NUM_PREEMPT_PRIORITIES - 1;
}

errno = EINVAL;
Expand Down
4 changes: 3 additions & 1 deletion tests/posix/common/src/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ extern void test_nanosleep_1_1(void);
extern void test_nanosleep_1_1001(void);
extern void test_sleep(void);
extern void test_usleep(void);
extern void test_sched_policy(void);

void test_main(void)
{
Expand Down Expand Up @@ -73,7 +74,8 @@ void test_main(void)
ztest_unit_test(test_nanosleep_1_1001),
ztest_unit_test(test_posix_pthread_create_negative),
ztest_unit_test(test_sleep),
ztest_unit_test(test_usleep)
ztest_unit_test(test_usleep),
ztest_unit_test(test_sched_policy)
);
ztest_run_test_suite(posix_apis);
}
129 changes: 129 additions & 0 deletions tests/posix/common/src/pthread.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -577,3 +577,132 @@ void test_pthread_descriptor_leak(void)
zassert_ok(pthread_join(pthread1, &unused), "unable to join thread %zu", i);
}
}

void test_sched_policy(void)
{
/*
* TODO:
* 1. assert that _POSIX_PRIORITY_SCHEDULING is defined
* 2. if _POSIX_SPORADIC_SERVER or _POSIX_THREAD_SPORADIC_SERVER are defined,
* also check SCHED_SPORADIC
* 3. SCHED_OTHER is mandatory (but may be equivalent to SCHED_FIFO or SCHED_RR,
* and is implementation defined)
*/

int pmin;
int pmax;
pthread_t th;
pthread_attr_t attr;
struct sched_param param;
static const int policies[] = {
SCHED_FIFO,
SCHED_RR,
SCHED_INVALID,
};
static const char *const policy_names[] = {
"SCHED_FIFO",
"SCHED_RR",
"SCHED_INVALID",
};
static const bool policy_enabled[] = {
IS_ENABLED(CONFIG_COOP_ENABLED),
IS_ENABLED(CONFIG_PREEMPT_ENABLED),
false,
};
static int nprio[] = {
CONFIG_NUM_COOP_PRIORITIES,
CONFIG_NUM_PREEMPT_PRIORITIES,
42,
};
const char *const prios[] = {"pmin", "pmax"};

BUILD_ASSERT(!(SCHED_INVALID == SCHED_FIFO || SCHED_INVALID == SCHED_RR),
"SCHED_INVALID is itself invalid");

for (int policy = 0; policy < ARRAY_SIZE(policies); ++policy) {
if (!policy_enabled[policy]) {
/* test degenerate cases */
errno = 0;
zassert_equal(-1, sched_get_priority_min(policies[policy]),
"expected sched_get_priority_min(%s) to fail",
policy_names[policy]);
zassert_equal(EINVAL, errno, "sched_get_priority_min(%s) did not set errno",
policy_names[policy]);

errno = 0;
zassert_equal(-1, sched_get_priority_max(policies[policy]),
"expected sched_get_priority_max(%s) to fail",
policy_names[policy]);
zassert_equal(EINVAL, errno, "sched_get_priority_max(%s) did not set errno",
policy_names[policy]);
continue;
}

/* get pmin and pmax for policies[policy] */
for (int i = 0; i < 2; ++i) {
errno = 0;
if (i == 0) {
pmin = sched_get_priority_min(policies[policy]);
param.sched_priority = pmin;
} else {
pmax = sched_get_priority_max(policies[policy]);
param.sched_priority = pmax;
}

zassert_not_equal(-1, param.sched_priority,
"sched_get_priority_%s(%s) failed: %d",
i == 0 ? "min" : "max", policy_names[policy], errno);
zassert_equal(0, errno, "sched_get_priority_%s(%s) set errno to %s",
i == 0 ? "min" : "max", policy_names[policy], errno);
}

/*
* IEEE 1003.1-2008 Section 2.8.4
* conforming implementations should provide a range of at least 32 priorities
*
* Note: we relax this requirement
*/
zassert_true(pmax > pmin, "pmax (%d) <= pmin (%d)", pmax, pmin,
"%s min/max inconsistency: pmin: %d pmax: %d", policy_names[policy],
pmin, pmax);

/*
* Getting into the weeds a bit (i.e. whitebox testing), Zephyr
* cooperative threads use [-CONFIG_NUM_COOP_PRIORITIES,-1] and
* preemptive threads use [0, CONFIG_NUM_PREEMPT_PRIORITIES - 1],
* where the more negative thread has the higher priority. Since we
* cannot map those directly (a return value of -1 indicates error),
* we simply map those to the positive space.
*/
zassert_equal(pmin, 0, "unexpected pmin for %s", policy_names[policy]);
zassert_equal(pmax, nprio[policy] - 1, "unexpected pmax for %s",
policy_names[policy]); /* test happy paths */

for (int i = 0; i < 2; ++i) {
/* create threads with min and max priority levels */
zassert_equal(0, pthread_attr_init(&attr),
"pthread_attr_init() failed for %s (%d) of %s", prios[i],
param.sched_priority, policy_names[policy]);

zassert_equal(0, pthread_attr_setschedpolicy(&attr, policies[policy]),
"pthread_attr_setschedpolicy() failed for %s (%d) of %s",
prios[i], param.sched_priority, policy_names[policy]);

zassert_equal(0, pthread_attr_setschedparam(&attr, &param),
"pthread_attr_setschedparam() failed for %s (%d) of %s",
prios[i], param.sched_priority, policy_names[policy]);

zassert_equal(0, pthread_attr_setstack(&attr, &stack_e[0][0], STACKS),
"pthread_attr_setstack() failed for %s (%d) of %s", prios[i],
param.sched_priority, policy_names[policy]);

zassert_equal(0, pthread_create(&th, &attr, create_thread1, NULL),
"pthread_create() failed for %s (%d) of %s", prios[i],
param.sched_priority, policy_names[policy]);

zassert_equal(0, pthread_join(th, NULL),
"pthread_join() failed for %s (%d) of %s", prios[i],
param.sched_priority, policy_names[policy]);
}
}
}