Changeset e0a4686 in mainline for uspace/lib/c/generic/fibril.c

Timestamp:

2018-03-08T18:25:31Z (7 years ago)

Author:

Jiří Zárevúcky <zarevucky.jiri@…>

Branches:

lfn, master, serial, ticket/834-toolchain-update, topic/msim-upgrade, topic/simplify-dev-export

Children:

53ad43c, 615e83d

Parents:

fd57cf17

git-author:

Jiří Zárevúcky <zarevucky.jiri@…> (2018-03-06 22:03:11)

git-committer:

Jiří Zárevúcky <zarevucky.jiri@…> (2018-03-08 18:25:31)

Message:

Introduce context_swap() and use it to simplify fibril implementation.

This wraps the use of context_save()/context_restore(), to provide a more
natural (and easier to understand) control flow. The patch also hides the
details of the underlying context, requiring the use of context_get_pc()
to acquire program counter. This might allow context_swap() to only store
one pointer in the context, storing the remaining information in its stack
frame. However, that is not done in this patch.

File:

• uspace/lib/c/generic/fibril.c (modified) (5 diffs)

uspace/lib/c/generic/fibril.c

@@ -44 +44 @@
 #include <stdio.h>
 #include <libarch/barrier.h>
-#include <libarch/faddr.h>
+#include <context.h>
 #include <futex.h>
 #include <assert.h>
@@ -155 +155 @@
         futex_lock(&fibril_futex);
 
+        fibril_t *srcf = __tcb_get()->fibril_data;
+        fibril_t *dstf = NULL;
+
+        /* Choose a new fibril to run */
         switch (stype) {
+        case FIBRIL_TO_MANAGER:
+        case FIBRIL_FROM_DEAD:
+                /* Make sure the async_futex is held. */
+                assert((atomic_signed_t) async_futex.val.count <= 0);
+
+                /* If we are going to manager and none exists, create it */
+                while (list_empty(&manager_list)) {
+                        futex_unlock(&fibril_futex);
+                        async_create_manager();
+                        futex_lock(&fibril_futex);
+                }
+
+                dstf = list_get_instance(list_first(&manager_list),
+                    fibril_t, link);
+
+                if (stype == FIBRIL_FROM_DEAD)
+                        dstf->clean_after_me = srcf;
+                break;
         case FIBRIL_PREEMPT:
         case FIBRIL_FROM_MANAGER:
@@ -162 +184 @@
                         return 0;
                 }
-                break;
-        case FIBRIL_TO_MANAGER:
-        case FIBRIL_FROM_DEAD:
-                /* Make sure the async_futex is held. */
-                assert((atomic_signed_t) async_futex.val.count <= 0);
-
-                /* If we are going to manager and none exists, create it */
-                while (list_empty(&manager_list)) {
-                        futex_unlock(&fibril_futex);
-                        async_create_manager();
-                        futex_lock(&fibril_futex);
-                }
-                break;
-        }
-
-        fibril_t *srcf = __tcb_get()->fibril_data;
-        if (stype != FIBRIL_FROM_DEAD) {
-
-                /* Save current state */
-                if (!context_save(&srcf->ctx)) {
-                        if (srcf->clean_after_me) {
-                                /*
-                                 * Cleanup after the dead fibril from which we
-                                 * restored context here.
-                                 */
-                                void *stack = srcf->clean_after_me->stack;
-                                if (stack) {
-                                        /*
-                                         * This check is necessary because a
-                                         * thread could have exited like a
-                                         * normal fibril using the
-                                         * FIBRIL_FROM_DEAD switch type. In that
-                                         * case, its fibril will not have the
-                                         * stack member filled.
-                                         */
-                                        as_area_destroy(stack);
-                                }
-                                fibril_teardown(srcf->clean_after_me, true);
-                                srcf->clean_after_me = NULL;
-                        }
-
-                        return 1;       /* futex_unlock already done here */
-                }
-
-                /* Put the current fibril into the correct run list */
-                switch (stype) {
-                case FIBRIL_PREEMPT:
-                        list_append(&srcf->link, &ready_list);
-                        break;
-                case FIBRIL_FROM_MANAGER:
-                        list_append(&srcf->link, &manager_list);
-                        break;
-                default:
-                        assert(stype == FIBRIL_TO_MANAGER);
-
-                        srcf->switches++;
-
-                        /*
-                         * Don't put the current fibril into any list, it should
-                         * already be somewhere, or it will be lost.
-                         */
-                        break;
-                }
-        }
-
-        fibril_t *dstf;
-
-        /* Choose a new fibril to run */
-        switch (stype) {
-        case FIBRIL_TO_MANAGER:
-        case FIBRIL_FROM_DEAD:
-                dstf = list_get_instance(list_first(&manager_list), fibril_t,
-                    link);
-
-                if (stype == FIBRIL_FROM_DEAD)
-                        dstf->clean_after_me = srcf;
-                break;
-        default:
+
                 dstf = list_get_instance(list_first(&ready_list), fibril_t,
                     link);
                 break;
         }
-
         list_remove(&dstf->link);
+
+        /* Put the current fibril into the correct run list */
+        switch (stype) {
+        case FIBRIL_PREEMPT:
+                list_append(&srcf->link, &ready_list);
+                break;
+        case FIBRIL_FROM_MANAGER:
+                list_append(&srcf->link, &manager_list);
+                break;
+        case FIBRIL_FROM_DEAD:
+                // Nothing.
+                break;
+        case FIBRIL_TO_MANAGER:
+                srcf->switches++;
+                /*
+                 * Don't put the current fibril into any list, it should
+                 * already be somewhere, or it will be lost.
+                 */
+                break;
+        }
 
         futex_unlock(&fibril_futex);
@@ -255 +219 @@
 #endif
 
-        context_restore(&dstf->ctx);
-        /* not reached */
+        /* Swap to the next fibril. */
+        context_swap(&srcf->ctx, &dstf->ctx);
+
+        /* Restored by another fibril! */
+
+        if (srcf->clean_after_me) {
+                /*
+                 * Cleanup after the dead fibril from which we
+                 * restored context here.
+                 */
+                void *stack = srcf->clean_after_me->stack;
+                if (stack) {
+                        /*
+                         * This check is necessary because a
+                         * thread could have exited like a
+                         * normal fibril using the
+                         * FIBRIL_FROM_DEAD switch type. In that
+                         * case, its fibril will not have the
+                         * stack member filled.
+                         */
+                        as_area_destroy(stack);
+                }
+                fibril_teardown(srcf->clean_after_me, true);
+                srcf->clean_after_me = NULL;
+        }
+
+        return 1;
 }
 
@@ -289 +278 @@
         fibril->arg = arg;
 
-        context_save(&fibril->ctx);
-        context_set(&fibril->ctx, FADDR(fibril_main), fibril->stack,
-            stack_size, fibril->tcb);
-
+        context_create_t sctx = {
+                .fn = fibril_main,
+                .stack_base = fibril->stack,
+                .stack_size = stack_size,
+                .tls = fibril->tcb,
+        };
+
+        context_create(&fibril->ctx, &sctx);
         return (fid_t) fibril;
 }