You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by ma...@apache.org on 2020/11/13 06:38:48 UTC
[incubator-nuttx] branch releases/10.0 updated (d7a93d6 -> 3de074b)
This is an automated email from the ASF dual-hosted git repository.
masayuki pushed a change to branch releases/10.0
in repository https://gitbox.apache.org/repos/asf/incubator-nuttx.git.
from d7a93d6 Fix issue #2098 non functional sim:touchscreen
new d51e736 sched: task: Fix nxtask_exit() for SMP
new f93152a sched: pthread: Fix pthread_join() for SMP
new 3de074b sched: sched: Remove sched_tasklistlock.c for SMP
The 3 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:
sched/pthread/pthread_join.c | 13 ++++
sched/sched/Make.defs | 1 -
sched/sched/sched.h | 3 -
sched/sched/sched_addblocked.c | 12 ----
sched/sched/sched_addreadytorun.c | 9 ---
sched/sched/sched_mergepending.c | 17 ++---
sched/sched/sched_mergeprioritized.c | 17 +----
sched/sched/sched_removereadytorun.c | 9 ---
sched/sched/sched_tasklistlock.c | 118 -----------------------------------
sched/task/task_exit.c | 18 +++---
10 files changed, 29 insertions(+), 188 deletions(-)
delete mode 100644 sched/sched/sched_tasklistlock.c
[incubator-nuttx] 02/03: sched: pthread: Fix pthread_join() for SMP
Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
masayuki pushed a commit to branch releases/10.0
in repository https://gitbox.apache.org/repos/asf/incubator-nuttx.git
commit f93152a867b16e46c63d38cda60c7741c041a6ec
Author: Masayuki Ishikawa <ma...@gmail.com>
AuthorDate: Tue Nov 10 11:03:12 2020 +0900
sched: pthread: Fix pthread_join() for SMP
Summary:
- I noticed 'pthread_rwlock test' in ostest sometimes stops
- This issue happened with spresense:wifi_smp (NCPUS=4) and sim:smp
- Finally, I found an issue in pthread_join()
- In pthread_join(), sched_lock() is used to avoid pre-emption
- However, this is not enough for SMP
- Because another CPU would continue the pthread and exit sequences
- So we need to protect it with a critical section
Impact:
- Affect SMP only
Testing:
- Tested with ostest with the following configurations
- spresnese:smp
- spresense:wifi_smp (NCPUS=2, NCPUS=4)
- sabre-6quad:smp (QEMU)
- esp32-core:smp (QEMU)
- maix-bit:smp (QEMU)
- sim:smp
- lc823450-xgevk:rndis
Signed-off-by: Masayuki Ishikawa <Ma...@jp.sony.com>
---
sched/pthread/pthread_join.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/sched/pthread/pthread_join.c b/sched/pthread/pthread_join.c
index 994bc7d..328708b 100644
--- a/sched/pthread/pthread_join.c
+++ b/sched/pthread/pthread_join.c
@@ -137,6 +137,15 @@ int pthread_join(pthread_t thread, FAR pthread_addr_t *pexit_value)
}
else
{
+ /* NOTE: sched_lock() is not enough for SMP
+ * because another CPU would continue the pthread and exit
+ * sequences so need to protect it with a critical section
+ */
+
+#ifdef CONFIG_SMP
+ irqstate_t flags = enter_critical_section();
+#endif
+
/* We found the join info structure. Increment for the reference
* to the join structure that we have. This will keep things
* stable for we have to do
@@ -211,6 +220,10 @@ int pthread_join(pthread_t thread, FAR pthread_addr_t *pexit_value)
sched_unlock();
+#ifdef CONFIG_SMP
+ leave_critical_section(flags);
+#endif
+
/* Release our reference to the join structure and, if the reference
* count decrements to zero, deallocate the join structure.
*/
[incubator-nuttx] 03/03: sched: sched: Remove sched_tasklistlock.c
for SMP
Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
masayuki pushed a commit to branch releases/10.0
in repository https://gitbox.apache.org/repos/asf/incubator-nuttx.git
commit 3de074be0bba0baba5c2d5d4ca63e82369ccfe11
Author: Masayuki Ishikawa <ma...@gmail.com>
AuthorDate: Wed Nov 11 08:13:45 2020 +0900
sched: sched: Remove sched_tasklistlock.c for SMP
Summary:
- sched_tasklistlock.c was introduced to stabilize NuttX SMP
- However, the current SMP implementation is stable by recent fixes
- So I decided to remove the file finally
Impact:
- SMP only
Testing:
- Tested with ostest with the following configurations
- spresense:smp
- spresense:wifi_smp (NCPUS=2 and 4)
- sabre-6quad:smp (QEMU)
- esp32-core:smp (QEMU)
- maix-bit:smp (QEMU)
- sim:smp
- lc823450-xgevk:rndis
Signed-off-by: Masayuki Ishikawa <Ma...@jp.sony.com>
---
sched/sched/Make.defs | 1 -
sched/sched/sched.h | 3 -
sched/sched/sched_addblocked.c | 12 ----
sched/sched/sched_addreadytorun.c | 9 ---
sched/sched/sched_mergepending.c | 17 ++---
sched/sched/sched_mergeprioritized.c | 17 +----
sched/sched/sched_removereadytorun.c | 9 ---
sched/sched/sched_tasklistlock.c | 118 -----------------------------------
8 files changed, 7 insertions(+), 179 deletions(-)
diff --git a/sched/sched/Make.defs b/sched/sched/Make.defs
index 21dd5cc..f46c39d 100644
--- a/sched/sched/Make.defs
+++ b/sched/sched/Make.defs
@@ -85,7 +85,6 @@ CSRCS += sched_processtimer.c
endif
ifeq ($(CONFIG_SMP),y)
-CSRCS += sched_tasklistlock.c
CSRCS += sched_thistask.c
endif
diff --git a/sched/sched/sched.h b/sched/sched/sched.h
index 084ba3c..71d4f46 100644
--- a/sched/sched/sched.h
+++ b/sched/sched/sched.h
@@ -417,9 +417,6 @@ FAR struct tcb_s *this_task(void);
int nxsched_select_cpu(cpu_set_t affinity);
int nxsched_pause_cpu(FAR struct tcb_s *tcb);
-irqstate_t nxsched_lock_tasklist(void);
-void nxsched_unlock_tasklist(irqstate_t lock);
-
# define nxsched_islocked_global() spin_islocked(&g_cpu_schedlock)
# define nxsched_islocked_tcb(tcb) nxsched_islocked_global()
diff --git a/sched/sched/sched_addblocked.c b/sched/sched/sched_addblocked.c
index 6b6d855..3965f61 100644
--- a/sched/sched/sched_addblocked.c
+++ b/sched/sched/sched_addblocked.c
@@ -77,12 +77,6 @@ void nxsched_add_blocked(FAR struct tcb_s *btcb, tstate_t task_state)
DEBUGASSERT(task_state >= FIRST_BLOCKED_STATE &&
task_state <= LAST_BLOCKED_STATE);
-#ifdef CONFIG_SMP
- /* Lock the tasklists before accessing */
-
- irqstate_t lock = nxsched_lock_tasklist();
-#endif
-
/* Add the TCB to the blocked task list associated with this state. */
tasklist = TLIST_BLOCKED(task_state);
@@ -102,12 +96,6 @@ void nxsched_add_blocked(FAR struct tcb_s *btcb, tstate_t task_state)
dq_addlast((FAR dq_entry_t *)btcb, tasklist);
}
-#ifdef CONFIG_SMP
- /* Unlock the tasklists */
-
- nxsched_unlock_tasklist(lock);
-#endif
-
/* Make sure the TCB's state corresponds to the list */
btcb->task_state = task_state;
diff --git a/sched/sched/sched_addreadytorun.c b/sched/sched/sched_addreadytorun.c
index acb5eca..0d610b5 100644
--- a/sched/sched/sched_addreadytorun.c
+++ b/sched/sched/sched_addreadytorun.c
@@ -174,10 +174,6 @@ bool nxsched_add_readytorun(FAR struct tcb_s *btcb)
int cpu;
int me;
- /* Lock the tasklists before accessing */
-
- irqstate_t lock = nxsched_lock_tasklist();
-
/* Check if the blocked TCB is locked to this CPU */
if ((btcb->flags & TCB_FLAG_CPU_LOCKED) != 0)
@@ -276,9 +272,7 @@ bool nxsched_add_readytorun(FAR struct tcb_s *btcb)
if (cpu != me)
{
- nxsched_unlock_tasklist(lock);
DEBUGVERIFY(up_cpu_pause(cpu));
- lock = nxsched_lock_tasklist();
}
/* Add the task to the list corresponding to the selected state
@@ -433,9 +427,6 @@ bool nxsched_add_readytorun(FAR struct tcb_s *btcb)
}
}
- /* Unlock the tasklists */
-
- nxsched_unlock_tasklist(lock);
return doswitch;
}
diff --git a/sched/sched/sched_mergepending.c b/sched/sched/sched_mergepending.c
index 3aba52e..05eaa66 100644
--- a/sched/sched/sched_mergepending.c
+++ b/sched/sched/sched_mergepending.c
@@ -184,10 +184,6 @@ bool nxsched_merge_pending(void)
int cpu;
int me;
- /* Lock the tasklist before accessing */
-
- irqstate_t lock = nxsched_lock_tasklist();
-
/* Remove and process every TCB in the g_pendingtasks list.
*
* Do nothing if (1) pre-emption is still disabled (by any CPU), or (2) if
@@ -204,7 +200,7 @@ bool nxsched_merge_pending(void)
{
/* The pending task list is empty */
- goto errout_with_lock;
+ goto errout;
}
cpu = nxsched_select_cpu(ALL_CPUS); /* REVISIT: Maybe ptcb->affinity */
@@ -228,9 +224,7 @@ bool nxsched_merge_pending(void)
/* Add the pending task to the correct ready-to-run list. */
- nxsched_unlock_tasklist(lock);
ret |= nxsched_add_readytorun(tcb);
- lock = nxsched_lock_tasklist();
/* This operation could cause the scheduler to become locked.
* Check if that happened.
@@ -251,7 +245,7 @@ bool nxsched_merge_pending(void)
* pending task list.
*/
- goto errout_with_lock;
+ goto errout;
}
/* Set up for the next time through the loop */
@@ -262,7 +256,7 @@ bool nxsched_merge_pending(void)
{
/* The pending task list is empty */
- goto errout_with_lock;
+ goto errout;
}
cpu = nxsched_select_cpu(ALL_CPUS); /* REVISIT: Maybe ptcb->affinity */
@@ -278,11 +272,8 @@ bool nxsched_merge_pending(void)
TSTATE_TASK_READYTORUN);
}
-errout_with_lock:
-
- /* Unlock the tasklist */
+errout:
- nxsched_unlock_tasklist(lock);
return ret;
}
#endif /* CONFIG_SMP */
diff --git a/sched/sched/sched_mergeprioritized.c b/sched/sched/sched_mergeprioritized.c
index 9ad4933..3afb2b2 100644
--- a/sched/sched/sched_mergeprioritized.c
+++ b/sched/sched/sched_mergeprioritized.c
@@ -68,12 +68,6 @@ void nxsched_merge_prioritized(FAR dq_queue_t *list1, FAR dq_queue_t *list2,
FAR struct tcb_s *tcb2;
FAR struct tcb_s *tmp;
-#ifdef CONFIG_SMP
- /* Lock the tasklists before accessing */
-
- irqstate_t lock = nxsched_lock_tasklist();
-#endif
-
DEBUGASSERT(list1 != NULL && list2 != NULL);
/* Get a private copy of list1, clearing list1. We do this early so that
@@ -90,7 +84,7 @@ void nxsched_merge_prioritized(FAR dq_queue_t *list1, FAR dq_queue_t *list2,
{
/* Special case.. list1 is empty. There is nothing to be done. */
- goto ret_with_lock;
+ goto out;
}
/* Now the TCBs are no longer accessible and we can change the state on
@@ -113,7 +107,7 @@ void nxsched_merge_prioritized(FAR dq_queue_t *list1, FAR dq_queue_t *list2,
/* Special case.. list2 is empty. Move list1 to list2. */
dq_move(&clone, list2);
- goto ret_with_lock;
+ goto out;
}
/* Now loop until all entries from list1 have been merged into list2. tcb1
@@ -163,12 +157,7 @@ void nxsched_merge_prioritized(FAR dq_queue_t *list1, FAR dq_queue_t *list2,
}
while (tcb1 != NULL);
-ret_with_lock:
-
-#ifdef CONFIG_SMP
- /* Unlock the tasklists */
+out:
- nxsched_unlock_tasklist(lock);
-#endif
return;
}
diff --git a/sched/sched/sched_removereadytorun.c b/sched/sched/sched_removereadytorun.c
index 094ffa2..963a259 100644
--- a/sched/sched/sched_removereadytorun.c
+++ b/sched/sched/sched_removereadytorun.c
@@ -140,10 +140,6 @@ bool nxsched_remove_readytorun(FAR struct tcb_s *rtcb)
bool doswitch = false;
int cpu;
- /* Lock the tasklists before accessing */
-
- irqstate_t lock = nxsched_lock_tasklist();
-
/* Which CPU (if any) is the task running on? Which task list holds the
* TCB?
*/
@@ -188,9 +184,7 @@ bool nxsched_remove_readytorun(FAR struct tcb_s *rtcb)
me = this_cpu();
if (cpu != me)
{
- nxsched_unlock_tasklist(lock);
DEBUGVERIFY(up_cpu_pause(cpu));
- lock = nxsched_lock_tasklist();
}
/* The task is running but the CPU that it was running on has been
@@ -336,9 +330,6 @@ bool nxsched_remove_readytorun(FAR struct tcb_s *rtcb)
rtcb->task_state = TSTATE_TASK_INVALID;
- /* Unlock the tasklists */
-
- nxsched_unlock_tasklist(lock);
return doswitch;
}
#endif /* CONFIG_SMP */
diff --git a/sched/sched/sched_tasklistlock.c b/sched/sched/sched_tasklistlock.c
deleted file mode 100644
index 64aa808..0000000
--- a/sched/sched/sched_tasklistlock.c
+++ /dev/null
@@ -1,118 +0,0 @@
-/****************************************************************************
- * sched/sched/sched_tasklistlock.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 <nuttx/spinlock.h>
-
-#include <sys/types.h>
-#include <arch/irq.h>
-
-#include "sched/sched.h"
-
-/****************************************************************************
- * Public Data
- ****************************************************************************/
-
-/* Splinlock to protect the tasklists */
-
-static volatile spinlock_t g_tasklist_lock SP_SECTION = SP_UNLOCKED;
-
-/* Handles nested calls */
-
-static volatile uint8_t g_tasklist_lock_count[CONFIG_SMP_NCPUS];
-
-/****************************************************************************
- * Public Functions
- ****************************************************************************/
-
-/****************************************************************************
- * Name: nxsched_lock_tasklist()
- *
- * Description:
- * Disable local interrupts and take the global spinlock (g_tasklist_lock)
- * if the call counter (g_tasklist_lock_count[cpu]) equals to 0. Then the
- * counter on the CPU is incremented to allow nested call.
- *
- * NOTE: This API is used to protect tasklists in the scheduler. So do not
- * use this API for other purposes.
- *
- * Returned Value:
- * An opaque, architecture-specific value that represents the state of
- * the interrupts prior to the call to nxsched_lock_tasklist();
- ****************************************************************************/
-
-irqstate_t nxsched_lock_tasklist(void)
-{
- int me;
- irqstate_t ret;
-
- ret = up_irq_save();
- me = this_cpu();
-
- if (0 == g_tasklist_lock_count[me])
- {
- spin_lock(&g_tasklist_lock);
- }
-
- g_tasklist_lock_count[me]++;
- DEBUGASSERT(0 != g_tasklist_lock_count[me]);
- return ret;
-}
-
-/****************************************************************************
- * Name: nxsched_unlock_tasklist()
- *
- * Description:
- * Decrement the call counter (g_tasklist_lock_count[cpu]) and if it
- * decrements to zero then release the spinlock (g_tasklist_lock) and
- * restore the interrupt state as it was prior to the previous call to
- * nxsched_lock_tasklist().
- *
- * NOTE: This API is used to protect tasklists in the scheduler. So do not
- * use this API for other purposes.
- *
- * Input Parameters:
- * lock - The architecture-specific value that represents the state of
- * the interrupts prior to the call to nxsched_lock_tasklist().
- *
- * Returned Value:
- * None
- ****************************************************************************/
-
-void nxsched_unlock_tasklist(irqstate_t lock)
-{
- int me;
-
- me = this_cpu();
-
- DEBUGASSERT(0 < g_tasklist_lock_count[me]);
- g_tasklist_lock_count[me]--;
-
- if (0 == g_tasklist_lock_count[me])
- {
- spin_unlock(&g_tasklist_lock);
- }
-
- up_irq_restore(lock);
-}
[incubator-nuttx] 01/03: sched: task: Fix nxtask_exit() for SMP
Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
masayuki pushed a commit to branch releases/10.0
in repository https://gitbox.apache.org/repos/asf/incubator-nuttx.git
commit d51e7361f611e40f933689fe382a4f5713e915d0
Author: Masayuki Ishikawa <ma...@gmail.com>
AuthorDate: Mon Nov 9 16:31:57 2020 +0900
sched: task: Fix nxtask_exit() for SMP
Summary:
- I noticed that nxsched_merge_pending() is called outside a critical section
- The issue happens if a new rtcb does not hold a critical section
- Actually, global IRQ control is done in nxsched_resume_scheduler() in nxtask_exit()
- However, nxsched_merge_pending() was called after calling nxsched_resume_scheduler()
- This commit fixes the issue by moving nxsched_merge_pending() before the function
- NOTE: the sequence was changed for SMP but works for non-SMP as well
Impact:
- This commit affects both SMP and non-SMP
Testing:
- Tested with ostest with the following configurations
- spresense:wifi_smp (NCPUS=2 and 4)
- spresense:wifi (non SMP)
- sabre-6quad:smp (QEMU)
- esp32-core:smp (QEMU)
- maix-bit:smp (QEMU)
- sim:smp
- lc823450-xgevk:rndis
Signed-off-by: Masayuki Ishikawa <Ma...@jp.sony.com>
---
sched/task/task_exit.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/sched/task/task_exit.c b/sched/task/task_exit.c
index 6607f49..d570e45 100644
--- a/sched/task/task_exit.c
+++ b/sched/task/task_exit.c
@@ -102,6 +102,15 @@ int nxtask_exit(void)
nxsched_remove_readytorun(dtcb);
+ /* If there are any pending tasks, then add them to the ready-to-run
+ * task list now
+ */
+
+ if (g_pendingtasks.head != NULL)
+ {
+ nxsched_merge_pending();
+ }
+
/* Get the new task at the head of the ready to run list */
#ifdef CONFIG_SMP
@@ -190,14 +199,5 @@ int nxtask_exit(void)
}
#endif
- /* If there are any pending tasks, then add them to the ready-to-run
- * task list now
- */
-
- if (g_pendingtasks.head != NULL)
- {
- nxsched_merge_pending();
- }
-
return ret;
}