[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Minios-devel] [UNIKRAFT PATCH 13/17] lib/uksched: Updates for inlining with plat/common changes



Can you add some brief comments to sched.h and thread.h that explain what each function is for and which return values are being expected? We do not have a style defined for this yet. I used a doxygen-like style for now. You can do this with a separate patch. It does not have to be part of this patch series - but please do not forget ;-).

On 27.03.2018 14:29, Costin Lupu wrote:
When creating a new thread, the scheduler allocates the stack, sets the thread
address on the stack top and pushes the thread function and argument on the
newly created stack. The context resource will be created by the platform which
will set the stack pointer as provided by the scheduler and the instruction
pointer to a value representing the start routine for the context.

Other changes:
- introducing uk_sched_default_init function
- using new context callbacks abstraction
- new uk_sched_start function replacing uk_sched_run; the start function is the
same for all schedulers
- all schedulers have an idle thread
- revisited semantics: dropping start/stop names for threads functions since we
cannot control exactly when threads start/stop runing; more suitable function
names are add/remove

Signed-off-by: Costin Lupu <costin.lupu@xxxxxxxxx>
---
  lib/uksched/include/uk/sched.h  | 120 ++++++++++++++++++++++++++-------------
  lib/uksched/include/uk/thread.h |  38 +++++++------
  lib/uksched/sched.c             | 123 +++++++++++++++++++++++++++++++---------
  lib/uksched/thread.c            |  75 +++++++++++++++++++++++-
  4 files changed, 271 insertions(+), 85 deletions(-)

diff --git a/lib/uksched/include/uk/sched.h b/lib/uksched/include/uk/sched.h
index 13492a7..bbfe442 100644
--- a/lib/uksched/include/uk/sched.h
+++ b/lib/uksched/include/uk/sched.h
@@ -42,86 +42,128 @@
  #include <uk/essentials.h>
  #include <errno.h>
