This closes #13.
Restructure signal handling logic in 'sim' to make it similar to interrupts on
real hardware. This allows other interrupt sources beyond just timers in sim
(e.g. reads/write using aio).

Switch the interval timer from ITIMER_VIRTUAL to ITIMER_REAL. This allows us
to yield when we are idle and still get track passage of time faithfully (to
do this with ITIMER_VIRTUAL requires busy waiting in the idle loop). This
change coupled with using 'sigsuspend()' in the idle loop gets the CPU
utilization from 100% to 4% at a timer frequency of 1000hz.

A detailed explanation of how context switching works with signals follows:

A voluntary context switch ends up with 'os_sched()' entering the critical
section and doing a 'longjmp()' to the task being scheduled. The new task
returns via 'os_sched()' and exits the critical section on its way out.

An involuntary context switch starts with a signal handler that enters a
critical section implicitly because all signals are masked when the handler
is executing. This in turn means that 'os_sched()' is running in a nested
critical section. The interrupted task remains on the run queue and resumes
execution in the signal handler when it runs again. The cpu executes with
signals/interrupts blocked until the handler returns at which time the
interrupted task gets control again.


Project: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-larva/repo
Commit: 
http://git-wip-us.apache.org/repos/asf/incubator-mynewt-larva/commit/9a0f9aff
Tree: 
http://git-wip-us.apache.org/repos/asf/incubator-mynewt-larva/tree/9a0f9aff
Diff: 
http://git-wip-us.apache.org/repos/asf/incubator-mynewt-larva/diff/9a0f9aff

Branch: refs/heads/master
Commit: 9a0f9aff1c72feb4331b01035e1c7547b65380b2
Parents: f91d760
Author: Neel Natu <n...@nahannisys.com>
Authored: Mon Mar 7 17:16:40 2016 -0800
Committer: wes3 <w...@micosa.io>
Committed: Mon Mar 7 20:44:46 2016 -0800

----------------------------------------------------------------------
 libs/os/include/os/arch/cortex_m0/os/os_arch.h |   1 +
 libs/os/include/os/arch/cortex_m4/os/os_arch.h |   1 +
 libs/os/include/os/arch/sim/os/os_arch.h       |   3 +-
 libs/os/src/arch/cortex_m0/os_arch_arm.c       |   5 +
 libs/os/src/arch/cortex_m4/os_arch_arm.c       |   5 +
 libs/os/src/arch/sim/os_arch_sim.c             | 263 +++++++++-----------
 libs/os/src/os.c                               |   1 +
 7 files changed, 135 insertions(+), 144 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-mynewt-larva/blob/9a0f9aff/libs/os/include/os/arch/cortex_m0/os/os_arch.h
----------------------------------------------------------------------
diff --git a/libs/os/include/os/arch/cortex_m0/os/os_arch.h 
b/libs/os/include/os/arch/cortex_m0/os/os_arch.h
index b64534e..348a33a 100755
--- a/libs/os/include/os/arch/cortex_m0/os/os_arch.h
+++ b/libs/os/include/os/arch/cortex_m0/os/os_arch.h
@@ -68,6 +68,7 @@ os_error_t os_arch_os_start(void);
 void os_set_env(void);
 void os_arch_init_task_stack(os_stack_t *sf);
 void os_default_irq_asm(void);
+void os_arch_idle(void);
 
 /* External function prototypes supplied by BSP */
 void os_bsp_systick_init(uint32_t os_ticks_per_sec);

http://git-wip-us.apache.org/repos/asf/incubator-mynewt-larva/blob/9a0f9aff/libs/os/include/os/arch/cortex_m4/os/os_arch.h
----------------------------------------------------------------------
diff --git a/libs/os/include/os/arch/cortex_m4/os/os_arch.h 
b/libs/os/include/os/arch/cortex_m4/os/os_arch.h
index 0a3a507..299d95f 100755
--- a/libs/os/include/os/arch/cortex_m4/os/os_arch.h
+++ b/libs/os/include/os/arch/cortex_m4/os/os_arch.h
@@ -68,6 +68,7 @@ os_error_t os_arch_os_start(void);
 void os_set_env(void);
 void os_arch_init_task_stack(os_stack_t *sf);
 void os_default_irq_asm(void);
