You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by je...@apache.org on 2020/12/10 07:34:05 UTC
[incubator-nuttx] branch master updated (8d01185 -> f3a81cb)
This is an automated email from the ASF dual-hosted git repository.
jerpelea pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-nuttx.git.
from 8d01185 [Performance]net/tcp: send the ACK in time after obtain ahead buffer from iobs
new a249050 sched: irq: Change irq_waitlock() from private to public
new 409c65c arch, sched: Fix global IRQ control logics for SMP
new ad9f88f Revert "Revert "arch/sim: Make the SIGUSR1 host signal to use the NuttX irq logic""
new f3a81cb sim: Fix interrupt handling for SMP
The 4 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
TODO | 68 +----
arch/arm/src/armv7-a/arm_cpupause.c | 17 +-
arch/arm/src/armv7-a/arm_schedulesigaction.c | 14 +-
arch/arm/src/armv7-m/arm_schedulesigaction.c | 14 +-
arch/arm/src/cxd56xx/cxd56_cpupause.c | 32 ++-
arch/arm/src/lc823450/lc823450_cpupause.c | 17 +-
arch/risc-v/src/k210/k210_cpupause.c | 17 +-
arch/risc-v/src/k210/k210_schedulesigaction.c | 14 +-
arch/sim/include/arch.h | 33 ---
arch/sim/src/Makefile | 1 +
arch/sim/src/sim/up_blocktask.c | 9 +-
.../src/sim/up_copyfullstate.c} | 18 +-
arch/sim/src/sim/up_exit.c | 6 +
arch/sim/src/sim/up_hostirq.c | 5 +
arch/sim/src/sim/up_idle.c | 28 ++
arch/sim/src/sim/up_internal.h | 36 ++-
arch/sim/src/sim/up_interruptcontext.c | 54 ++--
arch/sim/src/sim/up_releasepending.c | 9 +-
arch/sim/src/sim/up_reprioritizertr.c | 9 +-
arch/sim/src/sim/up_schedulesigaction.c | 10 +
arch/sim/src/sim/up_simsmp.c | 166 +++---------
arch/sim/src/sim/up_smpsignal.c | 283 ++++++++++++++++++---
arch/sim/src/sim/up_unblocktask.c | 27 +-
arch/xtensa/src/common/xtensa_cpupause.c | 17 +-
arch/xtensa/src/common/xtensa_schedsigaction.c | 14 +-
boards/sim/sim/sim/configs/smp/defconfig | 7 +-
include/nuttx/irq.h | 50 ++++
sched/irq/irq_csection.c | 2 +-
sched/sched/sched_addreadytorun.c | 43 +---
sched/sched/sched_removereadytorun.c | 43 +---
sched/sched/sched_resumescheduler.c | 7 +-
sched/task/task_exit.c | 7 +
32 files changed, 624 insertions(+), 453 deletions(-)
copy arch/{arm/src/armv7-m/arm_copyfullstate.c => sim/src/sim/up_copyfullstate.c} (81%)
[incubator-nuttx] 03/04: Revert "Revert "arch/sim: Make the SIGUSR1
host signal to use the NuttX irq logic""
Posted by je...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
jerpelea pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-nuttx.git
commit ad9f88f04203478bcdc0ad6a823b7dbdfa1e70b2
Author: Masayuki Ishikawa <ma...@gmail.com>
AuthorDate: Thu Nov 26 21:00:18 2020 +0900
Revert "Revert "arch/sim: Make the SIGUSR1 host signal to use the NuttX irq logic""
This reverts commit 3098b617760a0a1a5f3b74a0ae8a1f8648467902.
---
arch/sim/include/arch.h | 33 -------------------------
arch/sim/src/sim/up_internal.h | 1 +
arch/sim/src/sim/up_simsmp.c | 55 ++---------------------------------------
arch/sim/src/sim/up_smpsignal.c | 45 +++++++++++++++++++++++++++++++++
4 files changed, 48 insertions(+), 86 deletions(-)
diff --git a/arch/sim/include/arch.h b/arch/sim/include/arch.h
index 4798968..3fca681 100644
--- a/arch/sim/include/arch.h
+++ b/arch/sim/include/arch.h
@@ -41,14 +41,6 @@
#define __ARCH_SIM_INCLUDE_ARCH_H
/****************************************************************************
- * Included Files
- ****************************************************************************/
-
-/****************************************************************************
- * Pre-processor Definitions
- ****************************************************************************/
-
-/****************************************************************************
* Inline functions
****************************************************************************/
@@ -61,29 +53,4 @@ static inline uintptr_t sim_getsp(void)
return (uintptr_t)__builtin_frame_address(0);
}
-/****************************************************************************
- * Public Types
- ****************************************************************************/
-
-/****************************************************************************
- * Public Data
- ****************************************************************************/
-
-/****************************************************************************
- * Public Function Prototypes
- ****************************************************************************/
-
-#ifdef __cplusplus
-#define EXTERN extern "C"
-extern "C"
-{
-#else
-#define EXTERN extern
-#endif
-
-#undef EXTERN
-#ifdef __cplusplus
-}
-#endif
-
#endif /* __ARCH_SIM_INCLUDE_ARCH_H */
diff --git a/arch/sim/src/sim/up_internal.h b/arch/sim/src/sim/up_internal.h
index c356308..48ea33f 100644
--- a/arch/sim/src/sim/up_internal.h
+++ b/arch/sim/src/sim/up_internal.h
@@ -243,6 +243,7 @@ void sim_cpu0_start(void);
void up_cpu_started(void);
int up_cpu_paused(int cpu);
struct tcb_s *up_this_task(void);
+int up_cpu_set_pause_handler(int irq);
#endif
/* up_oneshot.c *************************************************************/
diff --git a/arch/sim/src/sim/up_simsmp.c b/arch/sim/src/sim/up_simsmp.c
index 56f8ac6..478b4d7 100644
--- a/arch/sim/src/sim/up_simsmp.c
+++ b/arch/sim/src/sim/up_simsmp.c
@@ -123,7 +123,6 @@ static void *sim_idle_trampoline(void *arg)
#ifdef CONFIG_SIM_WALLTIME
uint64_t now = 0;
#endif
- sigset_t set;
int ret;
/* Set the CPU number for the CPU thread */
@@ -137,14 +136,7 @@ static void *sim_idle_trampoline(void *arg)
/* Make sure the SIGUSR1 is not masked */
- sigemptyset(&set);
- sigaddset(&set, SIGUSR1);
-
- ret = pthread_sigmask(SIG_UNBLOCK, &set, NULL);
- if (ret < 0)
- {
- return NULL;
- }
+ up_cpu_set_pause_handler(SIGUSR1);
/* Let up_cpu_start() continue */
@@ -181,28 +173,6 @@ static void *sim_idle_trampoline(void *arg)
}
/****************************************************************************
- * Name: sim_handle_signal
- *
- * Description:
- * This is the SIGUSR signal handler. It implements the core logic of
- * up_cpu_pause() on the thread of execution the simulated CPU.
- *
- * Input Parameters:
- * arg - Standard sigaction arguments
- *
- * Returned Value:
- * None
- *
- ****************************************************************************/
-
-static void sim_handle_signal(int signo, siginfo_t *info, void *context)
-{
- int cpu = (int)((uintptr_t)pthread_getspecific(g_cpu_key));
-
- up_cpu_paused(cpu);
-}
-
-/****************************************************************************
* Public Functions
****************************************************************************/
@@ -223,8 +193,6 @@ static void sim_handle_signal(int signo, siginfo_t *info, void *context)
void sim_cpu0_start(void)
{
- struct sigaction act;
- sigset_t set;
int ret;
g_cpu_thread[0] = pthread_self();
@@ -247,26 +215,7 @@ void sim_cpu0_start(void)
/* Register the common signal handler for all threads */
- act.sa_sigaction = sim_handle_signal;
- act.sa_flags = SA_SIGINFO;
- sigemptyset(&act.sa_mask);
-
- ret = sigaction(SIGUSR1, &act, NULL);
- if (ret < 0)
- {
- return;
- }
-
- /* Make sure the SIGUSR1 is not masked */
-
- sigemptyset(&set);
- sigaddset(&set, SIGUSR1);
-
- ret = pthread_sigmask(SIG_UNBLOCK, &set, NULL);
- if (ret < 0)
- {
- return;
- }
+ up_cpu_set_pause_handler(SIGUSR1);
}
/****************************************************************************
diff --git a/arch/sim/src/sim/up_smpsignal.c b/arch/sim/src/sim/up_smpsignal.c
index 2b22aa5..7d9baca 100644
--- a/arch/sim/src/sim/up_smpsignal.c
+++ b/arch/sim/src/sim/up_smpsignal.c
@@ -47,6 +47,32 @@
#include "up_internal.h"
/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: sim_cpupause_handler
+ *
+ * Description:
+ * This is the SIGUSR signal handler. It implements the core logic of
+ * up_cpu_pause() on the thread of execution the simulated CPU.
+ *
+ * Input Parameters:
+ * irq - the interrupt number
+ * context - not used
+ * arg - not used
+ *
+ * Returned Value:
+ * In case of success OK (0) is returned otherwise a negative value.
+ *
+ ****************************************************************************/
+
+static int sim_cpupause_handler(int irq, FAR void *context, FAR void *arg)
+{
+ return up_cpu_paused(this_cpu());
+}
+
+/****************************************************************************
* Public Functions
****************************************************************************/
@@ -204,3 +230,22 @@ struct tcb_s *up_this_task(void)
{
return this_task();
}
+
+/****************************************************************************
+ * Name: up_cpu_set_pause_handler
+ *
+ * Description:
+ * Attach the CPU pause request interrupt to the NuttX logic.
+ *
+ * Input Parameters:
+ * irq - the SIGUSR1 interrupt number
+ *
+ * Returned Value:
+ * On success returns OK (0), otherwise a negative value.
+ ****************************************************************************/
+
+int up_cpu_set_pause_handler(int irq)
+{
+ up_enable_irq(irq);
+ return irq_attach(irq, sim_cpupause_handler, NULL);
+}
[incubator-nuttx] 01/04: sched: irq: Change irq_waitlock() from
private to public
Posted by je...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
jerpelea pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-nuttx.git
commit a24905059e8d3dcf9e5cad2d4251af9177de8606
Author: Masayuki Ishikawa <ma...@gmail.com>
AuthorDate: Wed Nov 25 09:53:03 2020 +0900
sched: irq: Change irq_waitlock() from private to public
Signed-off-by: Masayuki Ishikawa <Ma...@jp.sony.com>
---
include/nuttx/irq.h | 50 ++++++++++++++++++++++++++++++++++++++++++++++++
sched/irq/irq_csection.c | 2 +-
2 files changed, 51 insertions(+), 1 deletion(-)
diff --git a/include/nuttx/irq.h b/include/nuttx/irq.h
index 07474bc..0a6c094 100644
--- a/include/nuttx/irq.h
+++ b/include/nuttx/irq.h
@@ -46,6 +46,9 @@
#ifndef __ASSEMBLY__
# include <stdint.h>
# include <assert.h>
+# ifdef CONFIG_SMP
+# include <stdbool.h>
+# endif
#endif
/* Now include architecture-specific types */
@@ -171,6 +174,53 @@ int irqchain_detach(int irq, xcpt_t isr, FAR void *arg);
#endif
/****************************************************************************
+ * Name: irq_waitlock
+ *
+ * Description:
+ * Spin to get g_irq_waitlock, handling a known deadlock condition:
+ *
+ * A deadlock may occur if enter_critical_section is called from an
+ * interrupt handler. Suppose:
+ *
+ * - CPUn is in a critical section and has the g_cpu_irqlock spinlock.
+ * - CPUm takes an interrupt and attempts to enter the critical section.
+ * - It spins waiting on g_cpu_irqlock with interrupts disabled.
+ * - CPUn calls up_cpu_pause() to pause operation on CPUm. This will
+ * issue an inter-CPU interrupt to CPUm
+ * - But interrupts are disabled on CPUm so the up_cpu_pause() is never
+ * handled, causing the deadlock.
+ *
+ * This same deadlock can occur in the normal tasking case:
+ *
+ * - A task on CPUn enters a critical section and has the g_cpu_irqlock
+ * spinlock.
+ * - Another task on CPUm attempts to enter the critical section but has
+ * to wait, spinning to get g_cpu_irqlock with interrupts disabled.
+ * - The task on CPUn causes a new task to become ready-to-run and the
+ * scheduler selects CPUm. CPUm is requested to pause via a pause
+ * interrupt.
+ * - But the task on CPUm is also attempting to enter the critical
+ * section. Since it is spinning with interrupts disabled, CPUm cannot
+ * process the pending pause interrupt, causing the deadlock.
+ *
+ * This function detects this deadlock condition while spinning with \
+ * interrupts disabled.
+ *
+ * Input Parameters:
+ * cpu - The index of CPU that is trying to enter the critical section.
+ *
+ * Returned Value:
+ * True: The g_cpu_irqlock spinlock has been taken.
+ * False: The g_cpu_irqlock spinlock has not been taken yet, but there is
+ * a pending pause interrupt request.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_SMP
+bool irq_waitlock(int cpu);
+#endif
+
+/****************************************************************************
* Name: enter_critical_section
*
* Description:
diff --git a/sched/irq/irq_csection.c b/sched/irq/irq_csection.c
index c26e6d6..34565da 100644
--- a/sched/irq/irq_csection.c
+++ b/sched/irq/irq_csection.c
@@ -105,7 +105,7 @@ volatile uint8_t g_cpu_nestcount[CONFIG_SMP_NCPUS];
****************************************************************************/
#ifdef CONFIG_SMP
-static inline bool irq_waitlock(int cpu)
+bool irq_waitlock(int cpu)
{
#ifdef CONFIG_SCHED_INSTRUMENTATION_SPINLOCKS
FAR struct tcb_s *tcb = current_task(cpu);
[incubator-nuttx] 04/04: sim: Fix interrupt handling for SMP
Posted by je...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
jerpelea pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-nuttx.git
commit f3a81cb1b70a8ccf024c2deb593b1102aece62c3
Author: Masayuki Ishikawa <ma...@gmail.com>
AuthorDate: Thu Nov 26 20:43:51 2020 +0900
sim: Fix interrupt handling for SMP
Summary
- This commit fixes interrupt handling for SMP
- The following are the changes
- Introduce up_copyfullstate.c
- Add enter_critical_section() to up_exit()
- Add a critical section to up_schedule_sigaction()
- Introduce pseudo timer thread to send periodic events
- UART and interval timer are now handled in the pause handler
- Apply the same SMP related code as other CPU architectures
- However, signal handling and context switching are not changed
- Also enable debug features and some tools in smp/defconfig
Imact
- SMP only
Testing
- Tested with sim:smp on ubuntu18.04 x86_64
- Tested with hello, taskset, smp, ostest
Signed-off-by: Masayuki Ishikawa <Ma...@jp.sony.com>
---
arch/sim/src/Makefile | 1 +
arch/sim/src/sim/up_blocktask.c | 9 +-
arch/sim/src/sim/up_copyfullstate.c | 64 +++++++++
arch/sim/src/sim/up_exit.c | 6 +
arch/sim/src/sim/up_hostirq.c | 5 +
arch/sim/src/sim/up_idle.c | 28 ++++
arch/sim/src/sim/up_internal.h | 35 +++--
arch/sim/src/sim/up_interruptcontext.c | 54 ++++---
arch/sim/src/sim/up_releasepending.c | 9 +-
arch/sim/src/sim/up_reprioritizertr.c | 9 +-
arch/sim/src/sim/up_schedulesigaction.c | 10 ++
arch/sim/src/sim/up_simsmp.c | 131 +++++------------
arch/sim/src/sim/up_smpsignal.c | 240 +++++++++++++++++++++++++------
arch/sim/src/sim/up_unblocktask.c | 27 +++-
boards/sim/sim/sim/configs/smp/defconfig | 7 +-
15 files changed, 459 insertions(+), 176 deletions(-)
diff --git a/arch/sim/src/Makefile b/arch/sim/src/Makefile
index a6935ef..f21bcbc 100644
--- a/arch/sim/src/Makefile
+++ b/arch/sim/src/Makefile
@@ -74,6 +74,7 @@ CSRCS += up_createstack.c up_usestack.c up_releasestack.c up_stackframe.c
CSRCS += up_unblocktask.c up_blocktask.c up_releasepending.c
CSRCS += up_reprioritizertr.c up_exit.c up_schedulesigaction.c
CSRCS += up_allocateheap.c up_uart.c
+CSRCS += up_copyfullstate.c
VPATH = sim
DEPPATH = $(patsubst %,--dep-path %,$(subst :, ,$(VPATH)))
diff --git a/arch/sim/src/sim/up_blocktask.c b/arch/sim/src/sim/up_blocktask.c
index e81df88..8edf230 100644
--- a/arch/sim/src/sim/up_blocktask.c
+++ b/arch/sim/src/sim/up_blocktask.c
@@ -113,12 +113,19 @@ void up_block_task(struct tcb_s *tcb, tstate_t task_state)
nxsched_suspend_scheduler(rtcb);
+ /* TODO */
+
+ if (CURRENT_REGS)
+ {
+ ASSERT(false);
+ }
+
/* Copy the exception context into the TCB at the (old) head of the
* ready-to-run Task list. if up_setjmp returns a non-zero
* value, then this is really the previously running task restarting!
*/
- if (!up_setjmp(rtcb->xcp.regs))
+ else if (!up_setjmp(rtcb->xcp.regs))
{
/* Restore the exception context of the rtcb at the (new) head
* of the ready-to-run task list.
diff --git a/arch/sim/src/sim/up_copyfullstate.c b/arch/sim/src/sim/up_copyfullstate.c
new file mode 100644
index 0000000..25b2bc4
--- /dev/null
+++ b/arch/sim/src/sim/up_copyfullstate.c
@@ -0,0 +1,64 @@
+/****************************************************************************
+ * arch/sim/src/sim/sim_copyfullstate.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 <nuttx/config.h>
+
+#include <stdint.h>
+#include <arch/irq.h>
+
+#include "up_internal.h"
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: up_copyfullstate
+ *
+ * Description:
+ * Copy the entire register save area
+ *
+ ****************************************************************************/
+
+#if defined(CONFIG_HOST_X86_64) && !defined(CONFIG_SIM_M32)
+void up_copyfullstate(unsigned long *dest, unsigned long *src)
+#else
+void up_copyfullstate(uint32_t *dest, uint32_t *src)
+#endif
+{
+ int i;
+
+ /* In the sim model, the state is copied from the stack to the TCB,
+ * but only a reference is passed to get the state from the TCB. So the
+ * following check avoids copying the TCB save area onto itself:
+ */
+
+ if (src != dest)
+ {
+ for (i = 0; i < XCPTCONTEXT_REGS; i++)
+ {
+ *dest++ = *src++;
+ }
+ }
+}
diff --git a/arch/sim/src/sim/up_exit.c b/arch/sim/src/sim/up_exit.c
index 8cac071..400a3ee 100644
--- a/arch/sim/src/sim/up_exit.c
+++ b/arch/sim/src/sim/up_exit.c
@@ -67,6 +67,12 @@ void up_exit(int status)
{
FAR struct tcb_s *tcb;
+ /* Make sure that we are in a critical section with local interrupts.
+ * The IRQ state will be restored when the next task is started.
+ */
+
+ enter_critical_section();
+
sinfo("TCB=%p exiting\n", this_task());
/* Destroy the task at the head of the ready to run list. */
diff --git a/arch/sim/src/sim/up_hostirq.c b/arch/sim/src/sim/up_hostirq.c
index b929620..132bc9e 100644
--- a/arch/sim/src/sim/up_hostirq.c
+++ b/arch/sim/src/sim/up_hostirq.c
@@ -111,6 +111,11 @@ void up_irq_restore(uint64_t flags)
void up_irqinitialize(void)
{
+#ifdef CONFIG_SMP
+ /* Register the pause handler */
+
+ up_cpu_set_pause_handler(SIGUSR1);
+#endif
}
/****************************************************************************
diff --git a/arch/sim/src/sim/up_idle.c b/arch/sim/src/sim/up_idle.c
index a17d8d3..b5c5891 100644
--- a/arch/sim/src/sim/up_idle.c
+++ b/arch/sim/src/sim/up_idle.c
@@ -70,6 +70,7 @@
*
****************************************************************************/
+#ifndef CONFIG_SMP
void up_idle(void)
{
#ifdef CONFIG_PM
@@ -124,3 +125,30 @@ void up_idle(void)
up_timer_update();
#endif
}
+#endif /* !CONFIG_SMP */
+
+#ifdef CONFIG_SMP
+void up_idle(void)
+{
+ host_sleep(100 * 1000);
+}
+#endif
+
+/****************************************************************************
+ * Name: sim_timer_handler
+ ****************************************************************************/
+
+#ifdef CONFIG_SMP
+void sim_timer_handler(void)
+{
+ /* Handle UART data availability */
+
+ up_uartloop();
+
+#ifdef CONFIG_ONESHOT
+ /* Driver the simulated interval timer */
+
+ up_timer_update();
+#endif
+}
+#endif /* CONFIG_SMP */
diff --git a/arch/sim/src/sim/up_internal.h b/arch/sim/src/sim/up_internal.h
index 48ea33f..3367d78 100644
--- a/arch/sim/src/sim/up_internal.h
+++ b/arch/sim/src/sim/up_internal.h
@@ -128,6 +128,11 @@
#define SIM_HEAP_SIZE (64*1024*1024)
+/* Macros to handle saving and restoring interrupt state ********************/
+
+#define up_savestate(regs) up_copyfullstate(regs, (xcpt_reg_t *)CURRENT_REGS)
+#define up_restorestate(regs) (CURRENT_REGS = regs)
+
/* File System Definitions **************************************************/
/* These definitions characterize the compressed filesystem image */
@@ -188,29 +193,18 @@ extern volatile void *g_current_regs[1];
#endif
-#ifdef CONFIG_SMP
-/* These spinlocks are used in the SMP configuration in order to implement
- * up_cpu_pause(). The protocol for CPUn to pause CPUm is as follows
- *
- * 1. The up_cpu_pause() implementation on CPUn locks both g_cpu_wait[m]
- * and g_cpu_paused[m]. CPUn then waits spinning on g_cpu_paused[m].
- * 2. CPUm receives the interrupt it (1) unlocks g_cpu_paused[m] and
- * (2) locks g_cpu_wait[m]. The first unblocks CPUn and the second
- * blocks CPUm in the interrupt handler.
- *
- * When CPUm resumes, CPUn unlocks g_cpu_wait[m] and the interrupt handler
- * on CPUm continues. CPUm must, of course, also then unlock g_cpu_wait[m]
- * so that it will be ready for the next pause operation.
- */
-
-extern volatile uint8_t g_cpu_wait[CONFIG_SMP_NCPUS];
-extern volatile uint8_t g_cpu_paused[CONFIG_SMP_NCPUS];
-#endif
-
/****************************************************************************
* Public Function Prototypes
****************************************************************************/
+/* Context switching */
+
+#if defined(CONFIG_HOST_X86_64) && !defined(CONFIG_SIM_M32)
+void up_copyfullstate(unsigned long *dest, unsigned long *src);
+#else
+void up_copyfullstate(uint32_t *dest, uint32_t *src);
+#endif
+
void *up_doirq(int irq, void *regs);
/* up_setjmp32.S ************************************************************/
@@ -244,6 +238,9 @@ void up_cpu_started(void);
int up_cpu_paused(int cpu);
struct tcb_s *up_this_task(void);
int up_cpu_set_pause_handler(int irq);
+void sim_send_ipi(int cpu);
+void sim_timer_handler(void);
+void sim_sigdeliver(void);
#endif
/* up_oneshot.c *************************************************************/
diff --git a/arch/sim/src/sim/up_interruptcontext.c b/arch/sim/src/sim/up_interruptcontext.c
index 7a3a9e4..853a779 100644
--- a/arch/sim/src/sim/up_interruptcontext.c
+++ b/arch/sim/src/sim/up_interruptcontext.c
@@ -66,31 +66,53 @@ bool up_interrupt_context(void)
* Name: up_doirq
****************************************************************************/
-void *up_doirq(int irq, void *regs)
+void *up_doirq(int irq, void *context)
{
- /* Current regs non-zero indicates that we are processing an interrupt;
+ /* Allocate temporary context on the stack */
+
+ xcpt_reg_t tmp[XCPTCONTEXT_REGS];
+ void *regs = (void *)tmp;
+
+ /* CURRENT_REGS non-zero indicates that we are processing an interrupt.
* CURRENT_REGS is also used to manage interrupt level context switches.
*/
- CURRENT_REGS = regs;
+#ifdef CONFIG_SMP
+ if (up_setjmp(regs) == 0)
+ {
+#endif
- /* Deliver the IRQ */
+ CURRENT_REGS = regs;
- irq_dispatch(irq, regs);
+ /* Deliver the IRQ */
- /* If a context switch occurred while processing the interrupt then
- * CURRENT_REGS may have change value. If we return any value different
- * from the input regs, then the lower level will know that a context
- * switch occurred during interrupt processing.
- */
+ irq_dispatch(irq, regs);
- regs = (void *)CURRENT_REGS;
+ /* If a context switch occurred while processing the interrupt then
+ * CURRENT_REGS may have change value. If we return any value
+ * different from the input regs, then the lower level will know that
+ * context switch occurred during interrupt processing.
+ */
- /* Restore the previous value of CURRENT_REGS. NULL would indicate that
- * we are no longer in an interrupt handler. It will be non-NULL if we
- * are returning from a nested interrupt.
- */
+ regs = (void *)CURRENT_REGS;
+
+ /* Restore the previous value of CURRENT_REGS. NULL would indicate
+ * that we are no longer in an interrupt handler. It will be non-NULL
+ * if we are returning from a nested interrupt.
+ */
+
+ CURRENT_REGS = NULL;
+
+#ifdef CONFIG_SMP
+ /* Handle signal */
+
+ sim_sigdeliver();
+
+ /* Then switch contexts */
+
+ up_longjmp(regs, 1);
+ }
+#endif
- CURRENT_REGS = NULL;
return regs;
}
diff --git a/arch/sim/src/sim/up_releasepending.c b/arch/sim/src/sim/up_releasepending.c
index b1c1fb9..77210d2 100644
--- a/arch/sim/src/sim/up_releasepending.c
+++ b/arch/sim/src/sim/up_releasepending.c
@@ -82,12 +82,19 @@ void up_release_pending(void)
nxsched_suspend_scheduler(rtcb);
+ /* TODO */
+
+ if (CURRENT_REGS)
+ {
+ ASSERT(false);
+ }
+
/* Copy the exception context into the TCB of the task that was
* currently active. if up_setjmp returns a non-zero value, then
* this is really the previously running task restarting!
*/
- if (!up_setjmp(rtcb->xcp.regs))
+ else if (!up_setjmp(rtcb->xcp.regs))
{
/* Restore the exception context of the rtcb at the (new) head
* of the ready-to-run task list.
diff --git a/arch/sim/src/sim/up_reprioritizertr.c b/arch/sim/src/sim/up_reprioritizertr.c
index c4ce317..7d529d2 100644
--- a/arch/sim/src/sim/up_reprioritizertr.c
+++ b/arch/sim/src/sim/up_reprioritizertr.c
@@ -136,13 +136,20 @@ void up_reprioritize_rtr(struct tcb_s *tcb, uint8_t priority)
nxsched_suspend_scheduler(rtcb);
+ if (CURRENT_REGS)
+ {
+ /* TODO */
+
+ ASSERT(false);
+ }
+
/* Copy the exception context into the TCB at the (old) head of the
* ready-to-run Task list. if up_setjmp returns a non-zero
* value, then this is really the previously running task
* restarting!
*/
- if (!up_setjmp(rtcb->xcp.regs))
+ else if (!up_setjmp(rtcb->xcp.regs))
{
/* Restore the exception context of the rtcb at the (new) head
* of the ready-to-run task list.
diff --git a/arch/sim/src/sim/up_schedulesigaction.c b/arch/sim/src/sim/up_schedulesigaction.c
index f624e1b..b95d10e 100644
--- a/arch/sim/src/sim/up_schedulesigaction.c
+++ b/arch/sim/src/sim/up_schedulesigaction.c
@@ -86,8 +86,16 @@
void up_schedule_sigaction(struct tcb_s *tcb, sig_deliver_t sigdeliver)
{
+ irqstate_t flags;
+
/* We don't have to anything complex for the simulated target */
+ sinfo("tcb=0x%p sigdeliver=0x%p\n", tcb, sigdeliver);
+
+ /* Make sure that interrupts are disabled */
+
+ flags = enter_critical_section();
+
if (tcb == this_task())
{
sigdeliver(tcb);
@@ -96,4 +104,6 @@ void up_schedule_sigaction(struct tcb_s *tcb, sig_deliver_t sigdeliver)
{
tcb->xcp.sigdeliver = sigdeliver;
}
+
+ leave_critical_section(flags);
}
diff --git a/arch/sim/src/sim/up_simsmp.c b/arch/sim/src/sim/up_simsmp.c
index 478b4d7..bbe7f80 100644
--- a/arch/sim/src/sim/up_simsmp.c
+++ b/arch/sim/src/sim/up_simsmp.c
@@ -66,22 +66,7 @@ struct sim_cpuinfo_s
static pthread_key_t g_cpu_key;
static pthread_t g_cpu_thread[CONFIG_SMP_NCPUS];
-/* These spinlocks are used in the SMP configuration in order to implement
- * up_cpu_pause(). The protocol for CPUn to pause CPUm is as follows
- *
- * 1. The up_cpu_pause() implementation on CPUn locks both g_cpu_wait[m]
- * and g_cpu_paused[m]. CPUn then waits spinning on g_cpu_paused[m].
- * 2. CPUm receives the interrupt it (1) unlocks g_cpu_paused[m] and
- * (2) locks g_cpu_wait[m]. The first unblocks CPUn and the second
- * blocks CPUm in the interrupt handler.
- *
- * When CPUm resumes, CPUn unlocks g_cpu_wait[m] and the interrupt handler
- * on CPUm continues. CPUm must, of course, also then unlock g_cpu_wait[m]
- * so that it will be ready for the next pause operation.
- */
-
-volatile uint8_t g_cpu_wait[CONFIG_SMP_NCPUS];
-volatile uint8_t g_cpu_paused[CONFIG_SMP_NCPUS];
+static pthread_t g_timer_thread;
/****************************************************************************
* NuttX domain function prototypes
@@ -93,6 +78,10 @@ void sched_note_cpu_pause(struct tcb_s *tcb, int cpu);
void sched_note_cpu_resume(struct tcb_s *tcb, int cpu);
#endif
+void up_irqinitialize(void);
+
+extern uint8_t g_nx_initstate;
+
/****************************************************************************
* Private Functions
****************************************************************************/
@@ -134,9 +123,9 @@ static void *sim_idle_trampoline(void *arg)
return NULL;
}
- /* Make sure the SIGUSR1 is not masked */
+ /* Initialize IRQ */
- up_cpu_set_pause_handler(SIGUSR1);
+ up_irqinitialize();
/* Let up_cpu_start() continue */
@@ -173,6 +162,30 @@ static void *sim_idle_trampoline(void *arg)
}
/****************************************************************************
+ * Name: sim_host_timer_handler
+ ****************************************************************************/
+
+static void *sim_host_timer_handler(void *arg)
+{
+ /* Wait until OSINIT_OSREADY(5) */
+
+ while (g_nx_initstate < 5)
+ {
+ host_sleep(10 * 1000); /* 10ms */
+ }
+
+ /* Send a periodic timer event to CPU0 */
+
+ while (1)
+ {
+ pthread_kill(g_cpu_thread[0], SIGUSR1);
+ host_sleep(10 * 1000); /* 10ms */
+ }
+
+ return NULL;
+}
+
+/****************************************************************************
* Public Functions
****************************************************************************/
@@ -213,9 +226,12 @@ void sim_cpu0_start(void)
return;
}
- /* Register the common signal handler for all threads */
+ /* NOTE: IRQ initialization will be done in up_irqinitialize */
- up_cpu_set_pause_handler(SIGUSR1);
+ /* Create timer thread to send a periodic timer event */
+
+ ret = pthread_create(&g_timer_thread,
+ NULL, sim_host_timer_handler, NULL);
}
/****************************************************************************
@@ -316,81 +332,10 @@ errout_with_cond:
}
/****************************************************************************
- * Name: up_cpu_pause
- *
- * Description:
- * Save the state of the current task at the head of the
- * g_assignedtasks[cpu] task list and then pause task execution on the
- * CPU.
- *
- * This function is called by the OS when the logic executing on one CPU
- * needs to modify the state of the g_assignedtasks[cpu] list for another
- * CPU.
- *
- * Input Parameters:
- * cpu - The index of the CPU to be stopped/
- *
- * Returned Value:
- * Zero on success; a negated errno value on failure.
- *
+ * Name: sim_send_ipi(int cpu)
****************************************************************************/
-int up_cpu_pause(int cpu)
+void sim_send_ipi(int cpu)
{
-#ifdef CONFIG_SCHED_INSTRUMENTATION
- /* Notify of the pause event */
-
- sched_note_cpu_pause(up_this_task(), cpu);
-#endif
-
- /* Take the spinlock that will prevent the CPU thread from running */
-
- g_cpu_wait[cpu] = 1;
- g_cpu_paused[cpu] = 1;
-
- /* Signal the CPU thread */
-
pthread_kill(g_cpu_thread[cpu], SIGUSR1);
-
- /* Spin, waiting for the thread to pause */
-
- while (g_cpu_paused[cpu] != 0)
- {
- sched_yield();
- }
-
- return 0;
-}
-
-/****************************************************************************
- * Name: up_cpu_resume
- *
- * Description:
- * Restart the cpu after it was paused via up_cpu_pause(), restoring the
- * state of the task at the head of the g_assignedtasks[cpu] list, and
- * resume normal tasking.
- *
- * This function is called after up_cpu_pause in order resume operation of
- * the CPU after modifying its g_assignedtasks[cpu] list.
- *
- * Input Parameters:
- * cpu - The index of the CPU being re-started.
- *
- * Returned Value:
- * Zero on success; a negated errno value on failure.
- *
- ****************************************************************************/
-
-int up_cpu_resume(int cpu)
-{
-#ifdef CONFIG_SCHED_INSTRUMENTATION
- /* Notify of the resume event */
-
- sched_note_cpu_resume(up_this_task(), cpu);
-#endif
-
- /* Release the spinlock that will alloc the CPU thread to continue */
-
- g_cpu_wait[cpu] = 0;
- return 0;
}
diff --git a/arch/sim/src/sim/up_smpsignal.c b/arch/sim/src/sim/up_smpsignal.c
index 7d9baca..86cf1e7 100644
--- a/arch/sim/src/sim/up_smpsignal.c
+++ b/arch/sim/src/sim/up_smpsignal.c
@@ -46,6 +46,23 @@
#include "sched/sched.h"
#include "up_internal.h"
+/* These spinlocks are used in the SMP configuration in order to implement
+ * up_cpu_pause(). The protocol for CPUn to pause CPUm is as follows
+ *
+ * 1. The up_cpu_pause() implementation on CPUn locks both g_cpu_wait[m]
+ * and g_cpu_paused[m]. CPUn then waits spinning on g_cpu_paused[m].
+ * 2. CPUm receives the interrupt it (1) unlocks g_cpu_paused[m] and
+ * (2) locks g_cpu_wait[m]. The first unblocks CPUn and the second
+ * blocks CPUm in the interrupt handler.
+ *
+ * When CPUm resumes, CPUn unlocks g_cpu_wait[m] and the interrupt handler
+ * on CPUm continues. CPUm must, of course, also then unlock g_cpu_wait[m]
+ * so that it will be ready for the next pause operation.
+ */
+
+static volatile spinlock_t g_cpu_wait[CONFIG_SMP_NCPUS];
+static volatile spinlock_t g_cpu_paused[CONFIG_SMP_NCPUS];
+
/****************************************************************************
* Private Functions
****************************************************************************/
@@ -69,7 +86,41 @@
static int sim_cpupause_handler(int irq, FAR void *context, FAR void *arg)
{
- return up_cpu_paused(this_cpu());
+ int cpu = this_cpu();
+
+ /* Check for false alarms. Such false could occur as a consequence of
+ * some deadlock breaking logic that might have already serviced the SG2
+ * interrupt by calling up_cpu_paused(). If the pause event has already
+ * been processed then g_cpu_paused[cpu] will not be locked.
+ */
+
+ if (up_cpu_pausereq(cpu))
+ {
+ /* NOTE: The following enter_critical_section() will call
+ * up_cpu_paused() to process a pause request to break a deadlock
+ * because the caller held a critical section. Once up_cpu_paused()
+ * finished, the caller will proceed and release the g_cpu_irqlock.
+ * Then this CPU will acquire g_cpu_irqlock in the function.
+ */
+
+ irqstate_t flags = enter_critical_section();
+
+ /* NOTE: the pause request should not exist here */
+
+ DEBUGVERIFY(!up_cpu_pausereq(cpu));
+
+ leave_critical_section(flags);
+ }
+ else
+ {
+ /* NOTE: sim specific logic
+ * In the case of no pause request, call sim_timer_handler()
+ */
+
+ sim_timer_handler();
+ }
+
+ return OK;
}
/****************************************************************************
@@ -125,72 +176,56 @@ bool up_cpu_pausereq(int cpu)
int up_cpu_paused(int cpu)
{
- struct tcb_s *rtcb = current_task(cpu);
+ struct tcb_s *tcb = current_task(cpu);
/* Update scheduler parameters */
- nxsched_suspend_scheduler(rtcb);
+ nxsched_suspend_scheduler(tcb);
#ifdef CONFIG_SCHED_INSTRUMENTATION
/* Notify that we are paused */
- sched_note_cpu_paused(rtcb);
+ sched_note_cpu_paused(tcb);
#endif
- /* Copy the exception context into the TCB at the (old) head of the
- * CPUs assigned task list. if up_setjmp returns a non-zero value, then
- * this is really the previously running task restarting!
+ /* Save the current context at CURRENT_REGS into the TCB at the head
+ * of the assigned task list for this CPU.
*/
- if (up_setjmp(rtcb->xcp.regs) == 0)
- {
- /* Unlock the g_cpu_paused spinlock to indicate that we are in the
- * paused state
- */
-
- spin_unlock(&g_cpu_paused[cpu]);
+ up_savestate(tcb->xcp.regs);
- /* Spin until we are asked to resume. When we resume, we need to
- * inicate that we are not longer paused.
- */
+ /* Wait for the spinlock to be released */
- spin_lock(&g_cpu_wait[cpu]);
- spin_unlock(&g_cpu_wait[cpu]);
+ spin_unlock(&g_cpu_paused[cpu]);
+ spin_lock(&g_cpu_wait[cpu]);
- /* While we were paused, logic on a different CPU probably changed
- * the task as that head of the assigned task list. So now we need
- * restore the exception context of the rtcb at the (new) head
- * of the assigned list in order to instantiate the new task.
- */
-
- rtcb = current_task(cpu);
+ /* Restore the exception context of the tcb at the (new) head of the
+ * assigned task list.
+ */
- /* The way that we handle signals in the simulation is kind of a
- * kludge. This would be unsafe in a truly multi-threaded,
- * interrupt driven environment.
- */
+ tcb = current_task(cpu);
- if (rtcb->xcp.sigdeliver)
- {
- sinfo("CPU%d: Delivering signals TCB=%p\n", cpu, rtcb);
- ((sig_deliver_t)rtcb->xcp.sigdeliver)(rtcb);
- rtcb->xcp.sigdeliver = NULL;
- }
+ /* The way that we handle signals in the simulation is kind of a
+ * kludge. This would be unsafe in a truly multi-threaded,
+ * interrupt driven environment.
+ */
#ifdef CONFIG_SCHED_INSTRUMENTATION
- /* Notify that we have resumed */
+ /* Notify that we have resumed */
- sched_note_cpu_resumed(rtcb);
+ sched_note_cpu_resumed(tcb);
#endif
- /* Reset scheduler parameters */
+ /* Reset scheduler parameters */
- nxsched_resume_scheduler(rtcb);
+ nxsched_resume_scheduler(tcb);
- /* Then switch contexts */
+ /* Then switch contexts. Any necessary address environment changes
+ * will be made when the interrupt returns.
+ */
- up_longjmp(rtcb->xcp.regs, 1);
- }
+ up_restorestate(tcb->xcp.regs);
+ spin_unlock(&g_cpu_wait[cpu]);
return OK;
}
@@ -249,3 +284,122 @@ int up_cpu_set_pause_handler(int irq)
up_enable_irq(irq);
return irq_attach(irq, sim_cpupause_handler, NULL);
}
+
+/****************************************************************************
+ * Name: up_cpu_pause
+ *
+ * Description:
+ * Save the state of the current task at the head of the
+ * g_assignedtasks[cpu] task list and then pause task execution on the
+ * CPU.
+ *
+ * This function is called by the OS when the logic executing on one CPU
+ * needs to modify the state of the g_assignedtasks[cpu] list for another
+ * CPU.
+ *
+ * Input Parameters:
+ * cpu - The index of the CPU to be stopped
+ *
+ * Returned Value:
+ * Zero on success; a negated errno value on failure.
+ *
+ ****************************************************************************/
+
+int up_cpu_pause(int cpu)
+{
+ DEBUGASSERT(cpu >= 0 && cpu < CONFIG_SMP_NCPUS && cpu != this_cpu());
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION
+ /* Notify of the pause event */
+
+ sched_note_cpu_pause(this_task(), cpu);
+#endif
+
+ /* Take the both spinlocks. The g_cpu_wait spinlock will prevent the
+ * handler from returning until up_cpu_resume() is called; g_cpu_paused
+ * is a handshake that will prefent this function from returning until
+ * the CPU is actually paused.
+ */
+
+ DEBUGASSERT(!spin_islocked(&g_cpu_wait[cpu]) &&
+ !spin_islocked(&g_cpu_paused[cpu]));
+
+ spin_lock(&g_cpu_wait[cpu]);
+ spin_lock(&g_cpu_paused[cpu]);
+
+ /* Generate IRQ for CPU(cpu) */
+
+ sim_send_ipi(cpu);
+
+ /* Wait for the other CPU to unlock g_cpu_paused meaning that
+ * it is fully paused and ready for up_cpu_resume();
+ */
+
+ spin_lock(&g_cpu_paused[cpu]);
+ spin_unlock(&g_cpu_paused[cpu]);
+
+ /* On successful return g_cpu_wait will be locked, the other CPU will be
+ * spinning on g_cpu_wait and will not continue until g_cpu_resume() is
+ * called. g_cpu_paused will be unlocked in any case.
+ */
+
+ return OK;
+}
+
+/****************************************************************************
+ * Name: up_cpu_resume
+ *
+ * Description:
+ * Restart the cpu after it was paused via up_cpu_pause(), restoring the
+ * state of the task at the head of the g_assignedtasks[cpu] list, and
+ * resume normal tasking.
+ *
+ * This function is called after up_cpu_pause in order resume operation of
+ * the CPU after modifying its g_assignedtasks[cpu] list.
+ *
+ * Input Parameters:
+ * cpu - The index of the CPU being re-started.
+ *
+ * Returned Value:
+ * Zero on success; a negated errno value on failure.
+ *
+ ****************************************************************************/
+
+int up_cpu_resume(int cpu)
+{
+ DEBUGASSERT(cpu >= 0 && cpu < CONFIG_SMP_NCPUS && cpu != this_cpu());
+
+#ifdef CONFIG_SCHED_INSTRUMENTATION
+ /* Notify of the resume event */
+
+ sched_note_cpu_resume(this_task(), cpu);
+#endif
+
+ /* Release the spinlock. Releasing the spinlock will cause the SGI2
+ * handler on 'cpu' to continue and return from interrupt to the newly
+ * established thread.
+ */
+
+ DEBUGASSERT(spin_islocked(&g_cpu_wait[cpu]) &&
+ !spin_islocked(&g_cpu_paused[cpu]));
+
+ spin_unlock(&g_cpu_wait[cpu]);
+ return OK;
+}
+
+/****************************************************************************
+ * Name: sim_sigdeliver
+ ****************************************************************************/
+
+void sim_sigdeliver(void)
+{
+ int cpu = this_cpu();
+ struct tcb_s *tcb = current_task(cpu);
+
+ if (tcb->xcp.sigdeliver)
+ {
+ sinfo("Delivering signals TCB=%p\n", tcb);
+ ((sig_deliver_t)tcb->xcp.sigdeliver)(tcb);
+ tcb->xcp.sigdeliver = NULL;
+ }
+}
diff --git a/arch/sim/src/sim/up_unblocktask.c b/arch/sim/src/sim/up_unblocktask.c
index 5f739bc..341608c 100644
--- a/arch/sim/src/sim/up_unblocktask.c
+++ b/arch/sim/src/sim/up_unblocktask.c
@@ -96,12 +96,37 @@ void up_unblock_task(FAR struct tcb_s *tcb)
nxsched_suspend_scheduler(rtcb);
+ /* Are we in an interrupt handler? */
+
+ if (CURRENT_REGS)
+ {
+ /* Yes, then we have to do things differently.
+ * Just copy the CURRENT_REGS into the OLD rtcb.
+ */
+
+ up_savestate(rtcb->xcp.regs);
+
+ /* Restore the exception context of the rtcb at the (new) head
+ * of the ready-to-run task list.
+ */
+
+ rtcb = this_task();
+
+ /* Update scheduler parameters */
+
+ nxsched_resume_scheduler(rtcb);
+
+ /* Then switch contexts */
+
+ up_restorestate(rtcb->xcp.regs);
+ }
+
/* Copy the exception context into the TCB of the task that was
* previously active. if up_setjmp returns a non-zero value, then
* this is really the previously running task restarting!
*/
- if (!up_setjmp(rtcb->xcp.regs))
+ else if (!up_setjmp(rtcb->xcp.regs))
{
/* 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
diff --git a/boards/sim/sim/sim/configs/smp/defconfig b/boards/sim/sim/sim/configs/smp/defconfig
index 8c916e7..cbc3554 100644
--- a/boards/sim/sim/sim/configs/smp/defconfig
+++ b/boards/sim/sim/sim/configs/smp/defconfig
@@ -13,6 +13,9 @@ CONFIG_ARCH_CHIP="sim"
CONFIG_ARCH_SIM=y
CONFIG_BOARDCTL_POWEROFF=y
CONFIG_BUILTIN=y
+CONFIG_DEBUG_ASSERTIONS=y
+CONFIG_DEBUG_ERROR=y
+CONFIG_DEBUG_FEATURES=y
CONFIG_DEBUG_SYMBOLS=y
CONFIG_DRIVER_NOTE=y
CONFIG_EXAMPLES_HELLO=y
@@ -23,9 +26,11 @@ CONFIG_NSH_READLINE=y
CONFIG_READLINE_CMD_HISTORY=y
CONFIG_SCHED_HAVE_PARENT=y
CONFIG_SCHED_INSTRUMENTATION=y
-CONFIG_SCHED_WAITPID=y
CONFIG_SMP=y
CONFIG_SYSTEM_NSH=y
+CONFIG_SYSTEM_SYSTEM=y
+CONFIG_SYSTEM_TASKSET=y
+CONFIG_TESTING_GETPRIME=y
CONFIG_TESTING_OSTEST=y
CONFIG_TESTING_SMP=y
CONFIG_USER_ENTRYPOINT="nsh_main"
[incubator-nuttx] 02/04: arch,
sched: Fix global IRQ control logics for SMP
Posted by je...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
jerpelea pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-nuttx.git
commit 409c65ce0bc27df685bd595f5100ad03692d674a
Author: Masayuki Ishikawa <ma...@gmail.com>
AuthorDate: Wed Nov 25 11:18:24 2020 +0900
arch, sched: Fix global IRQ control logics for SMP
Summary:
- This commit fixes global IRQ control logic
- In previous implementation, g_cpu_irqset for a remote CPU was
set in sched_add_readytorun(), sched_remove_readytorun() and
up_schedule_sigaction()
- In this implementation, they are removed.
- Instead, in the pause handler, call enter_critical_setion()
which will call up_cpu_paused() then acquire g_cpu_irqlock
- So if a new task with irqcount > 1 restarts on the remote CPU,
the CPU will only hold a critical section. Thus, the issue such as
'POSSIBLE FOR TWO CPUs TO HOLD A CRITICAL SECTION' could be resolved.
- Fix nxsched_resume_scheduler() so that it does not call spin_clrbit()
if a CPU does not hold a g_cpu_irqset
- Fix nxtask_exit() so that it acquires g_cpu_irqlock
- Update TODO
Impact:
- All SMP implementations
Testing:
- Tested with smp, ostest with the following configurations
- Tested with spresense:wifi_smp (NCPUS=2,4)
- Tested with sabre-6quad:smp (QEMU, dev board)
- Tested with maix-bit:smp (QEMU)
- Tested with esp32-core:smp (QEMU)
- Tested with lc823450-xgevk:rndis
Signed-off-by: Masayuki Ishikawa <Ma...@jp.sony.com>
---
TODO | 68 +-------------------------
arch/arm/src/armv7-a/arm_cpupause.c | 17 ++++++-
arch/arm/src/armv7-a/arm_schedulesigaction.c | 14 ++----
arch/arm/src/armv7-m/arm_schedulesigaction.c | 14 ++----
arch/arm/src/cxd56xx/cxd56_cpupause.c | 32 ++++++++++--
arch/arm/src/lc823450/lc823450_cpupause.c | 17 ++++++-
arch/risc-v/src/k210/k210_cpupause.c | 17 ++++++-
arch/risc-v/src/k210/k210_schedulesigaction.c | 14 ++----
arch/xtensa/src/common/xtensa_cpupause.c | 17 ++++++-
arch/xtensa/src/common/xtensa_schedsigaction.c | 14 ++----
sched/sched/sched_addreadytorun.c | 43 ++--------------
sched/sched/sched_removereadytorun.c | 43 ++--------------
sched/sched/sched_resumescheduler.c | 7 ++-
sched/task/task_exit.c | 7 +++
14 files changed, 131 insertions(+), 193 deletions(-)
diff --git a/TODO b/TODO
index acc6e5b..00bb931 100644
--- a/TODO
+++ b/TODO
@@ -1,4 +1,4 @@
-NuttX TODO List (Last updated November 20, 2020)
+NuttX TODO List (Last updated November 26, 2020)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This file summarizes known NuttX bugs, limitations, inconsistencies with
@@ -10,7 +10,7 @@ issues related to each board port.
nuttx/:
(16) Task/Scheduler (sched/)
- (3) SMP
+ (2) SMP
(1) Memory Management (mm/)
(0) Power Management (drivers/pm)
(5) Signals (sched/signal, arch/)
@@ -485,70 +485,6 @@ o SMP
an bugs caused by this. But I believe that failures are
possible.
- Title: POSSIBLE FOR TWO CPUs TO HOLD A CRITICAL SECTION?
- Description: The SMP design includes logic that will support multiple
- CPUs holding a critical section. Is this necessary? How
- can that occur? I think it can occur in the following
- situation:
-
- The log below was reported is NuttX running on two cores
- Cortex-A7 architecture in SMP mode. You can notice see that
- when nxsched_add_readytorun() was called, the g_cpu_irqset is 3.
-
- nxsched_add_readytorun: irqset cpu 1, me 0 btcbname init, irqset 1 irqcount 2.
- nxsched_add_readytorun: nxsched_add_readytorun line 338 g_cpu_irqset = 3.
-
- This can happen, but only under a very certain condition.
- g_cpu_irqset only exists to support this certain condition:
-
- a. A task running on CPU 0 takes the critical section. So
- g_cpu_irqset == 0x1.
-
- b. A task exits on CPU 1 and a waiting, ready-to-run task
- is re-started on CPU 1. This new task also holds the
- critical section. So when the task is re-restarted on
- CPU 1, we than have g_cpu_irqset == 0x3
-
- So we are in a very perverse state! There are two tasks
- running on two different CPUs and both hold the critical
- section. I believe that is a dangerous situation and there
- could be undiscovered bugs that could happen in that case.
- However, as of this moment, I have not heard of any specific
- problems caused by this weird behavior.
-
- A possible solution would be to add a new task state that
- would exist only for SMP.
-
- - Add a new SMP-only task list and state. Say,
- g_csection_wait[]. It should be prioritized.
- - When a task acquires the critical section, all tasks in
- g_readytorun[] that need the critical section would be
- moved to g_csection_wait[].
- - When any task is unblocked for any reason and moved to the
- g_readytorun[] list, if that unblocked task needs the
- critical section, it would also be moved to the
- g_csection_wait[] list. No task that needs the critical
- section can be in the ready-to-run list if the critical
- section is not available.
- - When the task releases the critical section, all tasks in
- the g_csection_wait[] needs to be moved back to
- g_readytorun[].
- - This may result in a context switch. The tasks should be
- moved back to g_readytorun[] highest priority first. If a
- context switch occurs and the critical section to re-taken
- by the re-started task, the lower priority tasks in
- g_csection_wait[] must stay in that list.
-
- That is really not as much work as it sounds. It is
- something that could be done in 2-3 days of work if you know
- what you are doing. Getting the proper test setup and
- verifying the change would be the more difficult task.
-
-Status: Open
-Priority: Unknown. Might be high, but first we would need to confirm
- that this situation can occur and that is actually causes
- a failure.
-
o Memory Management (mm/)
^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/arch/arm/src/armv7-a/arm_cpupause.c b/arch/arm/src/armv7-a/arm_cpupause.c
index 4725418..8b2c56f 100644
--- a/arch/arm/src/armv7-a/arm_cpupause.c
+++ b/arch/arm/src/armv7-a/arm_cpupause.c
@@ -212,9 +212,22 @@ int arm_pause_handler(int irq, FAR void *context, FAR void *arg)
* been processed then g_cpu_paused[cpu] will not be locked.
*/
- if (spin_islocked(&g_cpu_paused[cpu]))
+ if (up_cpu_pausereq(cpu))
{
- return up_cpu_paused(cpu);
+ /* NOTE: The following enter_critical_section() will call
+ * up_cpu_paused() to process a pause request to break a deadlock
+ * because the caller held a critical section. Once up_cpu_paused()
+ * finished, the caller will proceed and release the g_cpu_irqlock.
+ * Then this CPU will acquire g_cpu_irqlock in the function.
+ */
+
+ irqstate_t flags = enter_critical_section();
+
+ /* NOTE: the pause request should not exist here */
+
+ DEBUGVERIFY(!up_cpu_pausereq(cpu));
+
+ leave_critical_section(flags);
}
return OK;
diff --git a/arch/arm/src/armv7-a/arm_schedulesigaction.c b/arch/arm/src/armv7-a/arm_schedulesigaction.c
index 424ba0d..8b44b32 100644
--- a/arch/arm/src/armv7-a/arm_schedulesigaction.c
+++ b/arch/arm/src/armv7-a/arm_schedulesigaction.c
@@ -314,17 +314,13 @@ void up_schedule_sigaction(struct tcb_s *tcb, sig_deliver_t sigdeliver)
DEBUGASSERT(tcb->irqcount < INT16_MAX);
tcb->irqcount++;
- /* In an SMP configuration, the interrupt disable logic also
- * involves spinlocks that are configured per the TCB irqcount
- * field. This is logically equivalent to
- * enter_critical_section(). The matching call to
- * leave_critical_section() will be performed in
- * arm_sigdeliver().
+ /* NOTE: If the task runs on another CPU(cpu), adjusting
+ * global IRQ controls will be done in the pause handler
+ * on the CPU(cpu) by taking a critical section.
+ * If the task is scheduled on this CPU(me), do nothing
+ * because this CPU already took a critical section
*/
- spin_setbit(&g_cpu_irqset, cpu, &g_cpu_irqsetlock,
- &g_cpu_irqlock);
-
/* RESUME the other CPU if it was PAUSED */
if (cpu != me)
diff --git a/arch/arm/src/armv7-m/arm_schedulesigaction.c b/arch/arm/src/armv7-m/arm_schedulesigaction.c
index 96aaf3f..7376939 100644
--- a/arch/arm/src/armv7-m/arm_schedulesigaction.c
+++ b/arch/arm/src/armv7-m/arm_schedulesigaction.c
@@ -362,17 +362,13 @@ void up_schedule_sigaction(struct tcb_s *tcb, sig_deliver_t sigdeliver)
DEBUGASSERT(tcb->irqcount < INT16_MAX);
tcb->irqcount++;
- /* In an SMP configuration, the interrupt disable logic also
- * involves spinlocks that are configured per the TCB irqcount
- * field. This is logically equivalent to
- * enter_critical_section(). The matching call to
- * leave_critical_section() will be performed in
- * arm_sigdeliver().
+ /* NOTE: If the task runs on another CPU(cpu), adjusting
+ * global IRQ controls will be done in the pause handler
+ * on the CPU(cpu) by taking a critical section.
+ * If the task is scheduled on this CPU(me), do nothing
+ * because this CPU already took a critical section
*/
- spin_setbit(&g_cpu_irqset, cpu, &g_cpu_irqsetlock,
- &g_cpu_irqlock);
-
/* RESUME the other CPU if it was PAUSED */
if (cpu != me)
diff --git a/arch/arm/src/cxd56xx/cxd56_cpupause.c b/arch/arm/src/cxd56xx/cxd56_cpupause.c
index c8bd410..925450a 100644
--- a/arch/arm/src/cxd56xx/cxd56_cpupause.c
+++ b/arch/arm/src/cxd56xx/cxd56_cpupause.c
@@ -283,6 +283,7 @@ int up_cpu_paused(int cpu)
int arm_pause_handler(int irq, void *c, FAR void *arg)
{
int cpu = up_cpu_index();
+ int ret = OK;
DPRINTF("cpu%d will be paused \n", cpu);
@@ -295,12 +296,37 @@ int arm_pause_handler(int irq, void *c, FAR void *arg)
* interrupt by calling up_cpu_paused.
*/
- if (spin_islocked(&g_cpu_paused[cpu]))
+ if (up_cpu_pausereq(cpu))
{
- return up_cpu_paused(cpu);
+ /* NOTE: The following enter_critical_section() would call
+ * up_cpu_paused() to process a pause request to break a deadlock
+ * because the caller held a critical section. Once up_cpu_paused()
+ * finished, the caller will proceed and release the g_cpu_irqlock.
+ * Then this CPU will acquire g_cpu_irqlock in the function.
+ */
+
+ irqstate_t flags = enter_critical_section();
+
+ /* NOTE: Normally, we do not call up_cpu_paused() here because
+ * the above enter_critical_setion() would call up_cpu_paused()
+ * inside because the caller holds a crtical section.
+ * Howerver, cxd56's remote IRQ control logic also uses this handler
+ * and a caller might not take a critical section to avoid a deadlock
+ * during up_enable_irq() and up_disable_irq(). This is allowed
+ * because IRQ control logic does not interact wtih the scheduler.
+ * This means that if the request was not handled above, we need
+ * to call up_cpu_puased() here again.
+ */
+
+ if (up_cpu_pausereq(cpu))
+ {
+ ret = up_cpu_paused(cpu);
+ }
+
+ leave_critical_section(flags);
}
- return OK;
+ return ret;
}
/****************************************************************************
diff --git a/arch/arm/src/lc823450/lc823450_cpupause.c b/arch/arm/src/lc823450/lc823450_cpupause.c
index 2714fc3..af8165c 100644
--- a/arch/arm/src/lc823450/lc823450_cpupause.c
+++ b/arch/arm/src/lc823450/lc823450_cpupause.c
@@ -209,9 +209,22 @@ int lc823450_pause_handler(int irq, void *c, FAR void *arg)
* interrupt by calling up_cpu_paused.
*/
- if (spin_islocked(&g_cpu_paused[cpu]))
+ if (up_cpu_pausereq(cpu))
{
- return up_cpu_paused(cpu);
+ /* NOTE: The following enter_critical_section() will call
+ * up_cpu_paused() to process a pause request to break a deadlock
+ * because the caller held a critical section. Once up_cpu_paused()
+ * finished, the caller will proceed and release the g_cpu_irqlock.
+ * Then this CPU will acquire g_cpu_irqlock in the function.
+ */
+
+ irqstate_t flags = enter_critical_section();
+
+ /* NOTE: the pause request should not exist here */
+
+ DEBUGVERIFY(!up_cpu_pausereq(cpu));
+
+ leave_critical_section(flags);
}
return OK;
diff --git a/arch/risc-v/src/k210/k210_cpupause.c b/arch/risc-v/src/k210/k210_cpupause.c
index 88af783..957d115 100644
--- a/arch/risc-v/src/k210/k210_cpupause.c
+++ b/arch/risc-v/src/k210/k210_cpupause.c
@@ -215,9 +215,22 @@ int riscv_pause_handler(int irq, void *c, FAR void *arg)
* interrupt by calling up_cpu_paused.
*/
- if (spin_islocked(&g_cpu_paused[cpu]))
+ if (up_cpu_pausereq(cpu))
{
- return up_cpu_paused(cpu);
+ /* NOTE: The following enter_critical_section() will call
+ * up_cpu_paused() to process a pause request to break a deadlock
+ * because the caller held a critical section. Once up_cpu_paused()
+ * finished, the caller will proceed and release the g_cpu_irqlock.
+ * Then this CPU will acquire g_cpu_irqlock in the function.
+ */
+
+ irqstate_t flags = enter_critical_section();
+
+ /* NOTE: the pause request should not exist here */
+
+ DEBUGVERIFY(!up_cpu_pausereq(cpu));
+
+ leave_critical_section(flags);
}
return OK;
diff --git a/arch/risc-v/src/k210/k210_schedulesigaction.c b/arch/risc-v/src/k210/k210_schedulesigaction.c
index 26a6c77..05aabbd 100644
--- a/arch/risc-v/src/k210/k210_schedulesigaction.c
+++ b/arch/risc-v/src/k210/k210_schedulesigaction.c
@@ -353,17 +353,13 @@ void up_schedule_sigaction(struct tcb_s *tcb, sig_deliver_t sigdeliver)
DEBUGASSERT(tcb->irqcount < INT16_MAX);
tcb->irqcount++;
- /* In an SMP configuration, the interrupt disable logic also
- * involves spinlocks that are configured per the TCB irqcount
- * field. This is logically equivalent to
- * enter_critical_section(). The matching call to
- * leave_critical_section() will be performed in
- * up_sigdeliver().
+ /* NOTE: If the task runs on another CPU(cpu), adjusting
+ * global IRQ controls will be done in the pause handler
+ * on the CPU(cpu) by taking a critical section.
+ * If the task is scheduled on this CPU(me), do nothing
+ * because this CPU already took a critical section
*/
- spin_setbit(&g_cpu_irqset, cpu, &g_cpu_irqsetlock,
- &g_cpu_irqlock);
-
/* RESUME the other CPU if it was PAUSED */
if (cpu != me)
diff --git a/arch/xtensa/src/common/xtensa_cpupause.c b/arch/xtensa/src/common/xtensa_cpupause.c
index 5b59200..7c58176 100644
--- a/arch/xtensa/src/common/xtensa_cpupause.c
+++ b/arch/xtensa/src/common/xtensa_cpupause.c
@@ -190,9 +190,22 @@ void xtensa_pause_handler(void)
* interrupt by calling up_cpu_paused.
*/
- if (spin_islocked(&g_cpu_paused[cpu]))
+ if (up_cpu_pausereq(cpu))
{
- up_cpu_paused(cpu);
+ /* NOTE: The following enter_critical_section() will call
+ * up_cpu_paused() to process a pause request to break a deadlock
+ * because the caller held a critical section. Once up_cpu_paused()
+ * finished, the caller will proceed and release the g_cpu_irqlock.
+ * Then this CPU will acquire g_cpu_irqlock in the function.
+ */
+
+ irqstate_t flags = enter_critical_section();
+
+ /* NOTE: the pause request should not exist here */
+
+ DEBUGVERIFY(!up_cpu_pausereq(cpu));
+
+ leave_critical_section(flags);
}
}
diff --git a/arch/xtensa/src/common/xtensa_schedsigaction.c b/arch/xtensa/src/common/xtensa_schedsigaction.c
index 9888557..62f4bfd 100644
--- a/arch/xtensa/src/common/xtensa_schedsigaction.c
+++ b/arch/xtensa/src/common/xtensa_schedsigaction.c
@@ -339,17 +339,13 @@ void up_schedule_sigaction(struct tcb_s *tcb, sig_deliver_t sigdeliver)
DEBUGASSERT(tcb->irqcount < INT16_MAX);
tcb->irqcount++;
- /* In an SMP configuration, the interrupt disable logic also
- * involves spinlocks that are configured per the TCB irqcount
- * field. This is logically equivalent to
- * enter_critical_section().
- * The matching call to leave_critical_section() will be
- * performed in up_sigdeliver().
+ /* NOTE: If the task runs on another CPU(cpu), adjusting
+ * global IRQ controls will be done in the pause handler
+ * on the CPU(cpu) by taking a critical section.
+ * If the task is scheduled on this CPU(me), do nothing
+ * because this CPU already took a critical section
*/
- spin_setbit(&g_cpu_irqset, cpu, &g_cpu_irqsetlock,
- &g_cpu_irqlock);
-
/* RESUME the other CPU if it was PAUSED */
if (cpu != me)
diff --git a/sched/sched/sched_addreadytorun.c b/sched/sched/sched_addreadytorun.c
index 0d610b5..15077d9 100644
--- a/sched/sched/sched_addreadytorun.c
+++ b/sched/sched/sched_addreadytorun.c
@@ -318,47 +318,12 @@ bool nxsched_add_readytorun(FAR struct tcb_s *btcb)
&g_cpu_schedlock);
}
- /* Adjust global IRQ controls. If irqcount is greater than zero,
- * then this task/this CPU holds the IRQ lock
+ /* NOTE: If the task runs on another CPU(cpu), adjusting global IRQ
+ * controls will be done in the pause handler on the new CPU(cpu).
+ * If the task is scheduled on this CPU(me), do nothing because
+ * this CPU already has a critical section
*/
- if (btcb->irqcount > 0)
- {
- /* Yes... make sure that scheduling logic on other CPUs knows
- * that we hold the IRQ lock.
- */
-
- spin_setbit(&g_cpu_irqset, cpu, &g_cpu_irqsetlock,
- &g_cpu_irqlock);
- }
-
- /* No.. This CPU will be relinquishing the lock. But this works
- * differently if we are performing a context switch from an
- * interrupt handler and the interrupt handler has established
- * a critical section. We can detect this case when
- * g_cpu_nestcount[me] > 0.
- */
-
- else if (g_cpu_nestcount[me] <= 0)
- {
- /* Do nothing here
- * NOTE: spin_clrbit() will be done in sched_resumescheduler()
- */
- }
-
- /* Sanity check. g_cpu_netcount should be greater than zero
- * only while we are within the critical section and within
- * an interrupt handler. If we are not in an interrupt handler
- * then there is a problem; perhaps some logic previously
- * called enter_critical_section() with no matching call to
- * leave_critical_section(), leaving the non-zero count.
- */
-
- else
- {
- DEBUGASSERT(up_interrupt_context());
- }
-
/* If the following task is not locked to this CPU, then it must
* be moved to the g_readytorun list. Since it cannot be at the
* head of the list, we can do this without invoking any heavy
diff --git a/sched/sched/sched_removereadytorun.c b/sched/sched/sched_removereadytorun.c
index 963a259..87b6cbd 100644
--- a/sched/sched/sched_removereadytorun.c
+++ b/sched/sched/sched_removereadytorun.c
@@ -260,47 +260,12 @@ bool nxsched_remove_readytorun(FAR struct tcb_s *rtcb)
&g_cpu_schedlock);
}
- /* Adjust global IRQ controls. If irqcount is greater than zero,
- * then this task/this CPU holds the IRQ lock
+ /* NOTE: If the task runs on another CPU(cpu), adjusting global IRQ
+ * controls will be done in the pause handler on the new CPU(cpu).
+ * If the task is scheduled on this CPU(me), do nothing because
+ * this CPU already has a critical section
*/
- if (nxttcb->irqcount > 0)
- {
- /* Yes... make sure that scheduling logic on other CPUs knows
- * that we hold the IRQ lock.
- */
-
- spin_setbit(&g_cpu_irqset, cpu, &g_cpu_irqsetlock,
- &g_cpu_irqlock);
- }
-
- /* No.. This CPU will be relinquishing the lock. But this works
- * differently if we are performing a context switch from an
- * interrupt handler and the interrupt handler has established
- * a critical section. We can detect this case when
- * g_cpu_nestcount[me] > 0.
- */
-
- else if (g_cpu_nestcount[me] <= 0)
- {
- /* Do nothing here
- * NOTE: spin_clrbit() will be done in sched_resumescheduler()
- */
- }
-
- /* Sanity check. g_cpu_netcount should be greater than zero
- * only while we are within the critical section and within
- * an interrupt handler. If we are not in an interrupt handler
- * then there is a problem; perhaps some logic previously
- * called enter_critical_section() with no matching call to
- * leave_critical_section(), leaving the non-zero count.
- */
-
- else
- {
- DEBUGASSERT(up_interrupt_context());
- }
-
nxttcb->task_state = TSTATE_TASK_RUNNING;
/* All done, restart the other CPU (if it was paused). */
diff --git a/sched/sched/sched_resumescheduler.c b/sched/sched/sched_resumescheduler.c
index fc9fe26..88c354b 100644
--- a/sched/sched/sched_resumescheduler.c
+++ b/sched/sched/sched_resumescheduler.c
@@ -136,8 +136,11 @@ void nxsched_resume_scheduler(FAR struct tcb_s *tcb)
{
/* Release our hold on the IRQ lock. */
- spin_clrbit(&g_cpu_irqset, me, &g_cpu_irqsetlock,
- &g_cpu_irqlock);
+ if ((g_cpu_irqset & (1 << me)) != 0)
+ {
+ spin_clrbit(&g_cpu_irqset, me, &g_cpu_irqsetlock,
+ &g_cpu_irqlock);
+ }
}
#endif /* CONFIG_SMP */
}
diff --git a/sched/task/task_exit.c b/sched/task/task_exit.c
index d570e45..64daf88 100644
--- a/sched/task/task_exit.c
+++ b/sched/task/task_exit.c
@@ -164,6 +164,13 @@ int nxtask_exit(void)
if (rtcb->irqcount == 0)
{
+ /* NOTE: Need to aquire g_cpu_irqlock here again before
+ * calling spin_setbit() becauses it was released in
+ * the above nxsched_resume_scheduler()
+ */
+
+ DEBUGVERIFY(irq_waitlock(this_cpu()));
+
spin_setbit(&g_cpu_irqset, this_cpu(), &g_cpu_irqsetlock,
&g_cpu_irqlock);
}