|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Minios-devel] [UNIKRAFT PATCH 3/8] lib/uksched: Introduce thread attributes
Hi Costin,
Please see a few comments inline.
On 9/18/18, 5:27 PM, "Costin Lupu" <costin.lupu@xxxxxxxxx> wrote:
>We introduce thread attributes abstraction which is used for
>configuring thread behaviors. The set of currently available
>attributes was inspired from the pthread interface.
s/thread attributes/a thread attributes
s/thread behaviors/thread behavior.
>Thread attributes can be set either when creating the threads or
>during threads execution by calling the corresponding setters. If
>an attribute is not supported by the underlying scheduler then
>-EINVAL is returned by setters.
s/or during threads/or during thread
>The detach attribute is common to all threads, regardless the
>scheduling policy, and if set it means that the thread resources
>will be automatically freed on thread exit. By default, threads
>are not detached. The complete detach support will be added in a
>following commit.
s/regardless/regardless of
>Time slice and priority make sense only for preemptive schedulers.
>
>Signed-off-by: Costin Lupu <costin.lupu@xxxxxxxxx>
>---
> lib/uksched/Makefile.uk | 1 +
> lib/uksched/exportsyms.uk | 12 +++
> lib/uksched/include/uk/sched.h | 75 +++++++++++++++++--
> lib/uksched/include/uk/thread.h | 12 ++-
> lib/uksched/include/uk/thread_attr.h | 73 +++++++++++++++++++
> lib/uksched/sched.c | 5 +-
> lib/uksched/thread.c | 34 +++++++++
> lib/uksched/thread_attr.c | 137
>+++++++++++++++++++++++++++++++++++
> lib/ukschedcoop/schedcoop.c | 6 +-
> 9 files changed, 345 insertions(+), 10 deletions(-)
> create mode 100644 lib/uksched/include/uk/thread_attr.h
> create mode 100644 lib/uksched/thread_attr.c
>
>diff --git a/lib/uksched/Makefile.uk b/lib/uksched/Makefile.uk
>index 24816f6..229d847 100644
>--- a/lib/uksched/Makefile.uk
>+++ b/lib/uksched/Makefile.uk
>@@ -5,3 +5,4 @@ CXXINCLUDES-$(CONFIG_LIBUKSCHED) +=
>-I$(LIBUKSCHED_BASE)/include
>
> LIBUKSCHED_SRCS-y += $(LIBUKSCHED_BASE)/sched.c
> LIBUKSCHED_SRCS-y += $(LIBUKSCHED_BASE)/thread.c
>+LIBUKSCHED_SRCS-y += $(LIBUKSCHED_BASE)/thread_attr.c
>diff --git a/lib/uksched/exportsyms.uk b/lib/uksched/exportsyms.uk
>index 45a9694..7d8e102 100644
>--- a/lib/uksched/exportsyms.uk
>+++ b/lib/uksched/exportsyms.uk
>@@ -14,3 +14,15 @@ uk_thread_fini
> uk_thread_block_timeout
> uk_thread_block
> uk_thread_wake
>+uk_thread_set_prio
>+uk_thread_get_prio
>+uk_thread_set_timeslice
>+uk_thread_get_timeslice
>+uk_thread_attr_init
>+uk_thread_attr_fini
>+uk_thread_attr_set_detachstate
>+uk_thread_attr_get_detachstate
>+uk_thread_attr_set_prio
>+uk_thread_attr_get_prio
>+uk_thread_attr_set_timeslice
>+uk_thread_attr_get_timeslice
>diff --git a/lib/uksched/include/uk/sched.h
>b/lib/uksched/include/uk/sched.h
>index d2fc8df..b21d65c 100644
>--- a/lib/uksched/include/uk/sched.h
>+++ b/lib/uksched/include/uk/sched.h
>@@ -60,16 +60,31 @@ typedef void (*uk_sched_yield_func_t)
> (struct uk_sched *s);
>
> typedef void (*uk_sched_thread_add_func_t)
>- (struct uk_sched *s, struct uk_thread *t);
>+ (struct uk_sched *s, struct uk_thread *t,
>+ struct uk_thread_attr *attr);
> typedef void (*uk_sched_thread_remove_func_t)
> (struct uk_sched *s, struct uk_thread *t);
>
>+typedef int (*uk_sched_thread_set_prio_func_t)
>+ (struct uk_sched *s, struct uk_thread *t, prio_t prio);
>+typedef int (*uk_sched_thread_get_prio_func_t)
>+ (struct uk_sched *s, const struct uk_thread *t, prio_t *prio);
>+typedef int (*uk_sched_thread_set_tslice_func_t)
>+ (struct uk_sched *s, struct uk_thread *t, int tslice);
>+typedef int (*uk_sched_thread_get_tslice_func_t)
>+ (struct uk_sched *s, const struct uk_thread *t, int *tslice);
>+
> struct uk_sched {
> uk_sched_yield_func_t yield;
>
> uk_sched_thread_add_func_t thread_add;
> uk_sched_thread_remove_func_t thread_remove;
>
>+ uk_sched_thread_set_prio_func_t thread_set_prio;
>+ uk_sched_thread_get_prio_func_t thread_get_prio;
>+ uk_sched_thread_set_tslice_func_t thread_set_tslice;
>+ uk_sched_thread_get_tslice_func_t thread_get_tslice;
>+
> /* internal */
> struct uk_thread idle;
> struct ukplat_ctx_callbacks plat_ctx_cbs;
>@@ -92,12 +107,12 @@ static inline void uk_sched_yield(void)
> }
>
> static inline void uk_sched_thread_add(struct uk_sched *s,
>- struct uk_thread *t)
>+ struct uk_thread *t, struct uk_thread_attr *attr)
> {
> UK_ASSERT(s);
> UK_ASSERT(t);
> t->sched = s;
>- s->thread_add(s, t);
>+ s->thread_add(s, t, attr);
> }
>
> static inline void uk_sched_thread_remove(struct uk_sched *s,
>@@ -109,6 +124,49 @@ static inline void uk_sched_thread_remove(struct
>uk_sched *s,
> t->sched = NULL;
> }
>
>+static inline int uk_sched_thread_set_prio(struct uk_sched *s,
>+ struct uk_thread *t, prio_t prio)
>+{
>+ UK_ASSERT(s);
>+
>+ if (!s->thread_set_prio)
>+ return -EINVAL;
>+
>+ return s->thread_set_prio(s, t, prio);
>+}
>+
>+static inline int uk_sched_thread_get_prio(struct uk_sched *s,
>+ const struct uk_thread *t, prio_t *prio)
>+{
>+ UK_ASSERT(s);
>+
>+ if (!s->thread_get_prio)
>+ return -EINVAL;
>+
>+ return s->thread_get_prio(s, t, prio);
>+}
>+
>+static inline int uk_sched_thread_set_timeslice(struct uk_sched *s,
>+ struct uk_thread *t, int tslice)
>+{
>+ UK_ASSERT(s);
>+
>+ if (!s->thread_set_tslice)
>+ return -EINVAL;
>+
>+ return s->thread_set_tslice(s, t, tslice);
>+}
>+
>+static inline int uk_sched_thread_get_timeslice(struct uk_sched *s,
>+ const struct uk_thread *t, int *tslice)
>+{
>+ UK_ASSERT(s);
>+
>+ if (!s->thread_get_tslice)
>+ return -EINVAL;
>+
>+ return s->thread_get_tslice(s, t, tslice);
>+}
>
> /*
> * Internal scheduler functions
>@@ -132,11 +190,17 @@ static inline struct uk_thread
>*uk_sched_get_idle(struct uk_sched *s)
> void uk_sched_start(struct uk_sched *sched) __noreturn;
>
> #define uk_sched_init(s, yield_func, \
>- thread_add_func, thread_remove_func) \
>+ thread_add_func, thread_remove_func, \
>+ thread_set_prio_func, thread_get_prio_func, \
>+ thread_set_tslice_func, thread_get_tslice_func) \
> do { \
> (s)->yield = yield_func; \
> (s)->thread_add = thread_add_func; \
> (s)->thread_remove = thread_remove_func; \
>+ (s)->thread_set_prio = thread_set_prio_func; \
>+ (s)->thread_get_prio = thread_get_prio_func; \
>+ (s)->thread_set_tslice = thread_set_tslice_func; \
>+ (s)->thread_get_tslice = thread_get_tslice_func; \
> uk_sched_register((s)); \
> } while (0)
>
>@@ -146,7 +210,8 @@ void uk_sched_start(struct uk_sched *sched)
>__noreturn;
> */
>
> struct uk_thread *uk_sched_thread_create(struct uk_sched *sched,
>- const char *name, void (*function)(void *), void *arg);
>+ const char *name, struct uk_thread_attr *attr,
>+ void (*function)(void *), void *arg);
> void uk_sched_thread_destroy(struct uk_sched *sched,
> struct uk_thread *thread);
>
>diff --git a/lib/uksched/include/uk/thread.h
>b/lib/uksched/include/uk/thread.h
>index e66a05b..5ff2dd6 100644
>--- a/lib/uksched/include/uk/thread.h
>+++ b/lib/uksched/include/uk/thread.h
>@@ -35,6 +35,7 @@
> #include <uk/arch/lcpu.h>
> #include <uk/arch/time.h>
> #include <uk/plat/thread.h>
>+#include <uk/thread_attr.h>
> #include <uk/list.h>
> #include <uk/essentials.h>
>
>@@ -59,11 +60,20 @@ struct uk_thread {
>
> UK_TAILQ_HEAD(uk_thread_list, struct uk_thread);
>
>+#define uk_thread_create_attr(name, attr, function, data) \
>+ uk_sched_thread_create(uk_sched_get_default(), \
>+ name, attr, function, data)
> #define uk_thread_create(name, function, data) \
>- uk_sched_thread_create(uk_sched_get_default(), name, function, data)
>+ uk_thread_create_attr(name, NULL, function, data)
> #define uk_thread_destroy(thread) \
> uk_sched_thread_destroy(thread->sched, thread)
>
>+int uk_thread_set_prio(struct uk_thread *thread, prio_t prio);
>+int uk_thread_get_prio(const struct uk_thread *thread, prio_t *prio);
>+
>+int uk_thread_set_timeslice(struct uk_thread *thread, int timeslice);
>+int uk_thread_get_timeslice(const struct uk_thread *thread, int
>*timeslice);
>+
> static inline
> struct uk_thread *uk_thread_current(void)
> {
>diff --git a/lib/uksched/include/uk/thread_attr.h
>b/lib/uksched/include/uk/thread_attr.h
>new file mode 100644
>index 0000000..30be708
>--- /dev/null
>+++ b/lib/uksched/include/uk/thread_attr.h
>@@ -0,0 +1,73 @@
>+/* SPDX-License-Identifier: BSD-3-Clause */
>+/*
>+ * Authors: Costin Lupu <costin.lupu@xxxxxxxxx>
>+ *
>+ * Copyright (c) 2018, NEC Europe Ltd., NEC Corporation. All rights
>reserved.
>+ *
>+ * Redistribution and use in source and binary forms, with or without
>+ * modification, are permitted provided that the following conditions
>+ * are met:
>+ *
>+ * 1. Redistributions of source code must retain the above copyright
>+ * notice, this list of conditions and the following disclaimer.
>+ * 2. Redistributions in binary form must reproduce the above copyright
>+ * notice, this list of conditions and the following disclaimer in the
>+ * documentation and/or other materials provided with the
>distribution.
>+ * 3. Neither the name of the copyright holder nor the names of its
>+ * contributors may be used to endorse or promote products derived
>from
>+ * this software without specific prior written permission.
>+ *
>+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
>"AS IS"
>+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
>THE
>+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
>PURPOSE
>+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR
>CONTRIBUTORS BE
>+ * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
>+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
>+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
>BUSINESS
>+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER
>IN
>+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR
>OTHERWISE)
>+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED
>OF THE
>+ * POSSIBILITY OF SUCH DAMAGE.
>+ *
>+ * THIS HEADER MAY NOT BE EXTRACTED OR MODIFIED IN ANY WAY.
>+ */
>+
>+#ifndef __UK_SCHED_THREAD_ATTR_H__
>+#define __UK_SCHED_THREAD_ATTR_H__
>+
>+#ifdef __cplusplus
>+extern "C" {
>+#endif
>+
>+#define UK_THREAD_ATTR_WAITABLE 0
>+#define UK_THREAD_ATTR_DETACHED 1
>+
>+#define UK_THREAD_ATTR_PRIO_MIN 0
>+#define UK_THREAD_ATTR_PRIO_MAX 255
>+#define UK_THREAD_ATTR_PRIO_DEFAULT 127
>+
>+typedef int prio_t;
>+
>+typedef struct uk_thread_attr {
>+ int is_detached;
>+ prio_t prio;
>+ int timeslice;
>+} uk_thread_attr_t;
>+
>+int uk_thread_attr_init(uk_thread_attr_t *attr);
>+int uk_thread_attr_fini(uk_thread_attr_t *attr);
>+
>+int uk_thread_attr_set_detachstate(uk_thread_attr_t *attr, int state);
>+int uk_thread_attr_get_detachstate(const uk_thread_attr_t *attr, int
>*state);
>+
>+int uk_thread_attr_set_prio(uk_thread_attr_t *attr, prio_t prio);
>+int uk_thread_attr_get_prio(const uk_thread_attr_t *attr, prio_t *prio);
>+
>+int uk_thread_attr_set_timeslice(uk_thread_attr_t *attr, int timeslice);
>+int uk_thread_attr_get_timeslice(const uk_thread_attr_t *attr, int
>*timeslice);
>+
>+#ifdef __cplusplus
>+}
>+#endif
>+
>+#endif /* __UK_SCHED_THREAD_ATTR_H__ */
>diff --git a/lib/uksched/sched.c b/lib/uksched/sched.c
>index 8276e15..f2eca33 100644
>--- a/lib/uksched/sched.c
>+++ b/lib/uksched/sched.c
>@@ -170,7 +170,8 @@ void uk_sched_idle_init(struct uk_sched *sched,
> }
>
> struct uk_thread *uk_sched_thread_create(struct uk_sched *sched,
>- const char *name, void (*function)(void *), void *arg)
>+ const char *name, struct uk_thread_attr *attr,
>+ void (*function)(void *), void *arg)
> {
> struct uk_thread *thread = NULL;
> void *stack = NULL;
>@@ -195,7 +196,7 @@ struct uk_thread *uk_sched_thread_create(struct
>uk_sched *sched,
> if (rc)
> goto err;
>
>- uk_sched_thread_add(sched, thread);
>+ uk_sched_thread_add(sched, thread, attr);
>
> return thread;
>
>diff --git a/lib/uksched/thread.c b/lib/uksched/thread.c
>index 4b00b1a..a6d90fc 100644
>--- a/lib/uksched/thread.c
>+++ b/lib/uksched/thread.c
>@@ -30,9 +30,11 @@
> * Ported from Mini-OS
> */
> #include <stdlib.h>
>+#include <errno.h>
> #include <uk/plat/config.h>
> #include <uk/plat/time.h>
> #include <uk/thread.h>
>+#include <uk/sched.h>
> #include <uk/print.h>
> #include <uk/assert.h>
>
>@@ -127,3 +129,35 @@ void uk_thread_wake(struct uk_thread *thread)
> thread->wakeup_time = 0LL;
> set_runnable(thread);
> }
>+
>+int uk_thread_set_prio(struct uk_thread *thread, prio_t prio)
>+{
>+ if (!thread)
>+ return -EINVAL;
>+
>+ return uk_sched_thread_set_prio(thread->sched, thread, prio);
>+}
>+
>+int uk_thread_get_prio(const struct uk_thread *thread, prio_t *prio)
>+{
>+ if (!thread)
>+ return -EINVAL;
>+
>+ return uk_sched_thread_get_prio(thread->sched, thread, prio);
>+}
>+
>+int uk_thread_set_timeslice(struct uk_thread *thread, int timeslice)
>+{
>+ if (!thread)
>+ return -EINVAL;
>+
>+ return uk_sched_thread_set_timeslice(thread->sched, thread, timeslice);
>+}
>+
>+int uk_thread_get_timeslice(const struct uk_thread *thread, int
>*timeslice)
>+{
>+ if (!thread)
>+ return -EINVAL;
>+
>+ return uk_sched_thread_get_timeslice(thread->sched, thread, timeslice);
>+}
>diff --git a/lib/uksched/thread_attr.c b/lib/uksched/thread_attr.c
>new file mode 100644
>index 0000000..cff1a55
>--- /dev/null
>+++ b/lib/uksched/thread_attr.c
>@@ -0,0 +1,137 @@
>+/* SPDX-License-Identifier: BSD-3-Clause */
>+/*
>+ * Authors: Costin Lupu <costin.lupu@xxxxxxxxx>
>+ *
>+ * Copyright (c) 2018, NEC Europe Ltd., NEC Corporation. All rights
>reserved.
>+ *
>+ * Redistribution and use in source and binary forms, with or without
>+ * modification, are permitted provided that the following conditions
>+ * are met:
>+ *
>+ * 1. Redistributions of source code must retain the above copyright
>+ * notice, this list of conditions and the following disclaimer.
>+ * 2. Redistributions in binary form must reproduce the above copyright
>+ * notice, this list of conditions and the following disclaimer in the
>+ * documentation and/or other materials provided with the
>distribution.
>+ * 3. Neither the name of the copyright holder nor the names of its
>+ * contributors may be used to endorse or promote products derived
>from
>+ * this software without specific prior written permission.
>+ *
>+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
>"AS IS"
>+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
>THE
>+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
>PURPOSE
>+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR
>CONTRIBUTORS BE
>+ * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
>+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
>+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
>BUSINESS
>+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER
>IN
>+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR
>OTHERWISE)
>+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED
>OF THE
>+ * POSSIBILITY OF SUCH DAMAGE.
>+ *
>+ * THIS HEADER MAY NOT BE EXTRACTED OR MODIFIED IN ANY WAY.
>+ */
>+
>+#include <stdlib.h>
>+#include <errno.h>
>+#include <uk/plat/time.h>
>+#include <uk/thread_attr.h>
>+#include <uk/assert.h>
>+
>+
>+int uk_thread_attr_init(uk_thread_attr_t *attr)
>+{
>+ if (attr == NULL)
>+ return EINVAL;
>+
>+ attr->is_detached = 0;
>+ attr->prio = -1;
>+ attr->timeslice = 0;
>+
>+ return 0;
>+}
Minor: perhaps define some macros for the 0s and -1s you have here. Also,
return -EINVAL instead of EINVAL.
Same for all uk_thread_attr_* functions below.
>+
>+int uk_thread_attr_fini(uk_thread_attr_t *attr)
>+{
>+ if (attr == NULL)
>+ return EINVAL;
>+
>+ return 0;
>+}
>+
>+int uk_thread_attr_set_detachstate(uk_thread_attr_t *attr, int state)
>+{
>+ if (attr == NULL)
>+ return EINVAL;
>+
>+ if (state == UK_THREAD_ATTR_DETACHED)
>+ attr->is_detached = 1;
>+
>+ else if (state == UK_THREAD_ATTR_WAITABLE)
>+ attr->is_detached = 0;
>+
>+ else
>+ return EINVAL;
>+
>+ return 0;
>+}
Shouldn’t you be setting attr’s members to macros, i.e., attr->is_detached
= UK_THREAD_ATTR_DETACHED ?
>+
>+int uk_thread_attr_get_detachstate(const uk_thread_attr_t *attr, int
>*state)
>+{
>+ if (attr == NULL || state == NULL)
>+ return EINVAL;
>+
>+ if (attr->is_detached)
>+ *state = UK_THREAD_ATTR_DETACHED;
>+ else
>+ *state = UK_THREAD_ATTR_WAITABLE;
>+
>+ return 0;
>+}
>+
>+int uk_thread_attr_set_prio(uk_thread_attr_t *attr, prio_t prio)
>+{
>+ int rc = EINVAL;
>+
>+ if (attr == NULL)
>+ return rc;
>+
>+ if (prio >= UK_THREAD_ATTR_PRIO_MIN &&
>+ prio <= UK_THREAD_ATTR_PRIO_MAX) {
>+ attr->prio = prio;
>+ rc = 0;
>+ }
>+
>+ return rc;
>+}
>+
>+int uk_thread_attr_get_prio(const uk_thread_attr_t *attr, prio_t *prio)
>+{
>+ if (attr == NULL || prio == NULL)
>+ return EINVAL;
>+
>+ *prio = attr->prio;
>+
>+ return 0;
>+}
>+
>+int uk_thread_attr_set_timeslice(uk_thread_attr_t *attr, int timeslice)
>+{
>+ if (attr == NULL)
>+ return EINVAL;
>+
>+ /* TODO check timeslice agains platform tick */
>+ attr->timeslice = timeslice;
>+
>+ return 0;
>+}
s/agains/against. Also, when will this be implemented?
Thanks,
— Felipe
>+
>+int uk_thread_attr_get_timeslice(const uk_thread_attr_t *attr, int
>*timeslice)
>+{
>+ if (attr == NULL || timeslice == NULL)
>+ return EINVAL;
>+
>+ *timeslice = attr->timeslice;
>+
>+ return 0;
>+}
>diff --git a/lib/ukschedcoop/schedcoop.c b/lib/ukschedcoop/schedcoop.c
>index d78ca54..bc02d08 100644
>--- a/lib/ukschedcoop/schedcoop.c
>+++ b/lib/ukschedcoop/schedcoop.c
>@@ -132,7 +132,8 @@ static void schedcoop_schedule(struct uk_sched *s)
> }
> }
>
>-static void schedcoop_thread_add(struct uk_sched *s, struct uk_thread *t)
>+static void schedcoop_thread_add(struct uk_sched *s, struct uk_thread *t,
>+ struct uk_thread_attr *attr __unused)
> {
> unsigned long flags;
> struct schedcoop_private *prv = s->prv;
>@@ -210,7 +211,8 @@ struct uk_sched *uk_schedcoop_init(struct uk_alloc *a)
> uk_sched_init(sched,
> schedcoop_yield,
> schedcoop_thread_add,
>- schedcoop_thread_remove);
>+ schedcoop_thread_remove,
>+ NULL, NULL, NULL, NULL);
>
> return sched;
> }
>--
>2.11.0
>
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |