[PATCH] pthreads: provide minimal implementation to handle SCHED_OTHER policy

[Waldemar Kozaczuk]

This patch provides minimal implementation of following
4 functions to handle SCHED_OTHER policy:
- sched_get_priority_min
- sched_get_priority_max
- pthread_getschedparam
- pthread_setschedparam

This implementation of these 4 functions is enough to make simple 'Hello World' -
https://github.com/cloudius-systems/osv-apps/tree/master/mono-example - run properly on OSv.

Fixes #34

Signed-off-by: Waldemar Kozaczuk <jwkoz...@gmail.com>
---
libc/pthread.cc | 33 ++++++++++++++++++++++++---------
1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/libc/pthread.cc b/libc/pthread.cc
index 125d579f..9b4eb768 100644
--- a/libc/pthread.cc
+++ b/libc/pthread.cc
@@ -928,30 +928,45 @@ void pthread_exit(void *retval)
t->_thread->exit();
}

-int sched_get_priority_max(int policy)
+static int sched_get_priority_minmax(int policy)
{
- WARN_STUBBED();
- return EINVAL;
+ switch (policy) {
+ case SCHED_OTHER:
+ return 0;
+ default:
+ return EINVAL;
+ }
}

+// Following 4 functions provide minimal implementation
+// that ONLY covers default Linux SCHED_OTHER policy
int sched_get_priority_min(int policy)
{
- WARN_STUBBED();
- return EINVAL;
+ return sched_get_priority_minmax(policy);
+}
+
+int sched_get_priority_max(int policy)
+{
+ return sched_get_priority_minmax(policy);
}

int pthread_setschedparam(pthread_t thread, int policy,
const struct sched_param *param)
{
- WARN_STUBBED();
- return EINVAL;
+ switch (policy) {
+ case SCHED_OTHER:
+ return 0;
+ default:
+ return EINVAL;
+ }
}

int pthread_getschedparam(pthread_t thread, int *policy,
struct sched_param *param)
{
- WARN_STUBBED();
- return EINVAL;
+ *policy = SCHED_OTHER;
+ param->sched_priority = 0;
+ return 0;
}

int pthread_kill(pthread_t thread, int sig)
--
2.20.1

[Waldek Kozaczuk]

I looked at number of patches sent years ago that fully implemented the scheduler policies including the real time one and came up with this minimal implementation supporting the SCHED_OTHER. I am not sure though if I have not missed anything.

[Nadav Har'El]

On Thu, Aug 22, 2019 at 4:43 AM Waldemar Kozaczuk <jwkoz...@gmail.com> wrote:

This patch provides minimal implementation of following
4 functions to handle SCHED_OTHER policy:
- sched_get_priority_min
- sched_get_priority_max
- pthread_getschedparam
- pthread_setschedparam

This implementation of these 4 functions is enough to make simple 'Hello World' -
https://github.com/cloudius-systems/osv-apps/tree/master/mono-example - run properly on OSv.

Fixes #34

Thanks. Would be great to close #34, and with such a simple patch, but unfortunately I have one nitpick below:

Signed-off-by: Waldemar Kozaczuk <jwkoz...@gmail.com>
---
 libc/pthread.cc | 33 ++++++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/libc/pthread.cc b/libc/pthread.cc
index 125d579f..9b4eb768 100644
--- a/libc/pthread.cc
+++ b/libc/pthread.cc
@@ -928,30 +928,45 @@ void pthread_exit(void *retval)
     t->_thread->exit();
 }

-int sched_get_priority_max(int policy)
+static int sched_get_priority_minmax(int policy)

I don't want to replace the two completely different functions, max() and min() with just one.

Even if it means code duplication. Because "eventually" (maybe in 10 years ;-)), these are supposed to

