posix: sched: Implement sched_getparam() and sched_getscheduler() #67028

golowanow · 2023-12-27T18:37:55Z

Implement sched_getparam() and sched_getscheduler() POSIX APIs
as a part of PSE53 _POSIX_PRIORITY_SCHEDULING option group.

Add CONFIG_POSIX_SCHED Kconfig option for APIs in the group.

Add pid_t typedef for some configurations where it is missed and its value derived from pthread_t.

Resolves #66959
Resolves #66960

include/zephyr/posix/posix_types.h

lib/posix/sched.c

cfriedt · 2024-01-03T12:26:51Z

lib/posix/posix_internal.h

+ *     must be already initialized whereas valid PID should
+ *     have positive value.
+ */
+static inline pthread_t z_get_tid(pid_t pid)


Personally, I would welcome proper support for separate Process IDs and processes done the right way at the kernel level, but it's probably not a good idea to try and implement a PID structure outside of the kernel.

Currently, we can only assume there is at most 1 process ID (maybe 2 in a sense) and that Zephyr runs in a single-process environment.

Of course, if you have plans to add that functionality to the kernel, it would be quite important to submit that as an RFC first. That would be very interesting.
https://github.com/zephyrproject-rtos/zephyr/issues/new/choose

Personally, I would welcome proper support for separate Process IDs and processes done the right way at the kernel level, but it's probably not a good idea to try and implement a PID structure outside of the kernel.

I agree, that's why I'm trying to have a single wrapper function here at POSIX API level which can be later adjusted to whatever decision (or not ?) will be at the Zephyr kernel in regard of Process object and its PID going in the opposite way from POSIX standard's history perspective.

There is a bunch of POSIX APIs with pid_t parameters and in case of sched_* they need to be related to a thread, so we can assume for now that PID value maps 1:1 to the 'main' thread's id.

will be at the Zephyr kernel in regard of Process object and its PID going in the opposite way from POSIX standard's history perspective.

Can you please clarify?

There is a bunch of POSIX APIs with pid_t parameters and in case of sched_* they need to be related to a thread, so we can assume for now that PID value maps 1:1 to the 'main' thread's id.

I'm not sure if they need to be related to a thread or should even be related to a single thread. sched_getparam(), and sched_getscheduler() operate on processes (groups of threads) according to

https://pubs.opengroup.org/onlinepubs/9699919799/functions/sched_getparam.html
https://pubs.opengroup.org/onlinepubs/9699919799/functions/sched_getscheduler.html

I get the impression that your reference in this PR is the Linux implementation of these calls.

https://linux.die.net/man/2/sched_getscheduler

Is that correct? If so, then this PR makes significantly more sense

The scheduling policy and parameters are in fact per-thread attributes on Linux. The value returned from a call to gettid(2) can be passed in the argument pid. Specifying pid as 0 will operate on the attribute for the calling thread, and passing the value returned from a call to getpid(2) will operate on the attribute for the main thread of the thread group.

Of course, the following is directly from the same page...

(If you are using the POSIX threads API, then use pthread_setschedparam(3), pthread_getschedparam(3), and pthread_setschedprio(3), instead of the sched_*(2) system calls.)

So I believe it's really only appropriate in Linux to use sched_getparam() and sched_getscheduler() to query (and analogously, sched_setparam() and sched_setscheduler() to modify) the scheduling parameters of individual threads.

will be at the Zephyr kernel in regard of Process object and its PID going in the opposite way from POSIX standard's history perspective.

Can you please clarify?

well, I just mean that threads were accepted into POSIX after processes, as an extension, and now for Zephyr it is a kind of opposite task to implement processes having threads already.

I get the impression that your reference in this PR is the Linux implementation of these calls.

https://linux.die.net/man/2/sched_getscheduler

Is that correct? If so, then this PR makes significantly more sense

GNU libc, Linux NTPL, and the APUE book

I had no intention to suggest any solution for Zephyr process design in this tiny PR )), rather a proposal to discuss it and possibly do something meaningful with these sched_get* functions instead of simply return 'not-yet-implemented'.

cfriedt · 2024-01-03T15:15:46Z

lib/posix/sched.c

