Skip to content

kernel: posix: implement schedule related APIs for POSIX. #5510

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

Closed

Conversation

youvedeep
Copy link
Contributor

Map Zephyr APIs to Posix APIs corresponding to scheduler.

Signed-off-by: Youvedeep Singh [email protected]

Map Zephyr APIs to Posix APIs corresponding to scheduler.

Signed-off-by: Youvedeep Singh <[email protected]>
@SebastianBoe SebastianBoe removed their request for review December 29, 2017 10:31
* SPDX-License-Identifier: Apache-2.0
*/

#include <stdint.h>
Copy link
Collaborator

@lpereira lpereira Jan 2, 2018

Choose a reason for hiding this comment

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

There's no need to include stdint.h here: this header file is not using any definition from this header. Add the #include directive to the implementation file, if required there. Also, please add an include guard.

#include <pthread.h>

/*
* ======== sched_get_priority_min ========
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please provide the documentation in the header file, using the Doxygen format.

return -EINVAL;
}
#if defined(CONFIG_COOP_ENABLED) && defined(CONFIG_PREEMPT_ENABLED)
priority = (policy == SCHED_COOP) ? (-1 * -1) :
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why -1 * -1?


int priority = -EINVAL;

if (!((policy == CONFIG_COOP_ENABLED) || (policy == SCHED_PREMPT))) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

CONFIG_COOP_ENABLED sounds like a Zephyr thing, rather than a POSIX thing. Is it correct here?

@@ -0,0 +1,15 @@
/*
Copy link
Collaborator

Choose a reason for hiding this comment

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

These definitions are in a file named <sched.h>; shouldn't them be in a file with the same name so that code that assumes this will be able to find them without change?

priority = (policy == SCHED_PREMPT) ? K_LOWEST_APPLICATION_THREAD_PRIO :
-EINVAL;
#endif
return priority;
Copy link
Collaborator

Choose a reason for hiding this comment

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

These functions will return -1 on error, and the priority on success. It's returning a negative errno on error.

@@ -0,0 +1,15 @@
/*
* Copyright (c) 2017 Intel Corporation
Copy link
Contributor

Choose a reason for hiding this comment

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

Now that it's 2018, these copyright headers need updating when (significant) changes are made, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

I've personally never received any guidance that this is necessary.

* returns: priority corresponing to policy
* -EINVAL for Error
*/
int sched_get_priority_min(int policy)
Copy link
Collaborator

@lpereira lpereira Jan 2, 2018

Choose a reason for hiding this comment

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

This function can be simplified by using the IS_ENABLED() macro. Not complete, of course, just something to consider:

int sched_get_priority_min(int policy)
{
    if (IS_ENABLED(CONFIG_COOP_ENABLED)) {
       if (priority == SCHED_COOP) {
            return K_LOWEST_APPLICATION_THREAD_PRIO;
       }
    }
    if (IS_ENABLED(CONFIG_PREEMPT_ENABLED)) {
        if (priority == SCHED_PREEMPT) {
            return K_LOWEST_APPLICATION_THREAD_PRIO;
        }
    }

    errno = EINVAL;
    return -1;
}

@youvedeep
Copy link
Contributor Author

patch in this PR is discussed as part of PR : #5667
So please have a look at #5667 and provide comment.

@youvedeep
Copy link
Contributor Author

This PR is put as part of - #5667.
So please give review comments as part of #5667

@youvedeep youvedeep closed this Jan 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants