[V3 0/3] cyclictest support

35 views
Skip to first unread message

luca abeni

unread,
Jun 5, 2023, 8:19:29 AM6/5/23
to osv...@googlegroups.com, luca abeni
From: luca abeni <luca...@gmail.com>

Hi,

here is Version 3 of the patches I am using to run a simple real-time test
(cyclictest) on OSv.
The patches are based on github issue 386
(https://github.com/cloudius-systems/osv/issues/386) and are a forward
port of patches by Claudio Fontana (an additional patch by Nadav Har'El
has already been committed, so I dropped it). Respect to version 2 of
the patchset, I dropped patch 0001 (already applied) and I modified the
patches according to the previous review (I hope I got it right):
- Fixed a comment about sched_rr_get_interval() (it is not
Linux-specific)
- Fixed all the wrong checks in which getpid() was compared with a
thread id; I also tried to use sched::with_thread_by_id() when needed,
I hope I did it right. cyclictest still works correctly, but I did not
test much more
- Removed some commented code
- Dropped the changes to tests/misc-scheduler.cc, which looked wrong
- Changed the name of the new thread::sleep() function to
thread::sleep_until()
- Added "OSV_LIBC_API" to clock_nanosleep()
- Removed a useless check in the "TIMER_ABSTIME" case of
clock_nanosleep(). After some tests, it seems to me that if sleep_until()
is provided with a time in the past, it does not block the thread (I
think it inserts a wakeup timer in the CPU's timers list, but then the
timer immediately fires when setting the clockevent and the timer's
expired() method immediately returns true... So, the wait_until() call in
clock_nanosleep() returns immediately without blocking)
- Removed all the timespec* macros (only timespeccmp was used, but with
the last change it is not used anymore)

I do not think I changed anything major, so I did not add my name in the
copyright, I hope this is OK.


With these patches, cyclictest can start and measure some latencies; the
resulting worst-case latencies look better than with the original
version of the patches.


Claudio Fontana (2):
libc: add RT sched API
libc/time: add stub for clock_nanosleep

luca abeni (1):
Define __NEED_size_t in include/api/sched.h

Makefile | 2 +
include/api/sched.h | 1 +
include/osv/sched.hh | 10 +++
libc/pthread.cc | 22 ------
libc/sched.cc | 160 +++++++++++++++++++++++++++++++++++++++++++
libc/time.cc | 32 ++++++++-
6 files changed, 204 insertions(+), 23 deletions(-)
create mode 100644 libc/sched.cc

--
2.30.2

luca abeni

unread,
Jun 5, 2023, 8:19:34 AM6/5/23
to osv...@googlegroups.com, luca abeni
From: luca abeni <luca...@gmail.com>

This header file actually uses size_t (for example, in the definition of
__sched_cpucount()). Hence, including api/sched.h without having defined
__NED_size_t before can result in build errors.
This issue is not visible now because api/sched.h has only one used, which
includes it after osv/sched.hh, which ends up defining __NEED_size_t.
---
include/api/sched.h | 1 +
1 file changed, 1 insertion(+)

diff --git a/include/api/sched.h b/include/api/sched.h
index 1893efc1..68074b22 100644
--- a/include/api/sched.h
+++ b/include/api/sched.h
@@ -9,6 +9,7 @@ extern "C" {
#define __NEED_struct_timespec
#define __NEED_pid_t
#define __NEED_time_t
+#define __NEED_size_t

#include <bits/alltypes.h>

--
2.30.2

luca abeni

unread,
Jun 5, 2023, 8:19:36 AM6/5/23
to osv...@googlegroups.com, Claudio Fontana
From: Claudio Fontana <claudio...@huawei.com>

implement the sched_ API using the OSv realtime scheduler
implementation.

This includes:

sched_setscheduler, sched_getscheduler,
sched_setparam, sched_getparam,
sched_get_priority_min, sched_get_priority_max,
and a stub for Linux-only sched_rr_get_interval.

They are implemented together as they are tightly
integrated, and one calls the other.

Signed-off-by: Claudio Fontana <claudio...@huawei.com>
---
Makefile | 2 +
libc/pthread.cc | 22 -------
libc/sched.cc | 160 ++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 162 insertions(+), 22 deletions(-)
create mode 100644 libc/sched.cc

diff --git a/Makefile b/Makefile
index af0a9585..43755da3 100644
--- a/Makefile
+++ b/Makefile
@@ -1914,6 +1914,8 @@ libc += mallopt.o

libc += linux/makedev.o

+libc += sched.o
+
musl += fenv/fegetexceptflag.o
musl += fenv/feholdexcept.o
musl += fenv/fesetexceptflag.o
diff --git a/libc/pthread.cc b/libc/pthread.cc
index de5979e8..76e6d982 100644
--- a/libc/pthread.cc
+++ b/libc/pthread.cc
@@ -960,28 +960,6 @@ void pthread_exit(void *retval)
t->_thread->exit();
}

-// Following 4 functions provide minimal implementation
-// that ONLY covers default Linux SCHED_OTHER policy
-int sched_get_priority_min(int policy)
-{
- switch (policy) {
- case SCHED_OTHER:
- return 0;
- default:
- return EINVAL;
- }
-}
-
-int sched_get_priority_max(int policy)
-{
- switch (policy) {
- case SCHED_OTHER:
- return 0;
- default:
- return EINVAL;
- }
-}
-
int pthread_setschedparam(pthread_t thread, int policy,
const struct sched_param *param)
{
diff --git a/libc/sched.cc b/libc/sched.cc
new file mode 100644
index 00000000..d363ecd6
--- /dev/null
+++ b/libc/sched.cc
@@ -0,0 +1,160 @@
+/*
+ * Copyright (C) 2014 Huawei Technologies Duesseldorf GmbH
+ *
+ * This work is open source software, licensed under the terms of the
+ * BSD license as described in the LICENSE file in the top-level directory.
+ */
+
+#include <errno.h>
+#include <api/sched.h>
+#include <api/sys/resource.h>
+#include <osv/sched.hh>
+
+#include <osv/stubbing.hh>
+
+#define SCHED_PRIO_MIN 1
+#define SCHED_PRIO_MAX 99
+
+/* sched_rr_get_interval writes the time quantum for SCHED_RR
+ * processes to the tp timespec structure.
+ * Since the SCHED_RR is currently not implemented, we don't
+ * implement this function yet.
+ */
+
+int sched_rr_get_interval(pid_t pid, struct timespec * tp)
+{
+ WARN_STUBBED();
+ errno = ENOSYS;
+ return -1;
+}
+
+static int sched_get_priority_minmax(int policy, int value)
+{
+ switch (policy) {
+ case SCHED_OTHER:
+ return 0;
+ case SCHED_FIFO:
+ return value;
+ default:
+ errno = EINVAL;
+ /* error return value unfortunately overlaps with allowed
+ * POSIX API allowed range. Another good reason to have
+ * only rt priorities > 0.
+ */
+ return -1;
+ }
+}
+
+int sched_get_priority_min(int policy)
+{
+ return sched_get_priority_minmax(policy, SCHED_PRIO_MIN);
+}
+
+int sched_get_priority_max(int policy)
+{
+ return sched_get_priority_minmax(policy, SCHED_PRIO_MAX);
+}
+
+static int sched_setparam_aux(sched::thread *t, int policy, int prio)
+{
+ switch (policy) {
+ case SCHED_OTHER:
+ if (prio != 0) {
+ errno = EINVAL;
+ return -1;
+ }
+ break;
+ case SCHED_FIFO:
+ if (prio < SCHED_PRIO_MIN || prio > SCHED_PRIO_MAX) {
+ errno = EINVAL;
+ return -1;
+ }
+ break;
+ default:
+ errno = EINVAL;
+ return -1;
+ }
+
+ t->set_realtime_priority(prio);
+ return 0;
+}
+
+int sched_setscheduler(pid_t pid, int policy,
+ const struct sched_param *param)
+{
+ int res = -1;
+
+ if (!param) {
+ errno = EINVAL;
+ return -1;
+ }
+
+ if (pid == 0) {
+ sched::thread *t = sched::thread::current();
+
+ res = sched_setparam_aux(t, policy, param->sched_priority);
+ } else {
+ sched::with_thread_by_id(pid, [param, policy, &res] (sched::thread *t) {
+ if (t) res = sched_setparam_aux(t, policy, param->sched_priority);
+ });
+ if (res == -1) {
+ errno = ESRCH;
+ }
+ }
+
+ return res;
+}
+
+int sched_getscheduler(pid_t pid)
+{
+ int res = -1;
+
+ if (pid == 0) {
+ sched::thread *t = sched::thread::current();
+
+ if (t->realtime_priority() == 0) {
+ res = SCHED_OTHER;
+ } else {
+ res = SCHED_FIFO;
+ };
+ } else {
+ sched::with_thread_by_id(pid, [&res] (sched::thread *t) {
+ if (t) {
+ if (t->realtime_priority() == 0) {
+ res = SCHED_OTHER;
+ } else {
+ res = SCHED_FIFO;
+ }
+ }
+ });
+ if (res == -1) {
+ errno = ESRCH;
+ }
+ }
+
+ return res;
+}
+
+int sched_getparam(pid_t pid, struct sched_param *param)
+{
+ int res = 0;
+
+ if (pid == 0) {
+ sched::thread *t = sched::thread::current();
+
+ param->sched_priority = t->realtime_priority();
+ } else {
+ sched::with_thread_by_id(pid, [param, &res] (sched::thread *t) {
+ if (t) {
+ param->sched_priority = t->realtime_priority();
+ } else {
+ res = -1;
+ }
+ });
+ if (res == -1) {
+ errno = ESRCH;
+ }
+ }
+
+ return res;
+}
--
2.30.2

luca abeni

unread,
Jun 5, 2023, 8:19:38 AM6/5/23
to osv...@googlegroups.com, Claudio Fontana
From: Claudio Fontana <claudio...@huawei.com>

we only support CLOCK_MONOTONIC, and we don't support remainder
due to the signal implementation.

Signed-off-by: Claudio Fontana <claudio...@huawei.com>
---
include/osv/sched.hh | 10 ++++++++++
libc/time.cc | 32 +++++++++++++++++++++++++++++++-
2 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/include/osv/sched.hh b/include/osv/sched.hh
index c1f414ba..36e45645 100644
--- a/include/osv/sched.hh
+++ b/include/osv/sched.hh
@@ -513,6 +513,8 @@ public:
inline void wake_with_from_mutex(Action action);
template <class Rep, class Period>
static void sleep(std::chrono::duration<Rep, Period> duration);
+ template <class Clock, class Duration>
+ static void sleep_until(std::chrono::time_point<Clock, Duration> time_point);
/**
* Let the other thread on the current CPU run if there is any.
*
@@ -1407,6 +1409,14 @@ void thread::sleep(std::chrono::duration<Rep, Period> duration)
sleep_impl(t);
}

+template <class Clock, class Duration>
+void thread::sleep_until(std::chrono::time_point<Clock, Duration> time_point)
+{
+ timer t(*current());
+ t.set(time_point);
+ wait_until([&] { return t.expired(); });
+}
+
template <class Action>
inline
void thread::wake_with_irq_or_preemption_disabled(Action action)
diff --git a/libc/time.cc b/libc/time.cc
index 2660eb86..432128d3 100644
--- a/libc/time.cc
+++ b/libc/time.cc
@@ -15,7 +15,7 @@
#include <osv/sched.hh>
#include "pthread.hh"

-u64 convert(const timespec& ts)
+static u64 convert(const timespec& ts)
{
return ts.tv_sec * 1000000000 + ts.tv_nsec;
}
@@ -36,6 +36,9 @@ int gettimeofday(struct timeval* tv, struct timezone* tz)
OSV_LIBC_API
int nanosleep(const struct timespec* req, struct timespec* rem)
{
+ if (!req || req->tv_nsec < 0 || req->tv_nsec >= 1000000000L || req->tv_sec < 0)
+ return libc_error(EINVAL);
+
sched::thread::sleep(std::chrono::nanoseconds(convert(*req)));
return 0;
}
@@ -127,3 +130,30 @@ clock_t clock(void)
clock_gettime(CLOCK_PROCESS_CPUTIME_ID, &ts);
return ts.tv_sec * 1000000000L + ts.tv_nsec;
}
+
+OSV_LIBC_API
+int clock_nanosleep(clockid_t clock_id, int flags,
+ const struct timespec *request,
+ struct timespec *remain)
+{
+ /* XXX we are implementing really only CLOCK_MONOTONIC, */
+ /* and we don't support remain, due to signals. */
+ if (remain) {
+ UNIMPLEMENTED("clock_nanosleep(): remain not supported, due to signals");
+ }
+ if (clock_id != CLOCK_MONOTONIC) {
+ UNIMPLEMENTED("clock_nanosleep(): only CLOCK_MONOTONIC is supported");
+ }
+
+ switch (flags) {
+ case 0:
+ return nanosleep(request, NULL);
+ case TIMER_ABSTIME: {
+ sched::thread::sleep_until(osv::clock::uptime::time_point(
+ osv::clock::uptime::duration(convert(*request))));
+ return 0;
+ }
+ default:
+ return libc_error(EINVAL);
+ }
+}
--
2.30.2

Reply all
Reply to author
Forward
0 new messages