+	struct sched_param dummy_param = {0};
+	pthread_t tid = 0;
+	int t_policy = -1;
+	int ret = -1;
+
+	if (param == NULL) {
+		param = &dummy_param;
+	}
+
+	tid = (pid == 0) ? pthread_self() : z_get_tid(pid);
+
+	ret = pthread_getschedparam(tid, &t_policy, param);
+	errno = ret;
+
+	return ((ret) ? -1 : 0);


It might be more in line with the POSIX specification to simply return -1 and set errno to ENOSYS here, since Zephyr does not (yet) support processes or process scheduling.

https://pubs.opengroup.org/onlinepubs/9699919799/functions/sched_getparam.html

Suggested change

struct sched_param dummy_param = {0};

pthread_t tid = 0;

int t_policy = -1;

int ret = -1;

if (param == NULL) {

param = &dummy_param;

}

tid = (pid == 0) ? pthread_self() : z_get_tid(pid);

ret = pthread_getschedparam(tid, &t_policy, param);

errno = ret;

return ((ret) ? -1 : 0);

ARG_UNUSED(pid);

ARG_UNUSED(param);

errno = ENOSYS;

return -1;

It might be more in line with the POSIX specification to simply return -1 and set errno to ENOSYS here, since Zephyr does not (yet) support processes or process scheduling.

what do you think if we deal only with pid==0 for pthread_self() and errno EINVAL otherwise ?

It might be more in line with the POSIX specification to simply return -1 and set errno to ENOSYS here, since Zephyr does not (yet) support processes or process scheduling.

what do you think if we deal only with pid==0 for pthread_self() and errno EINVAL otherwise ?

There is still a chance for a great deal of inconsistency, even from the perspective of just querying the policy / priority.

For example, Thread A and Thread B are both threads that belong to pid zero (i.e. are part of the same process), but when one looks at them, they see that the individual thread-level scheduling policy and priority are different. So inherently, we are conflating process scheduling and thread scheduling.

The situation can even get a bit more worrisome when one considers that some threads might be running in Kernel mode, and others in user mode.

I think in order to do what Linux does (and I'm not saying it's wrong - that would be kind of nice), there would need to be support from the kernel for essentially threads being equivalent to lightweight processes (which is what Linux does), and for the kernel to support processes as well (and for the main thread of a given process to be equivalent to its PID).

It's a fantastically interesting area for potential improvement, but I would not want to jump ahead of the kernel / architecture on it.

ok, removed.

The situation can even get a bit more worrisome when one considers that some threads might be running in Kernel mode, and others in user mode.

yeah, its another design question on what to do with kernel/user threads at the posix subsys level.

lib/posix/sched.c

tests/posix/common/src/pthread.c

cfriedt

Quite happy for the contribution. Some changes requested to better reflect the fact that Zephyr currently does not support processes or scheduling of processes, but otherwise, looks OK.

Documentation changes should also be in a separate commit.

Thanks!

golowanow · 2024-01-03T20:14:22Z

Documentation changes should also be in a separate commit.

do you mean to put 'yes' here - https://github.com/zephyrproject-rtos/zephyr/blob/main/doc/services/portability/posix/option_groups/index.rst?plain=1#L449-L450
or what is in commit with CONFIG_POSIX_SHED ?
is the former fair with ENOSYS functions, and the latter better to keep in the same commit ?

cfriedt · 2024-01-07T23:41:10Z

arch/posix/include/posix_cheats.h

+#define sched_getparam(...)		zap_sched_getparam(__VA_ARGS__)
+#define sched_getscheduler(...)		zap_sched_getscheduler(__VA_ARGS__)


I don't know if this is necessary as part of this PR. I would maybe suggest making it in a separate PR.

i thought to do it same way how other APIs were introduced back then (6 years ago!) keeping related changes together in the same PR

@aescolar, what do you think ?

It is not necessary but quite harmless.
Background: This POSIX API renames were used at the time when building the POSIX API together with the host libC was allowed (the POSIX API function names were renamed with this trick to avoid them colliding with the host libC). That combination is not supported or permitted anymore, so there is no need to add more (or keep the majority of these).

