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/09/04 14:03:35 UTC
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #1703: arch/sim: Add support for ucontext API
xiaoxiang781216 commented on a change in pull request #1703:
URL: https://github.com/apache/incubator-nuttx/pull/1703#discussion_r483609908
##########
File path: arch/sim/Kconfig
##########
@@ -485,4 +485,10 @@ config SIM_HCISOCKET
control of the device, but is abstracted from the
physical interface which is still handled by Linux.
+config SIM_UCONTEXT_PREEMPTION
Review comment:
let's always use ucontext API? because:
1.It's hard to maintain two different switch mechanism
2.ucontext can be used on all interesting platform(Linux/macOS/Cygwin)
##########
File path: arch/sim/src/Makefile
##########
@@ -97,6 +97,14 @@ ifeq ($(CONFIG_SPINLOCK),y)
HOSTSRCS += up_testset.c
endif
+ifeq ($(CONFIG_SIM_UCONTEXT_PREEMPTION),y)
+ HOSTSRCS += up_ucontext.c
+ifeq ($(CONFIG_HOST_MACOS), y)
+ HOSTCFLAGS += -Wno-deprecated-declarations
+ HOSTCFLAGS += -pthread
Review comment:
no need, added at line 87
##########
File path: arch/sim/src/sim/up_blocktask.c
##########
@@ -118,7 +118,11 @@ 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_UCONTEXT_PREEMPTION
+ FAR struct tcb_s *prev_tcb = rtcb;
Review comment:
need move declaration to the begin of block to confirm C89 standard.
##########
File path: arch/sim/src/sim/up_exit.c
##########
@@ -94,7 +97,14 @@ void up_exit(int status)
/* Then switch contexts */
+#ifdef CONFIG_SIM_UCONTEXT_PREEMPTION
+ up_destroy_context(prev_tcb->xcp.ucontext_buffer,
Review comment:
it's enough to call up_setcontext, we can directly reuse the context without destroy. Actually, ucontext don't define destroycontext API.
##########
File path: arch/sim/src/sim/up_releasepending.c
##########
@@ -87,7 +87,11 @@ void up_release_pending(void)
* this is really the previously running task restarting!
*/
+#ifdef CONFIG_SIM_UCONTEXT_PREEMPTION
+ FAR struct tcb_s *prev_tcb = rtcb;
Review comment:
move to the begin of block.
##########
File path: arch/sim/src/sim/up_unblocktask.c
##########
@@ -129,7 +133,19 @@ void up_unblock_task(FAR struct tcb_s *tcb)
/* Then switch contexts */
+#ifdef CONFIG_SIM_UCONTEXT_PREEMPTION
+ if (prev_tcb == rtcb)
Review comment:
it's impossible, line 91 ensure the current task must change.
##########
File path: arch/sim/src/sim/up_reprioritizertr.c
##########
@@ -142,7 +142,11 @@ void up_reprioritize_rtr(struct tcb_s *tcb, uint8_t priority)
* restarting!
*/
+#ifdef CONFIG_SIM_UCONTEXT_PREEMPTION
+ FAR struct tcb_s *prev_tcb = rtcb;
Review comment:
move to begin of the block
##########
File path: arch/sim/src/sim/up_ucontext.c
##########
@@ -0,0 +1,283 @@
+/****************************************************************************
+ * 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
+ ****************************************************************************/
+
+/* This define is required to compile on OSX */
+
+#ifndef _XOPEN_SOURCE
+# define _XOPEN_SOURCE (500)
+#endif
+
+#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 stack 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
+ ****************************************************************************/
+
+/* This container stores the exit information for a NuttX task */
+
+typedef struct exit_data_context_s
+{
+ ucontext_t *parent_ucontext; /* We jump to this context after we release
+ * the resources for the task that wants to
+ * exit.
+ */
+ ucontext_t *current_ucontext;
+ void *ucontext_sp;
+ int cpu_index;
+} exit_data_context_t;
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+/* This context is used on the exit path for the initial task */
+
+static ucontext_t g_uctx_main;
+
+/* We use an alternate ucontext for the task exit point to tear down
+ * allocated resources. On the task exit we jump to this exit context
+ * and we free the stack memory used by ucontext.
+ */
+
+#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];
+static uint8_t
+ g_task_exit_context_stack[CONFIG_SMP_NCPUS][MIN_UCONTEXT_STACK_LENGTH];
+#else
+static exit_data_context_t g_task_exit_details[1];
+static ucontext_t g_task_exit_context[1];
+static uint8_t g_task_exit_context_stack[1][MIN_UCONTEXT_STACK_LENGTH];
+#endif
+
+/****************************************************************************
+ * 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)
+{
+ for (; ; )
+ {
+ int cpu_id = 0;
+
+#ifdef CONFIG_SMP
+ cpu_id = up_cpu_index();
+#endif
+ if (g_task_exit_details[cpu_id].ucontext_sp)
+ {
+ /* Free stack memory used by the ucontext structure */
+
+ free(g_task_exit_details[cpu_id].current_ucontext);
+ free(g_task_exit_details[cpu_id].ucontext_sp);
+
+ /* Activate the parent ucontext */
+
+ setcontext(g_task_exit_details[cpu_id].parent_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(int cpu)
+{
+ ucontext_t *exit_context = &g_task_exit_context[cpu];
+ int ret = getcontext(exit_context);
+ ASSERT(ret >= 0);
+
+ uint8_t *sp = &g_task_exit_context_stack[cpu][0];
+
+ 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);
+}
+
+/****************************************************************************
+ * 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);
Review comment:
we need use the stack allocated by NuttX's kernel.
##########
File path: arch/sim/src/sim/up_unblocktask.c
##########
@@ -101,7 +101,11 @@ void up_unblock_task(FAR struct tcb_s *tcb)
* this is really the previously running task restarting!
*/
+#ifdef CONFIG_SIM_UCONTEXT_PREEMPTION
+ FAR struct tcb_s *prev_tcb = rtcb;
Review comment:
move to begin of the block
##########
File path: arch/sim/src/sim/up_smpsignal.c
##########
@@ -116,7 +116,11 @@ int up_cpu_paused(int cpu)
* this is really the previously running task restarting!
*/
- if (up_setjmp(rtcb->xcp.regs) == 0)
+#ifdef CONFIG_SIM_UCONTEXT_PREEMPTION
+ FAR struct tcb_s *prev_tcb = rtcb;
Review comment:
move to begin of the block.
##########
File path: arch/sim/src/sim/up_ucontext.c
##########
@@ -0,0 +1,283 @@
+/****************************************************************************
+ * 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
+ ****************************************************************************/
+
+/* This define is required to compile on OSX */
+
+#ifndef _XOPEN_SOURCE
+# define _XOPEN_SOURCE (500)
+#endif
+
+#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 stack 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
+ ****************************************************************************/
+
+/* This container stores the exit information for a NuttX task */
+
+typedef struct exit_data_context_s
+{
+ ucontext_t *parent_ucontext; /* We jump to this context after we release
+ * the resources for the task that wants to
+ * exit.
+ */
+ ucontext_t *current_ucontext;
+ void *ucontext_sp;
+ int cpu_index;
+} exit_data_context_t;
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+/* This context is used on the exit path for the initial task */
+
+static ucontext_t g_uctx_main;
+
+/* We use an alternate ucontext for the task exit point to tear down
+ * allocated resources. On the task exit we jump to this exit context
+ * and we free the stack memory used by ucontext.
+ */
+
+#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];
+static uint8_t
+ g_task_exit_context_stack[CONFIG_SMP_NCPUS][MIN_UCONTEXT_STACK_LENGTH];
+#else
+static exit_data_context_t g_task_exit_details[1];
+static ucontext_t g_task_exit_context[1];
+static uint8_t g_task_exit_context_stack[1][MIN_UCONTEXT_STACK_LENGTH];
+#endif
+
+/****************************************************************************
+ * 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)
+{
+ for (; ; )
+ {
+ int cpu_id = 0;
+
+#ifdef CONFIG_SMP
+ cpu_id = up_cpu_index();
+#endif
+ if (g_task_exit_details[cpu_id].ucontext_sp)
+ {
+ /* Free stack memory used by the ucontext structure */
+
+ free(g_task_exit_details[cpu_id].current_ucontext);
+ free(g_task_exit_details[cpu_id].ucontext_sp);
+
+ /* Activate the parent ucontext */
+
+ setcontext(g_task_exit_details[cpu_id].parent_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(int cpu)
+{
+ ucontext_t *exit_context = &g_task_exit_context[cpu];
+ int ret = getcontext(exit_context);
+ ASSERT(ret >= 0);
+
+ uint8_t *sp = &g_task_exit_context_stack[cpu][0];
+
+ 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);
+}
+
+/****************************************************************************
+ * 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));
Review comment:
we can directly define a global ucontext_t array which will simplify the destroying process a lot.
----------------------------------------------------------------
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