You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2020/08/22 08:24:04 UTC
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #1602: arch/sim: Add support for preemptive scheduling in the simulator build using ucontext API
xiaoxiang781216 commented on a change in pull request #1602:
URL: https://github.com/apache/incubator-nuttx/pull/1602#discussion_r473016370
##########
File path: sched/task/task_setup.c
##########
@@ -593,6 +598,11 @@ static inline int nxtask_setup_stackargs(FAR struct task_tcb_s *tcb,
stackargv[argc + 1] = NULL;
tcb->argv = stackargv;
+#ifdef CONFIG_SIM_PREEMPTIBLE
+ up_setup_args_context(tcb->cmn.xcp.ucontext_buffer, tcb->cmn.start, argc,
Review comment:
it isn't good to modify the common code for the arch specific need.
##########
File path: arch/sim/src/sim/up_hosttime.c
##########
@@ -1,47 +1,251 @@
/****************************************************************************
* arch/sim/src/sim/up_hosttime.c
*
- * Copyright (C) 2008 Gregory Nutt. All rights reserved.
* Author: Gregory Nutt <gn...@nuttx.org>
+ * Sebastian Ene <nu...@fitbit.com>
Review comment:
Remove the author, the new apache license shouldn't contain the author.
##########
File path: arch/sim/src/sim/up_blocktask.c
##########
@@ -82,7 +82,7 @@ void up_block_task(struct tcb_s *tcb, tstate_t task_state)
DEBUGASSERT((tcb->task_state >= FIRST_READY_TO_RUN_STATE) &&
(tcb->task_state <= LAST_READY_TO_RUN_STATE));
- /* sinfo("Blocking TCB=%p\n", tcb); */
+ sinfo("Blocking TCB=%p %s\n", tcb, tcb->name);
Review comment:
tcb->name doesn't always exist.
##########
File path: arch/sim/src/Makefile
##########
@@ -92,6 +93,17 @@ ifeq ($(CONFIG_SPINLOCK),y)
HOSTSRCS += up_testset.c
endif
+ifeq ($(CONFIG_SIM_PREEMPTIBLE),y)
+ HOSTSRCS += up_ucontext.c
+ HOSTCFLAGS += -DCONFIG_SIM_PREEMPTIBLE=$(CONFIG_SIM_PREEMPTIBLE)
+ifeq ($(CONFIG_HOST_MACOS), y)
+ CFLAGS += -D_XOPEN_SOURCE -Wno-deprecated-declarations
Review comment:
remvoe CFLAGS change, only HOSTCFLAGS need to update
##########
File path: arch/sim/src/sim/up_initialstate.c
##########
@@ -66,7 +68,13 @@
void up_initial_state(struct tcb_s *tcb)
{
+#ifdef CONFIG_SIM_PREEMPTIBLE
+ FAR struct tcb_s *rtcb = this_task();
+ tcb->xcp.ucontext_buffer = up_create_context(&tcb->xcp.ucontext_sp,
+ rtcb->xcp.ucontext_buffer, tcb->start);
Review comment:
rtcb may exit before tcb, it isn't safe to set tcb's uc_link to rtcb's uncontext
##########
File path: arch/sim/src/sim/up_initialstate.c
##########
@@ -39,11 +39,13 @@
#include <nuttx/config.h>
+#include <setjmp.h>
Review comment:
why need include setjmp?
##########
File path: arch/sim/src/sim/up_ucontext.c
##########
@@ -0,0 +1,363 @@
+/****************************************************************************
+ * arch/sim/src/sim/up_ucontext.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <assert.h>
+#include <signal.h>
+#include <stdio.h>
+#include <stdbool.h>
+#include <stdlib.h>
+#include <stdint.h>
+#include <ucontext.h>
+
+/****************************************************************************
+ * Preprocessor Definition
+ ****************************************************************************/
+
+/* The minimum ucontext size */
+
+#define MIN_UCONTEXT_STACK_LENGTH (128 * 1024)
+#define STACK_ALIGNMENT_BYTES (8)
+#define ASSERT(x) assert((x))
+#define ARRAY_LENGTH(array) (sizeof((array))/sizeof((array)[0]))
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+typedef struct exit_data_context_s
+{
+ ucontext_t *next_ucontext;
+ ucontext_t *current_ucontext;
+ void *ucontext_sp;
+ int cpu_index;
+} exit_data_context_t;
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static ucontext_t g_uctx_main;
+
+#ifdef CONFIG_SMP
+static exit_data_context_t g_task_exit_details[CONFIG_SMP_NCPUS];
+static ucontext_t g_task_exit_context[CONFIG_SMP_NCPUS];
+#else
+static exit_data_context_t g_task_exit_details[1];
+static ucontext_t g_task_exit_context[1];
+#endif
+
+static volatile bool g_is_exit_context_init;
+static stack_t g_ss;
+
+/****************************************************************************
+ * Public Prototype
+ ****************************************************************************/
+
+#ifdef CONFIG_SMP
+int up_cpu_index(void);
+#endif
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: task_exit_point
+ *
+ * Description:
+ * This function is used to release the allocated stack memory for a task
+ * on it's exit path.
+ *
+ * Assumptions:
+ * This function does not return.
+ *
+ ****************************************************************************/
+
+static void task_exit_point(void)
+{
+ while (1)
+ {
+ int cpu_id = 0;
+
+#ifdef CONFIG_SMP
+ cpu_id = up_cpu_index();
+#endif
+ getcontext(&g_task_exit_context[cpu_id]);
+
+ if (g_task_exit_details[cpu_id].ucontext_sp)
+ {
+ free(g_task_exit_details[cpu_id].current_ucontext);
+ free(g_task_exit_details[cpu_id].ucontext_sp);
+ g_task_exit_details[cpu_id].ucontext_sp = NULL;
+ setcontext(g_task_exit_details[cpu_id].next_ucontext);
+ }
+ }
+}
+
+/****************************************************************************
+ * Name: up_setup_exit_context
+ *
+ * Description:
+ * Creates new exit context for tasks. In a SMP configuration every CPU
+ * has a different exit context.
+ *
+ ****************************************************************************/
+
+static void up_setup_exit_context(void)
+{
+ for (int i = 0; i < ARRAY_LENGTH(g_task_exit_context); i++)
+ {
+ ucontext_t *exit_context = &g_task_exit_context[i];
+ int ret = getcontext(exit_context);
+ ASSERT(ret >= 0);
+
+ uint8_t *sp = calloc(1, MIN_UCONTEXT_STACK_LENGTH);
+ ASSERT(sp != NULL);
+
+ uint64_t align_offset = STACK_ALIGNMENT_BYTES - ((uint64_t)sp %
+ STACK_ALIGNMENT_BYTES);
+
+ uint8_t *aligned_stack = sp + align_offset;
+ exit_context->uc_stack.ss_sp = aligned_stack;
+ exit_context->uc_stack.ss_size = MIN_UCONTEXT_STACK_LENGTH -
+ align_offset;
+ exit_context->uc_link = &g_uctx_main;
+
+ makecontext(exit_context, task_exit_point, 0);
+ }
+
+ g_is_exit_context_init = true;
+}
+
+/****************************************************************************
+ * Name: up_is_stack_downwards
+ *
+ * Description:
+ * This function returns true if the stack places elements downwards.
+ *
+ * Input Parameters:
+ * stack_arg - the address of a variable from the previous function stack
+ * frame
+ *
+ * Returned Value:
+ * True if the stack grows downwards otherwise false.
+ *
+ ****************************************************************************/
+
+static bool up_is_stack_downwards(void *stack_arg)
+{
+ int stack_arg_local;
+ return stack_arg > (void *)&stack_arg_local;
+}
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: up_create_context
+ *
+ * Description:
+ * Creates a new ucontext structure and initialize it.
+ *
+ * Input Parameters:
+ * ucontext_sp - buffer where we store the ucontext stack pointer
+ * prev_ucontext - the parent ucontext used%i to return to when the
+ * execution of the current context ends.
+ * entry_point - the entry point of the new context
+ *
+ * Return Value:
+ * A pointer to the ucontext structure or NULL in case something went
+ * wrong.
+ *
+ ****************************************************************************/
+
+void *up_create_context(void **ucontext_sp, void *prev_ucontext,
+ void (*entry_point)(void))
+{
+ ucontext_t *new_context = calloc(1, sizeof(ucontext_t));
+ ASSERT(new_context != NULL);
+
+ int ret = getcontext(new_context);
+ ASSERT(ret >= 0);
+
+ uint8_t *sp = calloc(1, MIN_UCONTEXT_STACK_LENGTH);
+ ASSERT(sp != NULL);
+
+ *((uintptr_t *)ucontext_sp) = (uintptr_t)sp;
+
+ uint64_t align_offset = STACK_ALIGNMENT_BYTES - ((uint64_t)sp %
+ STACK_ALIGNMENT_BYTES);
+
+ uint8_t *aligned_stack = sp + align_offset;
+ new_context->uc_stack.ss_sp = aligned_stack;
+ new_context->uc_stack.ss_size = MIN_UCONTEXT_STACK_LENGTH - align_offset;
+ new_context->uc_link = prev_ucontext == NULL ? &g_uctx_main :
+ prev_ucontext;
+
+ makecontext(new_context, entry_point, 0);
+
+ if (!g_is_exit_context_init)
+ up_setup_exit_context();
+
+ return new_context;
+}
+
+/****************************************************************************
+ * Name: up_destroy_context
+ *
+ * Description:
+ * This function releases the host memory for the context structure and the
+ * context stack and jumps to the next context specified as argument.
+ *
+ * Input Parameters:
+ * current_ucontext - pointer to the TCB context
+ * ucontext_sp - pointer to the ucontext stack
+ * new_ucontext - the next context that we want to jump to
+ * cpu_id - the CPU identificator
+ *
+ * Assumptions:
+ * This function does not return. In order to free the stack memory that we
+ * are currently executing on, we jump to a new exit context.
+ *
+ ****************************************************************************/
+
+void up_destroy_context(void *current_ucontext, void *ucontext_sp,
+ void *next_ucontext, int cpu_id)
+{
+ g_task_exit_details[cpu_id].next_ucontext = next_ucontext;
+ g_task_exit_details[cpu_id].current_ucontext = current_ucontext;
+ g_task_exit_details[cpu_id].cpu_index = cpu_id;
+ g_task_exit_details[cpu_id].ucontext_sp = ucontext_sp;
+
+ setcontext(&g_task_exit_context[cpu_id]);
+}
+
+/****************************************************************************
+ * Name: up_setup_args_context
+ *
+ * Description:
+ * This function specifies the entry point for the new context and sets up
+ * the arguments for the new task.
+ *
+ * Input Parameters:
+ * ucontext - pointer to the TCB context
+ * entry_point - the entry point for the task
+ * argc - the number of cmd arguments
+ * argv - array of arguments for the new task
+ *
+ ****************************************************************************/
+
+void up_setup_args_context(void *current_ucontext, void (*entry_point)(void),
+ int argc, char *argv[])
+{
+ makecontext(current_ucontext, entry_point, 2, argc, argv);
+}
+
+/****************************************************************************
+ * Name: up_swap_context
+ *
+ * Description:
+ * Save the current context in the old_ucontext and activate the
+ * context from activate_ucontext.
+ *
+ * Input Parameters:
+ * old_ucontext - place where we store the current context
+ * activate_ucontext - context that we will activate after function
+ * invocation.
+ *
+ ****************************************************************************/
+
+void up_swap_context(void *old_ucontext, void *activate_ucontext)
+{
+ swapcontext(old_ucontext, activate_ucontext);
+}
+
+/****************************************************************************
+ * Name: up_set_context
+ *
+ * Description:
+ * Set the current context to the specified ucontext
+ *
+ * Input Parameters:
+ * current_context - the current context
+ *
+ ****************************************************************************/
+
+void up_set_context(void *current_context)
+{
+ setcontext(current_context);
+}
+
+/****************************************************************************
+ * Name: up_cpu_simulated_interrupt
+ *
+ * Description:
+ * This function verifies if we are running on the signal handler stack.
+ *
+ * Returned Value:
+ * Return zero if we are not running on the signal stack otherwise return
+ * a non zero value.
+ *
+ ****************************************************************************/
+
+int up_cpu_simulated_interrupt(void)
+{
+ uint8_t test_address;
+ unsigned long verify_stack_arg = (unsigned long)&test_address;
+ unsigned long stack_start = (unsigned long)g_ss.ss_sp;
+ unsigned long stack_end;
+
+ if (up_is_stack_downwards(&verify_stack_arg) == true)
+ {
+ stack_end = stack_start - g_ss.ss_size;
+ return stack_start > verify_stack_arg && stack_end < verify_stack_arg;
+ }
+ else
+ {
+ stack_end = stack_start + g_ss.ss_size;
+ return stack_start < verify_stack_arg && stack_end < verify_stack_arg;
+ }
+}
+
+/****************************************************************************
+ * Name: up_setup_sigstackt
+ *
+ * Description:
+ * This function sets up an alternative stack for the signals from the
+ * host heap memory.
+ *
+ ****************************************************************************/
+
+void up_setup_sigstack(void)
+{
+ g_ss.ss_sp = malloc(SIGSTKSZ);
+ ASSERT(g_ss.ss_sp != NULL);
+
+ g_ss.ss_size = SIGSTKSZ;
+ g_ss.ss_flags = 0;
+
+ int ret = sigaltstack(&g_ss, NULL);
Review comment:
how to handle smp thread signal stack?
##########
File path: arch/sim/src/sim/up_oneshot.c
##########
@@ -357,6 +359,17 @@ FAR struct oneshot_lowerhalf_s *oneshot_initialize(int chan,
void up_timer_initialize(void)
{
+ /* Block the signals for the new threads created on the host to prevent
+ * a race condition where the simulated interrupt handler runs on another
+ * host thread. The new threads will inherit the signal mask which has
+ * blocked signals.
+ */
+
+#ifdef CONFIG_SIM_PREEMPTIBLE
+ host_prepare_timer();
+ host_init_timer(up_timer_update);
Review comment:
Why not merge host_prepare_timer into host_init_timer?
##########
File path: arch/sim/include/irq.h
##########
@@ -82,11 +78,15 @@ typedef int xcpt_reg_t;
struct xcptcontext
{
- void *sigdeliver; /* Actual type is sig_deliver_t */
+ void *sigdeliver;
+
+#ifdef CONFIG_SIM_PREEMPTIBLE
+ void *ucontext_buffer; /* Actual type is ucontext_t */
+ void *ucontext_sp;
+#endif
xcpt_reg_t regs[XCPTCONTEXT_REGS];
Review comment:
Should we change to:
```
#ifdef CONFIG_SIM_PREEMPTIBLE
void *ucontext_buffer; /* Actual type is ucontext_t */
void *ucontext_sp;
#else
xcpt_reg_t regs[XCPTCONTEXT_REGS];
#endif
```
since regs isn't used when CONFIG_SIM_PREEMPTIBLE == y.
##########
File path: arch/sim/src/Makefile
##########
@@ -92,6 +93,17 @@ ifeq ($(CONFIG_SPINLOCK),y)
HOSTSRCS += up_testset.c
endif
+ifeq ($(CONFIG_SIM_PREEMPTIBLE),y)
+ HOSTSRCS += up_ucontext.c
+ HOSTCFLAGS += -DCONFIG_SIM_PREEMPTIBLE=$(CONFIG_SIM_PREEMPTIBLE)
Review comment:
Why need pass CONFIG_SIM_PREEMPTIBLE? the code in up_ucontext.c should know that CONFIG_SIM_PREEMPTIBLE is true.
##########
File path: arch/sim/src/sim/up_hosttime.c
##########
@@ -1,47 +1,251 @@
/****************************************************************************
* arch/sim/src/sim/up_hosttime.c
*
- * Copyright (C) 2008 Gregory Nutt. All rights reserved.
* Author: Gregory Nutt <gn...@nuttx.org>
+ * Sebastian Ene <nu...@fitbit.com>
*
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
*
- * 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 NuttX nor the names of its contributors may be
- * used to endorse or promote products derived from this software
- * without specific prior written permission.
+ * http://www.apache.org/licenses/LICENSE-2.0
*
- * 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 OWNER 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.
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
*
****************************************************************************/
/****************************************************************************
* Included Files
****************************************************************************/
+#include <signal.h>
+#include <stdarg.h>
#include <stdbool.h>
+#include <stdio.h>
#include <stdint.h>
+#include <stdlib.h>
+#include <stdbool.h>
+#include <string.h>
+
+#include <sys/types.h>
+#include <sys/time.h>
+#include <sys/wait.h>
+
#include <time.h>
#include <unistd.h>
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/* The callback invoked from signal handler */
+
+typedef void (*sched_timer_callback_t)(void);
+
+/* The primask bit index corresponding to a host signal */
+
+enum host_primask_bitset_e
+{
+ PRIMASK_BIT_0_SIGALARM = 0,
+ PRIMASK_BIT_1_SIGUSR1 = 1,
Review comment:
The irq code should be generic to mask/restore all possible signals.
##########
File path: arch/sim/src/Makefile
##########
@@ -92,6 +93,17 @@ ifeq ($(CONFIG_SPINLOCK),y)
HOSTSRCS += up_testset.c
endif
+ifeq ($(CONFIG_SIM_PREEMPTIBLE),y)
+ HOSTSRCS += up_ucontext.c
+ HOSTCFLAGS += -DCONFIG_SIM_PREEMPTIBLE=$(CONFIG_SIM_PREEMPTIBLE)
+ifeq ($(CONFIG_HOST_MACOS), y)
+ CFLAGS += -D_XOPEN_SOURCE -Wno-deprecated-declarations
+ HOSTCFLAGS += -D_XOPEN_SOURCE -Wno-deprecated-declarations
Review comment:
_XOPEN_SOURCE is normally defined in source code, please move to up_ucontext.c
##########
File path: arch/sim/src/Makefile
##########
@@ -92,6 +93,17 @@ ifeq ($(CONFIG_SPINLOCK),y)
HOSTSRCS += up_testset.c
endif
+ifeq ($(CONFIG_SIM_PREEMPTIBLE),y)
+ HOSTSRCS += up_ucontext.c
+ HOSTCFLAGS += -DCONFIG_SIM_PREEMPTIBLE=$(CONFIG_SIM_PREEMPTIBLE)
+ifeq ($(CONFIG_HOST_MACOS), y)
+ CFLAGS += -D_XOPEN_SOURCE -Wno-deprecated-declarations
+ HOSTCFLAGS += -D_XOPEN_SOURCE -Wno-deprecated-declarations
+ HOSTCFLAGS += -pthread
Review comment:
why need -pthread?
##########
File path: arch/sim/src/sim/up_blocktask.c
##########
@@ -118,14 +118,18 @@ void up_block_task(struct tcb_s *tcb, tstate_t task_state)
* value, then this is really the previously running task restarting!
*/
+#ifdef CONFIG_SIM_PREEMPTIBLE
+ FAR struct tcb_s *prev_tcb = rtcb;
+#else
if (!up_setjmp(rtcb->xcp.regs))
+#endif
{
/* Restore the exception context of the rtcb at the (new) head
* of the ready-to-run task list.
*/
rtcb = this_task();
- sinfo("New Active Task TCB=%p\n", rtcb);
+ sinfo("New Active Task TCB=%p %s\n", rtcb, rtcb->name);
Review comment:
tcb->name doesn't always exist.
##########
File path: arch/sim/src/sim/up_exit.c
##########
@@ -65,9 +65,13 @@
void up_exit(int status)
{
- FAR struct tcb_s *tcb;
+ FAR struct tcb_s *tcb = this_task();
- sinfo("TCB=%p exiting\n", this_task());
+ sinfo("TCB=%p exiting\n", tcb);
+
+#ifdef CONFIG_SIM_PREEMPTIBLE
+ FAR struct tcb_s *prev_tcb = tcb;
Review comment:
variable declaration need to move to the beginning of function
##########
File path: arch/sim/src/sim/up_hosttime.c
##########
@@ -89,3 +293,107 @@ void host_sleepuntil(uint64_t nsec)
usleep((nsec - now) / 1000);
}
}
+
+#ifdef CONFIG_SIM_PREEMPTIBLE
+/****************************************************************************
+ * Name: host_prepare_timer
+ *
+ * Description:
+ * Blocks all the signals in the current host thread so that the new
+ * threads can inherit a blocked signal mask.
+ *
+ * Returned Value:
+ * This function returns 0 on success otherwise -1.
+ *
+ * Assumptions/Limitations:
+ * This function should be called prior to spawning new threads as we
+ * don't want to end up running nxsched_process_timer on a different
+ * host thread.
+ *
+ ****************************************************************************/
+
+int host_prepare_timer(void)
+{
+ sigset_t signal_mask;
+ sigemptyset(&signal_mask);
+
+ return pthread_sigmask(SIG_SETMASK, &signal_mask, NULL);
+}
+
+/****************************************************************************
+ * Name: host_init_timer
+ *
+ * Description:
+ * Creates a periodic timer and sets up a signal handler on the host.
+ *
+ * Input Parameters:
+ * nxsched_process_timer - the NuttX scheduler callback
+ *
+ * Returned Value:
+ * This function returns 0 on success otherwise a negative error code.
+ *
+ ****************************************************************************/
+
+int host_init_timer(sched_timer_callback_t sched_cb)
+{
+ g_sched_process_timer_cb = sched_cb;
+
+ int ret = host_setup_signals(host_signal_handler);
+ if (ret < 0)
+ {
+ return ret;
+ }
+
+ ret = host_setup_timer();
+ if (ret < 0)
+ {
+ return ret;
+ }
+
+ return ret;
+}
+
+/****************************************************************************
+ * Name: host_signal_save
+ *
+ * Description:
+ * Block signals and return the previous signal mask.
+ *
+ * Returned Value:
+ * This function returns the list of active signals before blocking them.
+ *
+ ****************************************************************************/
+
+uint32_t host_signal_save(void)
+{
+ sigset_t oldset, newset;
+ pthread_sigmask(0, NULL, &oldset);
+
+ /* Disable signals */
+
+ sigfillset(&newset);
+ pthread_sigmask(SIG_BLOCK, &newset, NULL);
Review comment:
change to:
pthread_sigmask(SIG_BLOCK, &newset, &oldset);
and remove line 370
##########
File path: arch/sim/src/sim/up_idle.c
##########
@@ -89,6 +89,14 @@ void up_idle(void)
}
#endif
+#ifndef CONFIG_SIM_PREEMPTIBLE
Review comment:
let' move the guard into up_timer_update, so we don't need change up_idle.c
##########
File path: arch/sim/src/sim/up_unblocktask.c
##########
@@ -78,7 +78,7 @@ void up_unblock_task(FAR struct tcb_s *tcb)
DEBUGASSERT((tcb->task_state >= FIRST_BLOCKED_STATE) &&
(tcb->task_state <= LAST_BLOCKED_STATE));
- sinfo("Unblocking TCB=%p\n", tcb);
+ sinfo("Unblocking TCB=%p %s\n", tcb, tcb->name);
Review comment:
name mayn't exit
##########
File path: arch/sim/src/sim/up_interruptcontext.c
##########
@@ -43,10 +43,49 @@
#include <nuttx/arch.h>
#include "up_internal.h"
+#ifdef CONFIG_SIM_PREEMPTIBLE
+uint32_t host_signal_save(void);
+void host_signal_restore(uint32_t flags);
+int up_cpu_simulated_interrupt(void);
+#else
+# define host_signal_save() 0
+# define host_signal_restore(f) (void)(f)
+# define up_cpu_simulated_interrupt() 0
+#endif
+
/****************************************************************************
* Public Functions
****************************************************************************/
+/****************************************************************************
+ * Name: up_irq_save
+ *
+ * Description:
+ * Disable interrupts and returned the mask before disabling them.
+ *
+ ****************************************************************************/
+
+irqstate_t up_irq_save(void)
+{
+ return host_signal_save();
Review comment:
SMP simulate IPI by signal, so we need disable/restore signal if CONFIG_SMP equals y too.
##########
File path: arch/sim/src/sim/up_unblocktask.c
##########
@@ -101,15 +101,19 @@ void up_unblock_task(FAR struct tcb_s *tcb)
* this is really the previously running task restarting!
*/
+#ifdef CONFIG_SIM_PREEMPTIBLE
+ FAR struct tcb_s *prev_tcb = rtcb;
+#else
if (!up_setjmp(rtcb->xcp.regs))
+#endif
{
/* Restore the exception context of the new task that is ready to
* run (probably tcb). This is the new rtcb at the head of the
* ready-to-run task list.
*/
rtcb = this_task();
- sinfo("New Active Task TCB=%p\n", rtcb);
+ sinfo("New Active Task TCB=%p %s\n", rtcb, rtcb->name);
Review comment:
name mayn't exist
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org