This is an automated email from the ASF dual-hosted git repository.
xiaoxiang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-nuttx.git
The following commit(s) were added to refs/heads/master by this push:
new ec73a4e arch & sched: task: Fix up_exit() and nxtask_exit() for SMP
ec73a4e is described below
commit ec73a4e69c4f0013c84eb2edf1536cad922df087
Author: Masayuki Ishikawa <masayuki.ishik...@gmail.com>
AuthorDate: Tue Dec 22 08:49:19 2020 +0900
arch & sched: task: Fix up_exit() and nxtask_exit() for SMP
Summary:
- During repeating ostest with sabre-6quad:smp (QEMU),
I noticed that pthread_rwlock_test sometimes stops
- Finally, I found that nxtask_exit() released a critical
section too early before context switching which resulted in
selecting inappropriate TCB
- This commit fixes this issue by moving nxsched_resume_scheduler()
from nxtask_exit() to up_exit() and also removing
spin_setbit() and spin_clrbit() from nxtask_exit()
because the caller holds a critical section
- To be consistent with non-SMP cases, the above changes
were done for all CPU architectures
Impact:
- This commit affects all CPU architectures regardless of SMP
Testing:
- Tested with ostest with the following configs
- sabre-6quad:smp (QEMU, dev board), sabre-6quad:nsh (QEMU)
- spresense:wifi_smp
- sim:smp, sim:ostest
- maix-bit:smp (QEMU)
- esp32-devkitc:smp (QEMU)
- lc823450-xgevk:rndis
Signed-off-by: Masayuki Ishikawa <masayuki.ishik...@jp.sony.com>
---
arch/arm/src/common/arm_exit.c | 6 ++++++
arch/avr/src/common/up_exit.c | 4 ++++
arch/hc/src/common/up_exit.c | 4 ++++
arch/mips/src/common/mips_exit.c | 6 ++++++
arch/misoc/src/lm32/lm32_exit.c | 4 ++++
arch/misoc/src/minerva/minerva_exit.c | 4 ++++
arch/or1k/src/common/up_exit.c | 4 ++++
arch/renesas/src/common/up_exit.c | 4 ++++
arch/risc-v/src/common/riscv_exit.c | 6 ++++++
arch/sim/src/sim/up_exit.c | 6 ++++++
arch/x86/src/common/up_exit.c | 6 ++++++
arch/x86_64/src/common/up_exit.c | 6 ++++++
arch/xtensa/src/common/xtensa_exit.c | 6 ++++++
arch/z16/src/common/z16_exit.c | 4 ++++
arch/z80/src/common/z80_exit.c | 4 ++++
sched/task/task_exit.c | 27 ++++-----------------------
16 files changed, 78 insertions(+), 23 deletions(-)
diff --git a/arch/arm/src/common/arm_exit.c b/arch/arm/src/common/arm_exit.c
index b538826..eb04f88 100644
--- a/arch/arm/src/common/arm_exit.c
+++ b/arch/arm/src/common/arm_exit.c
@@ -150,6 +150,12 @@ void up_exit(int status)
tcb = this_task();
+ /* Adjusts time slice for SCHED_RR & SCHED_SPORADIC cases
+ * NOTE: the API also adjusts the global IRQ control for SMP
+ */
+
+ nxsched_resume_scheduler(tcb);
+
#ifdef CONFIG_ARCH_ADDRENV
/* Make sure that the address environment for the previously running
* task is closed down gracefully (data caches dump, MMU flushed) and
diff --git a/arch/avr/src/common/up_exit.c b/arch/avr/src/common/up_exit.c
index 09939d6..23ffeb1 100644
--- a/arch/avr/src/common/up_exit.c
+++ b/arch/avr/src/common/up_exit.c
@@ -165,6 +165,10 @@ void up_exit(int status)
tcb = this_task();
+ /* Adjusts time slice for SCHED_RR & SCHED_SPORADIC cases */
+
+ nxsched_resume_scheduler(tcb);
+
#ifdef CONFIG_ARCH_ADDRENV
/* Make sure that the address environment for the previously running
* task is closed down gracefully (data caches dump, MMU flushed) and
diff --git a/arch/hc/src/common/up_exit.c b/arch/hc/src/common/up_exit.c
index febd544..96763f5 100644
--- a/arch/hc/src/common/up_exit.c
+++ b/arch/hc/src/common/up_exit.c
@@ -149,6 +149,10 @@ void up_exit(int status)
tcb = this_task();
+ /* Adjusts time slice for SCHED_RR & SCHED_SPORADIC cases */
+
+ nxsched_resume_scheduler(tcb);
+
#ifdef CONFIG_ARCH_ADDRENV
/* Make sure that the address environment for the previously running
* task is closed down gracefully (data caches dump, MMU flushed) and
diff --git a/arch/mips/src/common/mips_exit.c b/arch/mips/src/common/mips_exit.c
index 30d4216..c1d657b 100644
--- a/arch/mips/src/common/mips_exit.c
+++ b/arch/mips/src/common/mips_exit.c
@@ -167,6 +167,12 @@ void up_exit(int status)
tcb = this_task();
+ /* Adjusts time slice for SCHED_RR & SCHED_SPORADIC cases
+ * NOTE: the API also adjusts the global IRQ control for SMP
+ */
+
+ nxsched_resume_scheduler(tcb);
+
#ifdef CONFIG_ARCH_ADDRENV
/* Make sure that the address environment for the previously running
* task is closed down gracefully (data caches dump, MMU flushed) and
diff --git a/arch/misoc/src/lm32/lm32_exit.c b/arch/misoc/src/lm32/lm32_exit.c
index 9a77931..a4d552f 100644
--- a/arch/misoc/src/lm32/lm32_exit.c
+++ b/arch/misoc/src/lm32/lm32_exit.c
@@ -160,6 +160,10 @@ void up_exit(int status)
tcb = this_task();
+ /* Adjusts time slice for RR & SPORADIC cases */
+
+ nxsched_resume_scheduler(tcb);
+
#ifdef CONFIG_ARCH_ADDRENV
/* Make sure that the address environment for the previously running
* task is closed down gracefully (data caches dump, MMU flushed) and
diff --git a/arch/misoc/src/minerva/minerva_exit.c
b/arch/misoc/src/minerva/minerva_exit.c
index f437766..d3edbeb 100644
--- a/arch/misoc/src/minerva/minerva_exit.c
+++ b/arch/misoc/src/minerva/minerva_exit.c
@@ -158,6 +158,10 @@ void up_exit(int status)
tcb = this_task();
+ /* Adjusts time slice for SCHED_RR & SCHED_SPORADIC cases */
+
+ nxsched_resume_scheduler(tcb);
+
#ifdef CONFIG_ARCH_ADDRENV
/* Make sure that the address environment for the previously running task
* is closed down gracefully (data caches dump, MMU flushed) and set up the
diff --git a/arch/or1k/src/common/up_exit.c b/arch/or1k/src/common/up_exit.c
index 4d50254..1224b6e 100644
--- a/arch/or1k/src/common/up_exit.c
+++ b/arch/or1k/src/common/up_exit.c
@@ -165,6 +165,10 @@ void up_exit(int status)
tcb = this_task();
+ /* Adjusts time slice for SCHED_RR & SCHED_SPORADIC cases */
+
+ nxsched_resume_scheduler(tcb);
+
#ifdef CONFIG_ARCH_ADDRENV
/* Make sure that the address environment for the previously running
* task is closed down gracefully (data caches dump, MMU flushed) and
diff --git a/arch/renesas/src/common/up_exit.c
b/arch/renesas/src/common/up_exit.c
index b272fcb..e50baa6 100644
--- a/arch/renesas/src/common/up_exit.c
+++ b/arch/renesas/src/common/up_exit.c
@@ -149,6 +149,10 @@ void up_exit(int status)
tcb = this_task();
+ /* Adjusts time slice for RR & SPORADIC cases */
+
+ nxsched_resume_scheduler(tcb);
+
#ifdef CONFIG_ARCH_ADDRENV
/* Make sure that the address environment for the previously running
* task is closed down gracefully (data caches dump, MMU flushed) and
diff --git a/arch/risc-v/src/common/riscv_exit.c
b/arch/risc-v/src/common/riscv_exit.c
index b4f797e..0129728 100644
--- a/arch/risc-v/src/common/riscv_exit.c
+++ b/arch/risc-v/src/common/riscv_exit.c
@@ -167,6 +167,12 @@ void up_exit(int status)
tcb = this_task();
+ /* Adjusts time slice for SCHED_RR & SCHED_SPORADIC cases
+ * NOTE: the API also adjusts the global IRQ control for SMP
+ */
+
+ nxsched_resume_scheduler(tcb);
+
#ifdef CONFIG_ARCH_ADDRENV
/* Make sure that the address environment for the previously running
* task is closed down gracefully (data caches dump, MMU flushed) and
diff --git a/arch/sim/src/sim/up_exit.c b/arch/sim/src/sim/up_exit.c
index 400a3ee..55c189e 100644
--- a/arch/sim/src/sim/up_exit.c
+++ b/arch/sim/src/sim/up_exit.c
@@ -86,6 +86,12 @@ void up_exit(int status)
tcb = this_task();
sinfo("New Active Task TCB=%p\n", tcb);
+ /* Adjusts time slice for SCHED_RR & SCHED_SPORADIC cases
+ * NOTE: the API also adjusts the global IRQ control for SMP
+ */
+
+ nxsched_resume_scheduler(tcb);
+
/* 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.
diff --git a/arch/x86/src/common/up_exit.c b/arch/x86/src/common/up_exit.c
index 22edf7e..05a6c40 100644
--- a/arch/x86/src/common/up_exit.c
+++ b/arch/x86/src/common/up_exit.c
@@ -149,6 +149,12 @@ void up_exit(int status)
tcb = this_task();
+ /* Adjusts time slice for SCHED_RR & SCHED_SPORADIC cases
+ * NOTE: the API also adjusts the global IRQ control for SMP
+ */
+
+ nxsched_resume_scheduler(tcb);
+
#ifdef CONFIG_ARCH_ADDRENV
/* Make sure that the address environment for the previously running
* task is closed down gracefully (data caches dump, MMU flushed) and
diff --git a/arch/x86_64/src/common/up_exit.c b/arch/x86_64/src/common/up_exit.c
index 36808da..6136e6a 100644
--- a/arch/x86_64/src/common/up_exit.c
+++ b/arch/x86_64/src/common/up_exit.c
@@ -153,6 +153,12 @@ void up_exit(int status)
tcb = this_task();
+ /* Adjusts time slice for SCHED_RR & SCHED_SPORADIC cases
+ * NOTE: the API also adjusts the global IRQ control for SMP
+ */
+
+ nxsched_resume_scheduler(tcb);
+
/* Context switch, rearrange MMU */
up_restore_auxstate(tcb);
diff --git a/arch/xtensa/src/common/xtensa_exit.c
b/arch/xtensa/src/common/xtensa_exit.c
index fb16154..e9243e6 100644
--- a/arch/xtensa/src/common/xtensa_exit.c
+++ b/arch/xtensa/src/common/xtensa_exit.c
@@ -172,6 +172,12 @@ void up_exit(int status)
tcb = this_task();
+ /* Adjusts time slice for SCHED_RR & SCHED_SPORADIC cases
+ * NOTE: the API also adjusts the global IRQ control for SMP
+ */
+
+ nxsched_resume_scheduler(tcb);
+
#if XCHAL_CP_NUM > 0
/* Set up the co-processor state for the newly started thread. */
diff --git a/arch/z16/src/common/z16_exit.c b/arch/z16/src/common/z16_exit.c
index f92b03f..15a3487 100644
--- a/arch/z16/src/common/z16_exit.c
+++ b/arch/z16/src/common/z16_exit.c
@@ -150,6 +150,10 @@ void up_exit(int status)
tcb = this_task();
sinfo("New Active Task TCB=%p\n", tcb);
+ /* Adjusts time slice for SCHED_RR & SCHED_SPORADIC cases */
+
+ nxsched_resume_scheduler(tcb);
+
/* Then switch contexts */
RESTORE_USERCONTEXT(tcb);
diff --git a/arch/z80/src/common/z80_exit.c b/arch/z80/src/common/z80_exit.c
index 1a73ac9..495bf27 100644
--- a/arch/z80/src/common/z80_exit.c
+++ b/arch/z80/src/common/z80_exit.c
@@ -152,6 +152,10 @@ void up_exit(int status)
tcb = this_task();
sinfo("New Active Task TCB=%p\n", tcb);
+ /* Adjusts time slice for SCHED_RR & SCHED_SPORADIC cases */
+
+ nxsched_resume_scheduler(tcb);
+
#ifdef CONFIG_ARCH_ADDRENV
/* Make sure that the address environment for the previously running
* task is closed down gracefully (data caches dump, MMU flushed) and
diff --git a/sched/task/task_exit.c b/sched/task/task_exit.c
index 64daf88..b30a965 100644
--- a/sched/task/task_exit.c
+++ b/sched/task/task_exit.c
@@ -119,12 +119,12 @@ int nxtask_exit(void)
rtcb = this_task();
#endif
- /* Because clearing the global IRQ control in nxsched_remove_readytorun()
- * was moved to nxsched_resume_scheduler(). So call the API here.
+ /* NOTE: nxsched_resume_scheduler() was moved to up_exit()
+ * because the global IRQ control for SMP should be deferred until
+ * context switching, otherwise, the context switching would be done
+ * without a critical section
*/
- nxsched_resume_scheduler(rtcb);
-
/* We are now in a bad state -- the head of the ready to run task list
* does not correspond to the thread that is running. Disabling pre-
* emption on this TCB and marking the new ready-to-run task as not
@@ -162,19 +162,6 @@ int nxtask_exit(void)
* To prevent from aquiring, increment rtcb->irqcount here.
*/
- 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);
- }
-
rtcb->irqcount++;
#endif
@@ -182,12 +169,6 @@ int nxtask_exit(void)
#ifdef CONFIG_SMP
rtcb->irqcount--;
-
- if (rtcb->irqcount == 0)
- {
- spin_clrbit(&g_cpu_irqset, this_cpu(), &g_cpu_irqsetlock,
- &g_cpu_irqlock);
- }
#endif
rtcb->task_state = TSTATE_TASK_RUNNING;