+void os_arch_idle(void);
 
 /* External function prototypes supplied by BSP */
 void os_bsp_systick_init(uint32_t os_tick_per_sec);

http://git-wip-us.apache.org/repos/asf/incubator-mynewt-larva/blob/9a0f9aff/libs/os/include/os/arch/sim/os/os_arch.h
----------------------------------------------------------------------
diff --git a/libs/os/include/os/arch/sim/os/os_arch.h 
b/libs/os/include/os/arch/sim/os/os_arch.h
index 68c835c..0934ab1 100644
--- a/libs/os/include/os/arch/sim/os/os_arch.h
+++ b/libs/os/include/os/arch/sim/os/os_arch.h
@@ -54,10 +54,11 @@ os_stack_t *os_arch_task_stack_init(struct os_task *, 
os_stack_t *, int);
 void os_arch_ctx_sw(struct os_task *);
 void os_arch_ctx_sw_isr(struct os_task *);
 os_sr_t os_arch_save_sr(void);
-void os_arch_restore_sr(int);
+void os_arch_restore_sr(os_sr_t sr);
 os_error_t os_arch_os_init(void);
 void os_arch_os_stop(void);
 os_error_t os_arch_os_start(void);
+void os_arch_idle(void);
 
 void os_bsp_init(void);
 

http://git-wip-us.apache.org/repos/asf/incubator-mynewt-larva/blob/9a0f9aff/libs/os/src/arch/cortex_m0/os_arch_arm.c
----------------------------------------------------------------------
diff --git a/libs/os/src/arch/cortex_m0/os_arch_arm.c 
b/libs/os/src/arch/cortex_m0/os_arch_arm.c
index 1fd719b..ce5efd5 100755
--- a/libs/os/src/arch/cortex_m0/os_arch_arm.c
+++ b/libs/os/src/arch/cortex_m0/os_arch_arm.c
@@ -336,3 +336,8 @@ os_arch_os_start(void)
     return err;
 }
 
+void
+os_arch_idle(void)
+{
+    return;
+}

http://git-wip-us.apache.org/repos/asf/incubator-mynewt-larva/blob/9a0f9aff/libs/os/src/arch/cortex_m4/os_arch_arm.c
----------------------------------------------------------------------
diff --git a/libs/os/src/arch/cortex_m4/os_arch_arm.c 
b/libs/os/src/arch/cortex_m4/os_arch_arm.c
index 64ba05e..439d7e7 100755
--- a/libs/os/src/arch/cortex_m4/os_arch_arm.c
+++ b/libs/os/src/arch/cortex_m4/os_arch_arm.c
@@ -349,3 +349,8 @@ os_arch_os_start(void)
     return err;
 }
 
+void
+os_arch_idle(void)
+{
+    return;
+}

http://git-wip-us.apache.org/repos/asf/incubator-mynewt-larva/blob/9a0f9aff/libs/os/src/arch/sim/os_arch_sim.c
----------------------------------------------------------------------
diff --git a/libs/os/src/arch/sim/os_arch_sim.c 
b/libs/os/src/arch/sim/os_arch_sim.c
index 18e40a1..9e08ea1 100644
--- a/libs/os/src/arch/sim/os_arch_sim.c
+++ b/libs/os/src/arch/sim/os_arch_sim.c
@@ -36,7 +36,6 @@
 struct stack_frame {
     int sf_mainsp;              /* stack on which main() is executing */
     sigjmp_buf sf_jb;
-    int sf_sigsblocked;
     struct os_task *sf_task;
 };
 
@@ -49,47 +48,15 @@ CTASSERT(offsetof(struct stack_frame, sf_jb) == 4);
 
 extern void os_arch_frame_init(struct stack_frame *sf);
 
-#define CTX_SWITCH_ISR (1)
-#define CTX_SWITCH_TASK (2)
-
-#define ISR_BLOCK_OFF (0) 
-#define ISR_BLOCK_ON (1)
-
 #define sim_setjmp(__jb) sigsetjmp(__jb, 0)
 #define sim_longjmp(__jb, __ret) siglongjmp(__jb, __ret) 
 