be different functions.

 {

-    WARN_STUBBED();
-    return EINVAL;
+    switch (policy) {
+        case SCHED_OTHER:
+            return 0;

Did  you check that Linux returns 0 here, and not some actual numbers like -20, 20, etc?

--
You received this message because you are subscribed to the Google Groups "OSv Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to osv-dev+u...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/osv-dev/20190822014340.7771-1-jwkozaczuk%40gmail.com.

[Nadav Har'El]

On Sat, Aug 24, 2019 at 5:16 PM Waldek Kozaczuk <jwkoz...@gmail.com> wrote:

I looked at number of patches sent years ago that fully implemented the scheduler policies including the real time one

Yes, I actually spent quite a bit of effort in the past to add to our scheduler (a big part of) the machinary needed for realtime scheduling. If anyone ever cares about this again, we can revive those patches. It's on the mailing list archive (v3 of "sched: implement POSIX-compatible real-time scheduling") and also my own disk.

 

and came up with this minimal implementation supporting the SCHED_OTHER. I am not sure though if I have not missed anything.

--
You received this message because you are subscribed to the Google Groups "OSv Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to osv-dev+u...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/osv-dev/caa1ab72-fe0d-4f7c-9d41-8655a25a2073%40googlegroups.com.

[Waldemar Kozaczuk]

This patch provides minimal implementation of following
4 functions to handle SCHED_OTHER policy:
- sched_get_priority_min
- sched_get_priority_max
- pthread_getschedparam
- pthread_setschedparam

This implementation of these 4 functions is enough to make simple 'Hello World' -
https://github.com/cloudius-systems/osv-apps/tree/master/mono-example - run properly on OSv.

Fixes #34

Signed-off-by: Waldemar Kozaczuk <jwkoz...@gmail.com>
---

libc/pthread.cc | 35 +++++++++++++++++++++++++----------
1 file changed, 25 insertions(+), 10 deletions(-)

diff --git a/libc/pthread.cc b/libc/pthread.cc
index 125d579f..44b93b83 100644

--- a/libc/pthread.cc
+++ b/libc/pthread.cc
@@ -928,30 +928,45 @@ void pthread_exit(void *retval)
t->_thread->exit();
}

-int sched_get_priority_max(int policy)

+// Following 4 functions provide minimal implementation
+// that ONLY covers default Linux SCHED_OTHER policy

+int sched_get_priority_min(int policy)

{
- WARN_STUBBED();
- return EINVAL;

+ switch (policy) {

+ case SCHED_OTHER:
+ return 0;
+ default:
+ return EINVAL;
+ }
}

-int sched_get_priority_min(int policy)
+int sched_get_priority_max(int policy)

{
- WARN_STUBBED();
- return EINVAL;

+ switch (policy) {

[Waldek Kozaczuk]

I have sent new updated patch.

Yes in two places:

1) Per http://man7.org/linux/man-pages/man2/sched_get_priority_min.2.html which says this:

"

Linux allows the static priority range 1 to 99 for the SCHED_FIFO and
       SCHED_RR policies, and the priority 0 for the remaining policies.
       Scheduling priority ranges for the various policies are not
       alterable.

" 

2) Also in the code - https://github.com/torvalds/linux/blob/master/kernel/sched/core.c#L5613-L5656

To unsubscribe from this group and stop receiving emails from it, send an email to osv...@googlegroups.com.

[Commit Bot]

From: Waldemar Kozaczuk <jwkoz...@gmail.com>
Committer: Waldemar Kozaczuk <jwkoz...@gmail.com>
Branch: master

pthreads: provide minimal implementation to handle SCHED_OTHER policy

Signed-off-by: Waldemar Kozaczuk <jwkoz...@gmail.com>

---

diff --git a/libc/pthread.cc b/libc/pthread.cc

--- a/libc/pthread.cc
+++ b/libc/pthread.cc
@@ -928,30 +928,45 @@ void pthread_exit(void *retval)
t->_thread->exit();
}

-int sched_get_priority_max(int policy)

+// Following 4 functions provide minimal implementation
+// that ONLY covers default Linux SCHED_OTHER policy

+int sched_get_priority_min(int policy)

{
- WARN_STUBBED();
- return EINVAL;

+ switch (policy) {

+ case SCHED_OTHER:
+ return 0;
+ default:
+ return EINVAL;
+ }
}

-int sched_get_priority_min(int policy)
+int sched_get_priority_max(int policy)

{
- WARN_STUBBED();
- return EINVAL;

+ switch (policy) {

[Waldek Kozaczuk]

I have just committed this patch by accident. But I hope it addressed your suggestions accurately.

[Nadav Har'El]

Yes, it's fine. Thanks.

--
Nadav Har'El

n...@scylladb.com

To unsubscribe from this group and stop receiving emails from it, send an email to osv-dev+u...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/osv-dev/d699b16c-ce63-457a-92c2-68e3a1918dbb%40googlegroups.com.