You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by xi...@apache.org on 2020/12/22 05:30:04 UTC

[incubator-nuttx] branch master updated: arch & sched: task: Fix up_exit() and nxtask_exit() for SMP

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 <ma...@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 <Ma...@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;