-sigset_t g_sigset;  
-volatile int g_block_isr = ISR_BLOCK_OFF;
-
-static volatile int g_block_isr_on = ISR_BLOCK_ON;
-static volatile int g_block_isr_off = ISR_BLOCK_OFF;
-
-static int g_pending_ticks = 0;
-
-static void 
-isr_state(volatile int *state, volatile int *ostate) 
-{
-    if (ostate) {
-        *ostate = g_block_isr;
-    }
+#define OS_ASSERT_CRITICAL() (assert(os_arch_in_critical()))
 
-    if (state) {
-        g_block_isr = *state;
-    }
-}
+#define OS_USEC_PER_TICK    (1000000 / OS_TICKS_PER_SEC)
 
-
-static void 
-sigs_unblock(void)
-{
-    sigprocmask(SIG_UNBLOCK, &g_sigset, NULL);
-}
-
-static void
-sigs_block(void)
-{
-    sigprocmask(SIG_BLOCK, &g_sigset, NULL);
-}
+static int os_arch_in_critical(void);
+static sigset_t allsigs, nosigs;
 
 /*
  * Called from 'os_arch_frame_init()' when setjmp returns indirectly via
@@ -100,14 +67,17 @@ os_arch_task_start(struct stack_frame *sf, int rc)
 {
     struct os_task *task;
 
-    task = sf->sf_task;
-
-    isr_state(&g_block_isr_off, NULL);
-
-    if (rc == CTX_SWITCH_ISR) {
-        sigs_unblock();
-    }
+    /*
+     * Interrupts are disabled when a task starts executing. This happens in
+     * two different ways:
+     * - via os_arch_os_start() for the first task.
+     * - via os_sched() for all other tasks.
+     *
+     * Enable interrupts before starting the task.
+     */
+    OS_EXIT_CRITICAL(0);
 
+    task = sf->sf_task;
     task->t_func(task->t_arg);
 
     /* This should never return */ 
@@ -120,12 +90,9 @@ os_arch_task_stack_init(struct os_task *t, os_stack_t 
*stack_top, int size)
     struct stack_frame *sf;
 
     sf = (struct stack_frame *) ((uint8_t *) stack_top - sizeof(*sf));
-    sf->sf_sigsblocked = 0;
     sf->sf_task = t;
 
-    isr_state(&g_block_isr_on, NULL); 
     os_arch_frame_init(sf);
-    isr_state(&g_block_isr_off, NULL);
 
     return ((os_stack_t *)sf);
 }
@@ -137,15 +104,14 @@ os_arch_ctx_sw(struct os_task *next_t)
     struct stack_frame *sf; 
     int rc;
 
+    OS_ASSERT_CRITICAL();
     t = os_sched_get_current_task();
     if (t) {
         sf = (struct stack_frame *) t->t_stackptr;
 
         rc = sim_setjmp(sf->sf_jb);
         if (rc != 0) {
-            if (rc == CTX_SWITCH_ISR) { 
-                sigs_unblock();
-            }
+            OS_ASSERT_CRITICAL();
             return;
         }
     }
@@ -155,136 +121,147 @@ os_arch_ctx_sw(struct os_task *next_t)
     os_sched_set_current_task(next_t);
 
     sf = (struct stack_frame *) next_t->t_stackptr;
-    /* block signals if the task we switch to expects them blocked. */
-    if (sf->sf_sigsblocked) {
-        sigs_block();
-        rc = CTX_SWITCH_ISR;
-    } else {
-        rc = CTX_SWITCH_TASK;
-    }
-
-    sim_longjmp(sf->sf_jb, rc);
+    sim_longjmp(sf->sf_jb, 1);
 }
 
 void
 os_arch_ctx_sw_isr(struct os_task *next_t)
 {
-    struct os_task *t;
-    struct stack_frame *sf;
-    int isr_ctx;
-    volatile int block_isr_off;
-    int rc;
+    os_arch_ctx_sw(next_t);
+}
 