+#ifdef __cplusplus
+extern "C" {
+#endif
+
  struct uk_sched;
+struct uk_sched *uk_sched_default_init(struct uk_alloc *a);
+
  extern struct uk_sched *uk_sched_head;
  int uk_sched_register(struct uk_sched *s);
  struct uk_sched *uk_sched_get_default(void);
  int uk_sched_set_default(struct uk_sched *s);
-typedef void (*uk_sched_run_func_t)
-               (struct uk_sched *s) __noreturn;
-typedef void  (*uk_sched_schedule_func_t)
+
+typedef void  (*uk_sched_yield_func_t)
                (struct uk_sched *s);
-typedef void (*uk_sched_thread_start_func_t)
+typedef void  (*uk_sched_thread_add_func_t)
                (struct uk_sched *s, struct uk_thread *t);
-typedef void  (*uk_sched_thread_stop_func_t)
+typedef void  (*uk_sched_thread_remove_func_t)
                (struct uk_sched *s, struct uk_thread *t);
struct uk_sched {
-       uk_sched_schedule_func_t schedule;
+       uk_sched_yield_func_t yield;
- uk_sched_run_func_t run;
-
-       uk_sched_thread_start_func_t thread_start;
-       uk_sched_thread_stop_func_t  thread_stop;
+       uk_sched_thread_add_func_t      thread_add;
+       uk_sched_thread_remove_func_t   thread_remove;
/* internal */
+       struct uk_thread idle;
+       struct ukplat_ctx_callbacks plat_ctx_cbs;
        struct uk_alloc *allocator;
        struct uk_sched *next;
-       void *private;
+       void *prv;
  };
-/* wrapper functions */
-
-static inline void uk_sched_schedule(struct uk_sched *s)
-{
-       UK_ASSERT(s);
-       s->schedule(s);
-}
-
+/* wrapper functions over scheduler callbacks */
  static inline void uk_sched_yield(void)
  {
-       uk_sched_schedule(uk_sched_get_default());
+       struct uk_sched *s;
+       struct uk_thread *current = uk_thread_current();
+
+       UK_ASSERT(current);
+
+       s = current->sched;
+       UK_ASSERT(s);
+       s->yield(s);
  }
-static inline void uk_sched_run(struct uk_sched *s) __noreturn;
-static inline void uk_sched_run(struct uk_sched *s)
+static inline void uk_sched_thread_add(struct uk_sched *s,
+               struct uk_thread *t)
  {
        UK_ASSERT(s);
-       s->run(s);
+       UK_ASSERT(t);
+       t->sched = s;
+       s->thread_add(s, t);
  }
-static inline void uk_sched_thread_start(struct uk_sched *s,
+static inline void uk_sched_thread_remove(struct uk_sched *s,
                struct uk_thread *t)
  {
        UK_ASSERT(s);
-       s->thread_start(s, t);
+       UK_ASSERT(t);
+       s->thread_remove(s, t);
+       t->sched = NULL;
  }
-static inline void uk_sched_thread_stop(struct uk_sched *s,
-               struct uk_thread *t)
+
+/*
+ * Internal scheduler functions
+ */
+
+void uk_sched_idle_init(struct uk_sched *sched,
+               void *stack, void (*function)(void *));
+
+static inline struct uk_thread *uk_sched_get_idle(struct uk_sched *s)
  {
        UK_ASSERT(s);
-       s->thread_stop(s, t);
+       return &s->idle;
  }
-struct uk_thread *uk_sched_thread_create(struct uk_sched *sched,
-               char *name, void (*function)(void *), void *data);
-void uk_sched_thread_destroy(struct uk_sched *sched,
-               struct uk_thread *thread);
+/*
+ * Public scheduler functions
+ */
-void uk_sched_sleep(uint32_t millis);
+void uk_sched_start(struct uk_sched *sched) __noreturn;
-#define uk_sched_init(s, sched_func, run_func, \
-                       start_thread_func, stop_thread_func) \
+#define uk_sched_init(s, yield_func, \
+               thread_add_func, thread_remove_func) \
        do { \
-               (s)->schedule      = sched_func; \
-               (s)->run           = run_func; \
-               (s)->thread_start  = start_thread_func; \
-               (s)->thread_stop   = stop_thread_func; \
+               (s)->yield           = yield_func; \
+               (s)->thread_add      = thread_add_func; \
+               (s)->thread_remove   = thread_remove_func; \
                uk_sched_register((s)); \
        } while (0)
+
+/*
+ * Internal thread scheduling functions
+ */
+
+struct uk_thread *uk_sched_thread_create(struct uk_sched *sched,
+               const char *name, void (*function)(void *), void *arg);
+void uk_sched_thread_destroy(struct uk_sched *sched,
+               struct uk_thread *thread);
+
+static inline
+void uk_sched_thread_switch(struct uk_sched *sched,
+               struct uk_thread *prev, struct uk_thread *next)
+{
+       ukplat_thread_ctx_switch(&sched->plat_ctx_cbs, prev->ctx, next->ctx);
+}
+
+/*
+ * Public thread scheduling functions
+ */
+
+void uk_sched_thread_sleep(__nsec nsec);
+void uk_sched_thread_exit(void) __noreturn;
+
+#ifdef __cplusplus
+}
+#endif
+
  #endif /* __UK_SCHED_H__ */
diff --git a/lib/uksched/include/uk/thread.h b/lib/uksched/include/uk/thread.h
index 7ab8b31..2d40245 100644
--- a/lib/uksched/include/uk/thread.h
+++ b/lib/uksched/include/uk/thread.h
@@ -32,18 +32,22 @@
  #ifdef HAVE_LIBC
  #include <sys/reent.h>
  #endif
-#include <uk/arch/thread.h>
+#include <uk/arch/lcpu.h>
  #include <uk/arch/time.h>
  #include <uk/plat/thread.h>
  #include <uk/list.h>
  #include <uk/essentials.h>