This POSIX API renames were used at the time when building the POSIX API together with the host libC was allowed (the POSIX API function names were renamed with this trick to avoid them colliding with the host libC). That combination is not supported or permitted anymore, so there is no need to add more (or keep the majority of these).

is a bulk cleanup already planned for posix_cheats.h ? or should it be done gradually for API groups like sched_* ?
in scope of this PR it looks confusing to have only some of these APIs there.

It is in my low priority TODO. So it should happen eventually, probably when I need to change something around there.

doc/services/portability/posix/aep/index.rst

doc/services/portability/posix/conformance/index.rst

include/zephyr/posix/posix_types.h

lib/posix/Kconfig.sched

cfriedt · 2024-01-08T00:10:33Z

Documentation changes should also be in a separate commit.

do you mean to put 'yes' here - https://github.com/zephyrproject-rtos/zephyr/blob/main/doc/services/portability/posix/option_groups/index.rst?plain=1#L449-L450 or what is in commit with CONFIG_POSIX_SHED ? is the former fair with ENOSYS functions, and the latter better to keep in the same commit ?

Yep - that's about it. I made some code comments to hopefully clarify.

Add `CONFIG_POSIX_PRIORITY_SCHEDULING` Kconfig option to select APIs from PSE53 `_POSIX_PRIORITY_SCHEDULING` option group. Signed-off-by: Dmitrii Golovanov <dmitrii.golovanov@intel.com>

Initial implementation of `sched_getparam()` and `sched_getscheduler()` POSIX APIs as a part of PSE53 `_POSIX_PRIORITY_SCHEDULING` option group. Both functions are actually placeholders and just return `ENOSYS` since Zephyr does not yet support processes or process scheduling. Signed-off-by: Dmitrii Golovanov <dmitrii.golovanov@intel.com>

lib/posix/Kconfig.sched

golowanow force-pushed the posix_66959 branch 2 times, most recently from 70ac9c5 to 64a15b4 Compare December 27, 2023 19:51

golowanow changed the title ~~posix: sched: Implements sched_getparam()~~ posix: sched: Implement sched_getparam() and sched_getscheduler() Dec 27, 2023

golowanow force-pushed the posix_66959 branch from 64a15b4 to 8bd3471 Compare December 27, 2023 21:08

golowanow marked this pull request as ready for review December 27, 2023 22:33

zephyrbot added area: native port Host native arch port (native_sim) area: POSIX POSIX API Library labels Dec 27, 2023

zephyrbot requested review from aescolar and cfriedt December 27, 2023 22:34

zephyrbot assigned cfriedt Dec 27, 2023

golowanow requested a review from ycsin December 27, 2023 22:34

moonlight83340 mentioned this pull request Dec 28, 2023

posix: sched: Implement sched_setparam() and sched_setscheduler() #67038

Closed

cfriedt reviewed Jan 1, 2024

View reviewed changes

include/zephyr/posix/posix_types.h Outdated Show resolved Hide resolved

cfriedt reviewed Jan 1, 2024

View reviewed changes

lib/posix/sched.c Outdated Show resolved Hide resolved

cfriedt reviewed Jan 1, 2024

View reviewed changes

lib/posix/sched.c Outdated Show resolved Hide resolved

golowanow force-pushed the posix_66959 branch 2 times, most recently from ef03798 to 8218bea Compare January 1, 2024 10:54

golowanow requested a review from cfriedt January 1, 2024 11:02

golowanow force-pushed the posix_66959 branch 2 times, most recently from 6495257 to 2fbb113 Compare January 1, 2024 14:32

cfriedt reviewed Jan 3, 2024

View reviewed changes

lib/posix/sched.c Outdated Show resolved Hide resolved

cfriedt reviewed Jan 3, 2024

View reviewed changes

tests/posix/common/src/pthread.c Outdated Show resolved Hide resolved

cfriedt reviewed Jan 3, 2024

View reviewed changes

tests/posix/common/src/pthread.c Outdated Show resolved Hide resolved

cfriedt requested changes Jan 3, 2024

View reviewed changes

cfriedt mentioned this pull request Jan 3, 2024

posix: signal: implement sigprocmask() #67008

Merged