-    block_isr_off = g_block_isr_off; 
+/*
+ * Disable signals and enter a critical section.
+ *
+ * Returns 1 if signals were already blocked and 0 otherwise.
+ */
+os_sr_t 
+os_arch_save_sr(void)
+{
+    int error;
+    sigset_t omask;
 
-    t = os_sched_get_current_task();
-    if (t) {
-        sf = (struct stack_frame *) t->t_stackptr;
-        
-        /* block signals coming from an interrupt context */
-        sf->sf_sigsblocked = 1;
+    error = sigprocmask(SIG_BLOCK, &allsigs, &omask);
+    assert(error == 0);
 
-        isr_state(&g_block_isr_on, &isr_ctx);
+    return (omask == allsigs);
+}
 
-        rc = sim_setjmp(sf->sf_jb);
-        if (rc != 0) {
-            sf->sf_sigsblocked = 0;
-            isr_state(&block_isr_off, NULL);
-            return;
-        }
-    }
+void
+os_arch_restore_sr(os_sr_t osr)
+{
+    int error;
 
-    isr_state(&block_isr_off, NULL);
+    OS_ASSERT_CRITICAL();
+    assert(osr == 0 || osr == 1);
 
-    os_sched_ctx_sw_hook(next_t);
+    if (osr == 1) {
+        /* Exiting a nested critical section */
+        return;
+    }
 
-    os_sched_set_current_task(next_t);
-    
-    sf = (struct stack_frame *) next_t->t_stackptr;
-    sim_longjmp(sf->sf_jb, CTX_SWITCH_ISR); 
+    error = sigprocmask(SIG_UNBLOCK, &allsigs, NULL);
+    assert(error == 0);
 }
 
-os_sr_t 
-os_arch_save_sr(void)
+static int
+os_arch_in_critical(void)
 {
-    int isr_ctx;
+    int error;
+    sigset_t omask;
 
-    isr_state(&g_block_isr_on, &isr_ctx); 
+    error = sigprocmask(SIG_SETMASK, NULL, &omask);
+    assert(error == 0);
 
-    return (isr_ctx); 
+    return (omask == allsigs);
 }
 
 void
-os_arch_restore_sr(int isr_ctx)  
+os_arch_idle(void)
 {
-    if (isr_ctx == ISR_BLOCK_ON) {
-        return;
-    } else {
-        isr_state(&g_block_isr_off, NULL);
-    }
+    assert(!os_arch_in_critical());
+
+    sigsuspend(&nosigs);        /* Wait for a signal to wake us up */
 }
 
 static void timer_handler(int sig);
 
+static struct {
+    int num;
+    void (*handler)(int sig);
+} signals[] = {
+    { SIGALRM, timer_handler },
+};
+
+#define NUMSIGS     (sizeof(signals)/sizeof(signals[0]))
+
 static void
-initialize_signals(void)
+signals_init(void)
 {
+    int i, error;
     struct sigaction sa;
 
-    sigemptyset(&g_sigset);
-    sigaddset(&g_sigset, SIGALRM); 
-    sigaddset(&g_sigset, SIGVTALRM);
-
-    memset(&sa, 0, sizeof sa);
-    sa.sa_handler = timer_handler;
+    sigemptyset(&nosigs);
+    sigemptyset(&allsigs);
+    for (i = 0; i < NUMSIGS; i++) {
+        sigaddset(&allsigs, signals[i].num);
+    }
 
-    sigaction(SIGALRM, &sa, NULL);
-    sigaction(SIGVTALRM, &sa, NULL);
+    for (i = 0; i < NUMSIGS; i++) {
+        memset(&sa, 0, sizeof sa);
+        sa.sa_handler = signals[i].handler;
+        sa.sa_mask = allsigs;
+        error = sigaction(signals[i].num, &sa, NULL);
+        assert(error == 0);
+    }
 }
 
 static void