+#ifdef __cplusplus
+extern "C" {
+#endif
+
  struct uk_sched;
struct uk_thread {
-       char *name;
-       char *stack;
-       struct ukplat_thread_ctx plat_ctx;
+       const char *name;
+       void *stack;
+       void *ctx;
        UK_TAILQ_ENTRY(struct uk_thread) thread_list;
        uint32_t flags;
        __snsec wakeup_time;
@@ -59,23 +63,16 @@ UK_TAILQ_HEAD(uk_thread_list, struct uk_thread);
        uk_sched_thread_create(uk_sched_get_default(), name, function, data)
  #define uk_thread_destroy(thread) \
        uk_sched_thread_destroy(thread->sched, thread)
-#define uk_thread_start(thread) \
-       uk_sched_thread_start(thread->sched, thread)
-#define uk_thread_stop(thread) \
-       uk_sched_thread_stop(thread->sched, thread)
static inline
  struct uk_thread *uk_thread_current(void)
  {

What happens when we call this function from the interrupt context or from the boot stack, or from a CPU where we did not initialize any scheduling? Should a protection be added here, e.g., with an assertion?

-       struct ukplat_thread_ctx *ctx = ukplat_thread_ctx_current();
+       struct uk_thread **current;
+       unsigned long sp = ukarch_read_sp();
- return __containerof(ctx, struct uk_thread, plat_ctx);
-}
+       current = (struct uk_thread **) (sp & ~(__STACK_SIZE - 1));
-static inline
-void uk_thread_switch(struct uk_thread *prev, struct uk_thread *next)
-{
-       ukplat_thread_ctx_switch(&prev->plat_ctx, &next->plat_ctx);
+       return *current;
  }
#define RUNNABLE_FLAG 0x00000001
@@ -84,8 +81,17 @@ void uk_thread_switch(struct uk_thread *prev, struct 
uk_thread *next)
  #define set_runnable(_thread)   ((_thread)->flags |=  RUNNABLE_FLAG)
  #define clear_runnable(_thread) ((_thread)->flags &= ~RUNNABLE_FLAG)
-void uk_thread_block_millis(struct uk_thread *thread, uint32_t millis);
+int uk_thread_init(struct uk_thread *thread,
+               struct ukplat_ctx_callbacks *cbs, struct uk_alloc *allocator,
+               const char *name, void *stack, void (*function)(void *), void 
*arg);
+void uk_thread_fini(struct uk_thread *thread,
+               struct uk_alloc *allocator);
+void uk_thread_block_timeout(struct uk_thread *thread, __nsec nsec);
  void uk_thread_block(struct uk_thread *thread);
  void uk_thread_wake(struct uk_thread *thread);
+#ifdef __cplusplus
+}
+#endif
+
  #endif /* __UK_THREAD_H__ */
diff --git a/lib/uksched/sched.c b/lib/uksched/sched.c
index 3dfa2a4..5812583 100644
--- a/lib/uksched/sched.c
+++ b/lib/uksched/sched.c
@@ -34,11 +34,26 @@
#include <stdlib.h>
  #include <uk/plat/config.h>
+#include <uk/plat/thread.h>
  #include <uk/alloc.h>
  #include <uk/sched.h>
+#if LIBUKSCHEDCOOP
+#include <uk/schedcoop.h>
+#endif
struct uk_sched *uk_sched_head; +struct uk_sched *uk_sched_default_init(struct uk_alloc *a)
+{

Yeah, I see. A similar solution is also required for libukalloc. I will accept this but for sure we need to change this later because we want to support also external scheduler libraries being selectable as default. This might require some changes to the config system which we need to do first.
Can you add a FIXME or TODO comment?

+       struct uk_sched *s = NULL;
+
+#if LIBUKSCHEDCOOP
+       s = uk_schedcoop_init(a);
+#endif
+
+       return s;
+}
+
  int uk_sched_register(struct uk_sched *s)
  {
        struct uk_sched *this = uk_sched_head;
@@ -93,60 +108,114 @@ int uk_sched_set_default(struct uk_sched *s)
        return 0;
  }
+void uk_sched_start(struct uk_sched *sched)
+{
+       UK_ASSERT(sched != NULL);
+       ukplat_thread_ctx_start(&sched->plat_ctx_cbs, sched->idle.ctx);
+}
+
+static void *create_stack(struct uk_alloc *allocator)
+{
+       void *stack;
+
+       stack = uk_palloc(allocator, STACK_SIZE_PAGE_ORDER);
+       if (stack == NULL) {
+               uk_printd(DLVL_WARN, "Error allocating thread stack.");
+               return NULL;
+       }
+
+       return stack;
+}
+
+void uk_sched_idle_init(struct uk_sched *sched,
+               void *stack, void (*function)(void *))
+{
+       struct uk_thread *idle;
+       int rc;
+
+       UK_ASSERT(sched != NULL);
+
+       if (stack == NULL)
+               stack = create_stack(sched->allocator);
+       UK_ASSERT(stack != NULL);
+
+       idle = &sched->idle;
+
+       rc = uk_thread_init(idle,
+                       &sched->plat_ctx_cbs, sched->allocator,
+                       "Idle", stack, function, NULL);
+       if (rc)
+               UK_CRASH("Error initializing idle thread.");
+
+       idle->sched = sched;
+}
+
  struct uk_thread *uk_sched_thread_create(struct uk_sched *sched,
-               char *name, void (*function)(void *), void *data)
+               const char *name, void (*function)(void *), void *arg)
  {
-       struct uk_thread *thread;
+       struct uk_thread *thread = NULL;
+       void *stack;
+       int rc;
thread = uk_malloc(sched->allocator, sizeof(struct uk_thread));
        if (thread == NULL) {
                uk_printd(DLVL_WARN, "Error allocating memory for thread.");
-               goto out;
+               goto err;
        }
/* We can't use lazy allocation here
         * since the trap handler runs on the stack
         */
-       thread->stack = uk_palloc(sched->allocator, STACK_SIZE_PAGE_ORDER);
-       if (thread->stack == NULL) {
-               uk_printd(DLVL_WARN, "Error allocating thread stack.");
-               free(thread);
-               thread = NULL;
-               goto out;
-       }
+       stack = create_stack(sched->allocator);
+       if (stack == NULL)
+               goto err;
- thread->name = name;
-       uk_printd(DLVL_EXTRA, "Thread \"%s\": pointer: %p, stack: %p\n",
-                       name, thread, thread->stack);
+       rc = uk_thread_init(thread,
+                       &sched->plat_ctx_cbs, sched->allocator,
+                       name, stack, function, arg);
+       if (rc)
+               goto err;
- /* Not runnable, not exited, not sleeping */
-       thread->flags = 0;
-       thread->wakeup_time = 0LL;
+       uk_sched_thread_add(sched, thread);
- /* Call platform specific setup. */
-       ukplat_thread_ctx_init(&thread->plat_ctx, thread->stack,
-                              function, data);
-#ifdef HAVE_LIBC
-       //TODO _REENT_INIT_PTR(&thread->reent);
-#endif
+       return thread;
- thread->sched = sched;
+err:
+       if (stack)
+               uk_free(sched->allocator, stack);
+       if (thread)
+               uk_free(sched->allocator, thread);
-out:
-       return thread;
+       return NULL;
  }