golowanow force-pushed the posix_66959 branch from 2fbb113 to 9d34ae2 Compare January 3, 2024 19:08

golowanow requested a review from cfriedt January 3, 2024 20:14

cfriedt reviewed Jan 7, 2024

View reviewed changes

doc/services/portability/posix/aep/index.rst Outdated Show resolved Hide resolved

cfriedt reviewed Jan 7, 2024

View reviewed changes

doc/services/portability/posix/conformance/index.rst Outdated Show resolved Hide resolved

cfriedt reviewed Jan 7, 2024

View reviewed changes

include/zephyr/posix/posix_types.h Outdated Show resolved Hide resolved

cfriedt reviewed Jan 8, 2024

View reviewed changes

lib/posix/Kconfig.sched Outdated Show resolved Hide resolved

golowanow added 2 commits January 14, 2024 07:31

posix: sched: Add CONFIG_POSIX_PRIORITY_SCHEDULING

e67bea5

Add `CONFIG_POSIX_PRIORITY_SCHEDULING` Kconfig option to select APIs from PSE53 `_POSIX_PRIORITY_SCHEDULING` option group. Signed-off-by: Dmitrii Golovanov <dmitrii.golovanov@intel.com>

golowanow force-pushed the posix_66959 branch from 9d34ae2 to f3f0be3 Compare January 14, 2024 07:05

golowanow requested a review from cfriedt January 14, 2024 07:20

cfriedt reviewed Jan 14, 2024

View reviewed changes

lib/posix/Kconfig.sched Show resolved Hide resolved

cfriedt approved these changes Jan 14, 2024

View reviewed changes

ycsin approved these changes Jan 15, 2024

View reviewed changes

carlescufi merged commit 68d1a52 into zephyrproject-rtos:main Jan 15, 2024
38 checks passed

golowanow deleted the posix_66959 branch January 15, 2024 09:02

moonlight83340 mentioned this pull request Feb 5, 2024

doc: posix: mark putmsg as supported #68502

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

posix: sched: Implement sched_getparam() and sched_getscheduler() #67028

posix: sched: Implement sched_getparam() and sched_getscheduler() #67028

golowanow commented Dec 27, 2023 •

edited

Loading

cfriedt Jan 3, 2024 •

edited

Loading

golowanow Jan 3, 2024

cfriedt Jan 3, 2024 •

edited

Loading

golowanow Jan 3, 2024 •

edited

Loading

cfriedt Jan 3, 2024

golowanow Jan 3, 2024 •

edited

Loading

cfriedt Jan 3, 2024 •

edited

Loading

golowanow Jan 3, 2024

cfriedt left a comment •

edited

Loading

golowanow commented Jan 3, 2024 •

edited

Loading

cfriedt Jan 7, 2024

golowanow Jan 14, 2024

aescolar Jan 14, 2024

golowanow Jan 14, 2024

aescolar Jan 15, 2024

cfriedt commented Jan 8, 2024

		#define sched_getparam(...) zap_sched_getparam(__VA_ARGS__)
		#define sched_getscheduler(...) zap_sched_getscheduler(__VA_ARGS__)

posix: sched: Implement sched_getparam() and sched_getscheduler() #67028

posix: sched: Implement sched_getparam() and sched_getscheduler() #67028

Conversation

golowanow commented Dec 27, 2023 • edited Loading

cfriedt Jan 3, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cfriedt Jan 3, 2024 • edited Loading

Choose a reason for hiding this comment

golowanow Jan 3, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

golowanow Jan 3, 2024 • edited Loading

Choose a reason for hiding this comment

cfriedt Jan 3, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cfriedt left a comment • edited Loading

Choose a reason for hiding this comment

golowanow commented Jan 3, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cfriedt commented Jan 8, 2024

golowanow commented Dec 27, 2023 •

edited

Loading

cfriedt Jan 3, 2024 •

edited

Loading

cfriedt Jan 3, 2024 •

edited

Loading

golowanow Jan 3, 2024 •

edited

Loading

golowanow Jan 3, 2024 •

edited

Loading

cfriedt Jan 3, 2024 •

edited

Loading

cfriedt left a comment •

edited

Loading

golowanow commented Jan 3, 2024 •

edited

Loading