-cancel_signals(void)
+signals_cleanup(void)
 {
+    int i, error;
     struct sigaction sa;
 
-    memset(&sa, 0, sizeof sa);
-    sa.sa_handler = SIG_DFL;
-
-    sigaction(SIGALRM, &sa, NULL);
-    sigaction(SIGVTALRM, &sa, NULL);
+    for (i = 0; i < NUMSIGS; i++) {
+        memset(&sa, 0, sizeof sa);
+        sa.sa_handler = SIG_DFL;
+        error = sigaction(signals[i].num, &sa, NULL);
+        assert(error == 0);
+    }
 }
 
 static void
 timer_handler(int sig)
 {
+    struct timeval time_now, time_diff; 
+    int ticks;
+
     static struct timeval time_last;
     static int time_inited; 
-    struct timeval time_now, time_diff; 
-    int isr_ctx;
 
     if (!time_inited) {
         gettimeofday(&time_last, NULL);
         time_inited = 1;
     }
 
-    isr_state(NULL, &isr_ctx); 
-    if (isr_ctx == ISR_BLOCK_ON) {
-        ++g_pending_ticks;
-        return;
-    }
-
     gettimeofday(&time_now, NULL);
     timersub(&time_now, &time_last, &time_diff);
 
-    g_pending_ticks = time_diff.tv_usec / 1000;
+    ticks = time_diff.tv_sec * OS_TICKS_PER_SEC;
+    ticks += time_diff.tv_usec / OS_USEC_PER_TICK;
 
-    while (--g_pending_ticks >= 0) {
+    while (--ticks >= 0) {
         os_time_tick();
         os_callout_tick();
     }
 
-    time_last = time_now;
-    g_pending_ticks = 0;
+    /*
+     * Update 'time_last' but account for the remainder usecs that did not
+     * contribute towards whole 'ticks'.
+     */
+    time_diff.tv_sec = 0;
+    time_diff.tv_usec %= OS_USEC_PER_TICK;
+    timersub(&time_now, &time_diff, &time_last);
 
     os_sched_os_timer_exp();
     os_sched(NULL, 1); 
@@ -296,21 +273,14 @@ start_timer(void)
     struct itimerval it; 
     int rc;
 
-    initialize_signals();
-
-    /* 1 msec OS tick */
     memset(&it, 0, sizeof(it));
     it.it_value.tv_sec = 0;
-    it.it_value.tv_usec = 1000;
+    it.it_value.tv_usec = OS_USEC_PER_TICK;
     it.it_interval.tv_sec = 0;
-    it.it_interval.tv_usec = 1000;
+    it.it_interval.tv_usec = OS_USEC_PER_TICK;
 
-    rc = setitimer(ITIMER_VIRTUAL, &it, NULL);
-    if (rc != 0) {
-        const char msg[] = "Cannot set itimer";
-        write(2, msg, sizeof(msg));
-        _exit(1);
-    }
+    rc = setitimer(ITIMER_REAL, &it, NULL);
+    assert(rc == 0);
 }
 
 static void
@@ -321,14 +291,8 @@ stop_timer(void)
 
     memset(&it, 0, sizeof(it));
 
-    rc = setitimer(ITIMER_VIRTUAL, &it, NULL);
-    if (rc != 0) {
-        const char msg[] = "Cannot stop itimer";
-        write(2, msg, sizeof(msg));
-        _exit(1);
-    }
-
-    cancel_signals();
+    rc = setitimer(ITIMER_REAL, &it, NULL);
+    assert(rc == 0);
 }
 
 os_error_t 
@@ -352,7 +316,19 @@ os_arch_os_start(void)
 {
     struct stack_frame *sf; 
     struct os_task *t;
+    os_sr_t sr;
+
+    /* Setup all interrupt handlers */
+    signals_init();
+
+    /*
+     * Disable interrupts before enabling any interrupt sources. Pending
+     * interrupts will be recognized when the first task starts executing.
+     */
+    OS_ENTER_CRITICAL(sr);
+    assert(sr == 0);
 
+    /* Enable the interrupt sources */
     start_timer();
 
     t = os_sched_next_task();
@@ -361,7 +337,7 @@ os_arch_os_start(void)
     g_os_started = 1; 
 
     sf = (struct stack_frame *) t->t_stackptr;
-    sim_longjmp(sf->sf_jb, CTX_SWITCH_TASK); 
+    sim_longjmp(sf->sf_jb, 1);
 
     return 0;
 }
@@ -374,5 +350,6 @@ void
 os_arch_os_stop(void)
 {
     stop_timer();
+    signals_cleanup();
     g_os_started = 0;
 }

http://git-wip-us.apache.org/repos/asf/incubator-mynewt-larva/blob/9a0f9aff/libs/os/src/os.c
----------------------------------------------------------------------
diff --git a/libs/os/src/os.c b/libs/os/src/os.c
index dd43cf0..078800a 100644
--- a/libs/os/src/os.c
+++ b/libs/os/src/os.c
@@ -37,6 +37,7 @@ os_idle_task(void *arg)
     /* For now, idle task simply increments a counter to show it is running. */
     while (1) {
         ++g_os_idle_ctr;
+        os_arch_idle();
     }
 }
 

Reply via email to