void uk_sched_thread_destroy(struct uk_sched *sched, struct uk_thread *thread)
  {
+       UK_ASSERT(sched != NULL);
+       UK_ASSERT(thread != NULL);
+       uk_thread_fini(thread, sched->allocator);
        uk_pfree(sched->allocator, thread->stack, STACK_SIZE_PAGE_ORDER);
        uk_free(sched->allocator, thread);
  }
-void uk_sched_sleep(uint32_t millis)
+void uk_sched_thread_sleep(__nsec nsec)
  {
        struct uk_thread *thread;
thread = uk_thread_current();
-       uk_thread_block_millis(thread, millis);
+       uk_thread_block_timeout(thread, nsec);
        uk_sched_yield();
  }
+
+void uk_sched_thread_exit(void)
+{
+       struct uk_thread *thread;
+
+       thread = uk_thread_current();
+
+       uk_printd(DLVL_INFO, "Thread \"%s\" exited.\n", thread->name);
+
+       UK_ASSERT(thread->sched);
+       uk_sched_thread_remove(thread->sched, thread);
+       UK_CRASH("Error stopping thread.");
+}
diff --git a/lib/uksched/thread.c b/lib/uksched/thread.c
index 1334aaa..a98753d 100644
--- a/lib/uksched/thread.c
+++ b/lib/uksched/thread.c
@@ -29,20 +29,86 @@
   * Thread definitions
   * Ported from Mini-OS
   */
