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/11/25 02:14:50 UTC
[incubator-nuttx] branch releases/10.0 updated (2f7092b -> 5c3ce49)
This is an automated email from the ASF dual-hosted git repository.
xiaoxiang pushed a change to branch releases/10.0
in repository https://gitbox.apache.org/repos/asf/incubator-nuttx.git.
from 2f7092b tools: Fix nuttx-gdbinit for armv7-m with FPU
new f9df77b Revert "Update TODO regarding SMP"
new c9176df Revert "arch: xtensa: Fix the pause handler for SMP"
new 737ca80 Revert "arch: k210: Fix the pause handler for SMP"
new cfeef48 Revert "arch: lc823450: Fix the pause handler for SMP"
new 5a941d4 Revert "arch: armv7-a: Fix the pause handler for SMP"
new 5c3ce49 Revert "arch: cxd56xx: Fix the pause handler for SMP"
The 6 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 | 66 ++++++++++++++++++++++++++++++-
arch/arm/src/armv7-a/arm_cpupause.c | 16 +-------
arch/arm/src/cxd56xx/cxd56_cpupause.c | 16 +-------
arch/arm/src/lc823450/lc823450_cpupause.c | 16 +-------
arch/risc-v/src/k210/k210_cpupause.c | 16 +-------
arch/xtensa/src/common/xtensa_cpupause.c | 11 ------
6 files changed, 73 insertions(+), 68 deletions(-)
[incubator-nuttx] 02/06: Revert "arch: xtensa: Fix the pause
handler for SMP"
Posted by xi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
xiaoxiang pushed a commit to branch releases/10.0
in repository https://gitbox.apache.org/repos/asf/incubator-nuttx.git
commit c9176dfe5a34acea4f6204d2a188bf0390f78e02
Author: Masayuki Ishikawa <ma...@gmail.com>
AuthorDate: Wed Nov 25 06:58:49 2020 +0900
Revert "arch: xtensa: Fix the pause handler for SMP"
This reverts commit 1914aac05f5b29e4fc54b143f3c461154efde96f.
---
arch/xtensa/src/common/xtensa_cpupause.c | 11 -----------
1 file changed, 11 deletions(-)
diff --git a/arch/xtensa/src/common/xtensa_cpupause.c b/arch/xtensa/src/common/xtensa_cpupause.c
index 0d7c486..5b59200 100644
--- a/arch/xtensa/src/common/xtensa_cpupause.c
+++ b/arch/xtensa/src/common/xtensa_cpupause.c
@@ -192,18 +192,7 @@ void xtensa_pause_handler(void)
if (spin_islocked(&g_cpu_paused[cpu]))
{
- /* NOTE: up_cpu_paused() needs to be executed in a critical section
- * to ensure that this CPU holds g_cpu_irqlock. However, adding
- * a critical section in up_cpu_paused() is not a good idea,
- * because it is also called in enter_critical_section() to break
- * a deadlock
- */
-
- irqstate_t flags = enter_critical_section();
-
up_cpu_paused(cpu);
-
- leave_critical_section(flags);
}
}
[incubator-nuttx] 03/06: Revert "arch: k210: Fix the pause handler
for SMP"
Posted by xi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
xiaoxiang pushed a commit to branch releases/10.0
in repository https://gitbox.apache.org/repos/asf/incubator-nuttx.git
commit 737ca80207700a973c5d693b25df20057fcb7740
Author: Masayuki Ishikawa <ma...@gmail.com>
AuthorDate: Wed Nov 25 06:58:49 2020 +0900
Revert "arch: k210: Fix the pause handler for SMP"
This reverts commit a500bd0238e0db994642ed7487de3e9e4b17dd3d.
---
arch/risc-v/src/k210/k210_cpupause.c | 16 ++--------------
1 file changed, 2 insertions(+), 14 deletions(-)
diff --git a/arch/risc-v/src/k210/k210_cpupause.c b/arch/risc-v/src/k210/k210_cpupause.c
index 2eaa573..88af783 100644
--- a/arch/risc-v/src/k210/k210_cpupause.c
+++ b/arch/risc-v/src/k210/k210_cpupause.c
@@ -205,7 +205,6 @@ int up_cpu_paused(int cpu)
int riscv_pause_handler(int irq, void *c, FAR void *arg)
{
int cpu = up_cpu_index();
- int ret = OK;
/* Clear machine software interrupt */
@@ -218,21 +217,10 @@ int riscv_pause_handler(int irq, void *c, FAR void *arg)
if (spin_islocked(&g_cpu_paused[cpu]))
{
- /* NOTE: up_cpu_paused() needs to be executed in a critical section
- * to ensure that this CPU holds g_cpu_irqlock. However, adding
- * a critical section in up_cpu_paused() is not a good idea,
- * because it is also called in enter_critical_section() to break
- * a deadlock
- */
-
- irqstate_t flags = enter_critical_section();
-
- ret = up_cpu_paused(cpu);
-
- leave_critical_section(flags);
+ return up_cpu_paused(cpu);
}
- return ret;
+ return OK;
}
/****************************************************************************
[incubator-nuttx] 04/06: Revert "arch: lc823450: Fix the pause
handler for SMP"
Posted by xi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
xiaoxiang pushed a commit to branch releases/10.0
in repository https://gitbox.apache.org/repos/asf/incubator-nuttx.git
commit cfeef48732e8be13e32d06bf6d9be97e13a7bd49
Author: Masayuki Ishikawa <ma...@gmail.com>
AuthorDate: Wed Nov 25 06:58:49 2020 +0900
Revert "arch: lc823450: Fix the pause handler for SMP"
This reverts commit 42dea9edf903329d5a4cb285bcbf1fcd68b23637.
---
arch/arm/src/lc823450/lc823450_cpupause.c | 16 ++--------------
1 file changed, 2 insertions(+), 14 deletions(-)
diff --git a/arch/arm/src/lc823450/lc823450_cpupause.c b/arch/arm/src/lc823450/lc823450_cpupause.c
index b172df1..2714fc3 100644
--- a/arch/arm/src/lc823450/lc823450_cpupause.c
+++ b/arch/arm/src/lc823450/lc823450_cpupause.c
@@ -190,7 +190,6 @@ int up_cpu_paused(int cpu)
int lc823450_pause_handler(int irq, void *c, FAR void *arg)
{
int cpu = up_cpu_index();
- int ret = OK;
/* Clear : Pause IRQ */
@@ -212,21 +211,10 @@ int lc823450_pause_handler(int irq, void *c, FAR void *arg)
if (spin_islocked(&g_cpu_paused[cpu]))
{
- /* NOTE: up_cpu_paused() needs to be executed in a critical section
- * to ensure that this CPU holds g_cpu_irqlock. However, adding
- * a critical section in up_cpu_paused() is not a good idea,
- * because it is also called in enter_critical_section() to break
- * a deadlock
- */
-
- irqstate_t flags = enter_critical_section();
-
- ret = up_cpu_paused(cpu);
-
- leave_critical_section(flags);
+ return up_cpu_paused(cpu);
}
- return ret;
+ return OK;
}
/****************************************************************************
[incubator-nuttx] 06/06: Revert "arch: cxd56xx: Fix the pause
handler for SMP"
Posted by xi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
xiaoxiang pushed a commit to branch releases/10.0
in repository https://gitbox.apache.org/repos/asf/incubator-nuttx.git
commit 5c3ce49d8240a13899fb5a9d93b70c50140fcd41
Author: Masayuki Ishikawa <ma...@gmail.com>
AuthorDate: Wed Nov 25 06:58:49 2020 +0900
Revert "arch: cxd56xx: Fix the pause handler for SMP"
This reverts commit 55c00ad3d979ecf5d11530eaf15abfc4dcfce73f.
---
arch/arm/src/cxd56xx/cxd56_cpupause.c | 16 ++--------------
1 file changed, 2 insertions(+), 14 deletions(-)
diff --git a/arch/arm/src/cxd56xx/cxd56_cpupause.c b/arch/arm/src/cxd56xx/cxd56_cpupause.c
index 1f5e6b4..c8bd410 100644
--- a/arch/arm/src/cxd56xx/cxd56_cpupause.c
+++ b/arch/arm/src/cxd56xx/cxd56_cpupause.c
@@ -283,7 +283,6 @@ 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);
@@ -298,21 +297,10 @@ int arm_pause_handler(int irq, void *c, FAR void *arg)
if (spin_islocked(&g_cpu_paused[cpu]))
{
- /* NOTE: up_cpu_paused() needs to be executed in a critical section
- * to ensure that this CPU holds g_cpu_irqlock. However, adding
- * a critical section in up_cpu_paused() is not a good idea,
- * because it is also called in enter_critical_section() to break
- * a deadlock
- */
-
- irqstate_t flags = enter_critical_section();
-
- ret = up_cpu_paused(cpu);
-
- leave_critical_section(flags);
+ return up_cpu_paused(cpu);
}
- return ret;
+ return OK;
}
/****************************************************************************
[incubator-nuttx] 01/06: Revert "Update TODO regarding SMP"
Posted by xi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
xiaoxiang pushed a commit to branch releases/10.0
in repository https://gitbox.apache.org/repos/asf/incubator-nuttx.git
commit f9df77b180bd7ab356574f61f782b01fcbcf6bfb
Author: Masayuki Ishikawa <ma...@gmail.com>
AuthorDate: Wed Nov 25 06:58:49 2020 +0900
Revert "Update TODO regarding SMP"
This reverts commit 96c29e75b7edf076e7ae3b935918b0366fce0287.
---
TODO | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 65 insertions(+), 1 deletion(-)
diff --git a/TODO b/TODO
index d56cf2d..43d7b40 100644
--- a/TODO
+++ b/TODO
@@ -10,7 +10,7 @@ issues related to each board port.
nuttx/:
(16) Task/Scheduler (sched/)
- (2) SMP
+ (3) SMP
(1) Memory Management (mm/)
(0) Power Management (drivers/pm)
(5) Signals (sched/signal, arch/)
@@ -485,6 +485,70 @@ 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/)
^^^^^^^^^^^^^^^^^^^^^^^
[incubator-nuttx] 05/06: Revert "arch: armv7-a: Fix the pause
handler for SMP"
Posted by xi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
xiaoxiang pushed a commit to branch releases/10.0
in repository https://gitbox.apache.org/repos/asf/incubator-nuttx.git
commit 5a941d4913b4c07edb05260c091d013c51868d1a
Author: Masayuki Ishikawa <ma...@gmail.com>
AuthorDate: Wed Nov 25 06:58:49 2020 +0900
Revert "arch: armv7-a: Fix the pause handler for SMP"
This reverts commit 1978dcc9a98c5adb3eb5851ab5e3846b2de2f95c.
---
arch/arm/src/armv7-a/arm_cpupause.c | 16 ++--------------
1 file changed, 2 insertions(+), 14 deletions(-)
diff --git a/arch/arm/src/armv7-a/arm_cpupause.c b/arch/arm/src/armv7-a/arm_cpupause.c
index 36d1eb0..4725418 100644
--- a/arch/arm/src/armv7-a/arm_cpupause.c
+++ b/arch/arm/src/armv7-a/arm_cpupause.c
@@ -205,7 +205,6 @@ int up_cpu_paused(int cpu)
int arm_pause_handler(int irq, FAR void *context, FAR void *arg)
{
int cpu = this_cpu();
- int ret = OK;
/* Check for false alarms. Such false could occur as a consequence of
* some deadlock breaking logic that might have already serviced the SG2
@@ -215,21 +214,10 @@ int arm_pause_handler(int irq, FAR void *context, FAR void *arg)
if (spin_islocked(&g_cpu_paused[cpu]))
{
- /* NOTE: up_cpu_paused() needs to be executed in a critical section
- * to ensure that this CPU holds g_cpu_irqlock. However, adding
- * a critical section in up_cpu_paused() is not a good idea,
- * because it is also called in enter_critical_section() to break
- * a deadlock
- */
-
- irqstate_t flags = enter_critical_section();
-
- ret = up_cpu_paused(cpu);
-
- leave_critical_section(flags);
+ return up_cpu_paused(cpu);
}
- return ret;
+ return OK;
}
/****************************************************************************