+#include <stdlib.h>
+#include <uk/plat/config.h>
  #include <uk/plat/time.h>
  #include <uk/thread.h>
+#include <uk/print.h>
+#include <uk/assert.h>
+/* Pushes the specified value onto the stack of the specified thread */
+static void stack_push(unsigned long *sp, unsigned long value)
+{
+       *sp -= sizeof(unsigned long);
+       *((unsigned long *) *sp) = value;
+}
+
+static void init_sp(unsigned long *sp, char *stack,
+               void (*function)(void *), void *data)
+{
+       *sp = (unsigned long) stack + STACK_SIZE;
+
+       /* Must ensure that (%rsp + 8) is 16-byte aligned
+        * at the start of thread_starter.
+        */
+       stack_push(sp, (unsigned long) 0);
+
+       stack_push(sp, (unsigned long) function);
+       stack_push(sp, (unsigned long) data);
+}
+
+int uk_thread_init(struct uk_thread *thread,
+               struct ukplat_ctx_callbacks *cbs, struct uk_alloc *allocator,
+               const char *name, void *stack, void (*function)(void *), void 
*arg)
+{
+       unsigned long sp;
+
+       UK_ASSERT(thread != NULL);
+       UK_ASSERT(stack != NULL);
+
+       /* Save pointer to the thread on the stack to get current thread */
+       *((unsigned long *) stack) = (unsigned long) thread;
+
+       init_sp(&sp, stack, function, arg);
+
+       /* Call platform specific setup. */
+       thread->ctx = ukplat_thread_ctx_create(cbs, allocator, sp);
+       if (thread->ctx == NULL)
+               return -1;
+
+       thread->name = name;
+       thread->stack = stack;
+
+       /* Not runnable, not exited, not sleeping */
+       thread->flags = 0;
+       thread->wakeup_time = 0LL;
+
+#ifdef HAVE_LIBC
+       //TODO _REENT_INIT_PTR(&thread->reent);
+#endif
+
+       uk_printd(DLVL_INFO, "Thread \"%s\": pointer: %p, stack: %p\n",
+                       name, thread, thread->stack);
+
+       return 0;
+}
+
+void uk_thread_fini(struct uk_thread *thread, struct uk_alloc *allocator)
+{
+       UK_ASSERT(thread != NULL);
+       ukplat_thread_ctx_destroy(allocator, thread->ctx);
+}
+
  static void uk_thread_block_until(struct uk_thread *thread, __snsec until)
  {
        thread->wakeup_time = until;
        clear_runnable(thread);
  }
-void uk_thread_block_millis(struct uk_thread *thread, uint32_t millis)
+void uk_thread_block_timeout(struct uk_thread *thread, __nsec nsec)
  {
-       __snsec until = (__snsec) ukplat_monotonic_clock() +
-                       ukarch_time_msec_to_nsec(millis);
+       __snsec until = (__snsec) ukplat_monotonic_clock() + nsec;
uk_thread_block_until(thread, until);
  }
@@ -54,6 +120,9 @@ void uk_thread_block(struct uk_thread *thread)
void uk_thread_wake(struct uk_thread *thread)
  {
+       if (is_runnable(thread))
+               return;
+
        thread->wakeup_time = 0LL;
        set_runnable(thread);
  }


_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.