Summary:
- During investigating critical section with semaphores, I noticed
that nxtask_flushstreams() is called with a critical section.
- The function calls lib_flushall() which handles a semaphore
in userspace.
- So it should be done without a critical section
Impact:
- SMP only
Testing:
- Tested with ostest the following configs
- esp32-devkitc:smp (QEMU), sabre-6quad:smp (QEMU)
- maix-bit:smp (QEMU), sim:smp
- spresense:smp
- Tested with nxplayer and stress test with spresense:wifi_smp
Signed-off-by: Masayuki Ishikawa <Masayuki.Ishikawa@jp.sony.com>
Otherwise, long-sleeping system calls using nxsig_timedwait
are not actually interrupted.
Tested with "ntpcstop" using lm3s6965-ek:qemu-protected config.
Summary:
- This commit changes spinlock APIs (spin_lock_irqsave/spin_unlock_irqrestore)
- In the previous implementation, the global spinlock (i.e. g_irq_spin) was used.
- This commit allows to use caller specific spinlock but also supports to use
g_irq_spin for backword compatibility (In this case, NULL must be specified)
Impact:
- None
Testing:
- Tested with the following configurations
- spresnse:wifi, spresense:wifi_smp
- esp32-devkitc:smp (QEMU), sabre6-quad:smp (QEMU)
- maxi-bit:smp (QEMU), sim:smp
- stm32f4discovery:wifi
Signed-off-by: Masayuki Ishikawa <Masayuki.Ishikawa@jp.sony.com>
Summary:
- During testing iperf -s with lc823450-xgevk:rndis,
I noticed that hard fault happens
- Finally, I found that group_kill_children() is not
not protected by a critical section
- This commit fixes this issue
Impact:
- SMP only
Testing:
- Tested with iperf with the following configurations
- lc823450-xgevk:rndis, spresense:rndis_smp
- Tested with ostest with the following configurations
- lc823450-xgevk:rndis, esp32-devkitc:smp (QEMU)
- sabre-6quad:smp (QEMU), maix-bit:smp (QEMU)
- spresense:rndis_smp, sim:smp
Signed-off-by: Masayuki Ishikawa <Masayuki.Ishikawa@jp.sony.com>
Summary:
- This commit fixes comments and label in nxtask_assign_pid()
Impact:
- None
Testing:
- Built with spresense:wifi
Signed-off-by: Masayuki Ishikawa <Masayuki.Ishikawa@jp.sony.com>
Summary:
- During reviewing sched_lock() in nxtask_assign_pid(),
I noticed that g_pidhash is not protected by a critical section
- Because g_pidhash is accessed in an interrupt context,
it should be protected by a critical section.
- Actually, nxsched_foreach(), nxsched_get_tcb() and so on
use a critical section.
Impact:
- No impact
Testing:
- Tested with spresense:wifi (non-SMP) and spresense:wifi_smp
Signed-off-by: Masayuki Ishikawa <Masayuki.Ishikawa@jp.sony.com>
Summary:
- Because this_task() and nxsched_get_tcb() are protected inside the funtcions
- Also, enter_critical_section() is used in nxsched_set_affinity()
Impact:
- No impact
Testing:
- Tested with spresense:wifi_smp
Signed-off-by: Masayuki Ishikawa <Masayuki.Ishikawa@jp.sony.com>
let's call either nx_dup/nx_dup2 or file_dup/file_dup2
instead just like other fs api: xxx->nx_xxx->file_xxx
Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
Change-Id: I1aacfb9e25dc7b3fcb0345ff7b269b1953a01e5b
1. remove descript management in mqueue, save code size
2. use i_ops instead of i_mqueue to remove the dup logic
Change-Id: Ie88960e50ddcae9c87977c9ad65a45297c663291
Signed-off-by: ligd <liguiding1@xiaomi.com>
Summary:
- Calling sched_lock()/unlock() is unnecessary for SMP
because we've been using the critical section API
Impact:
- SMP only
Testing:
- Tested with smp and ostest with the following configurations
- sabre-6quad:smp (QEMU, dev board)
- spresense:smp, spresense:wifi_smp
- sim:smp, sim:ostest
- maix-bit:smp (QEMU)
- esp32-devkitc:smp (QEMU)
- lc823450-xgevk:rndis
Signed-off-by: Masayuki Ishikawa <Masayuki.Ishikawa@jp.sony.com>
use the default testset implement on single core platform
that does not support test-and-set, more flexibility for
SMP drivers(using spinlock) if configured in a single-core mode.
Signed-off-by: chao.an <anchao@xiaomi.com>
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.Ishikawa@jp.sony.com>
the action can't be dynamically allocated in any irq handler
so it's important to let the user extend the number as needed
Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
Change-Id: Id30c5d93003e63514c24f2ca0df2f634c4c63c5f
the message can't be dynamically allocated in any irq handler
so it's important to let the user extend the number as needed
Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
Change-Id: Ia26584c3815bac6cf24de4c88be0844ac8e8fba2
Summary:
- I found potential bugs in pthread_condclockwait.c
- Actually, sched_unlock() and wd_cancel() were misplaced.
- This commit fixes this issue
Impact:
- pthread_cond_clockwait() only
Testing:
- Tested with ostest with the following configs
- sabre-6quad:nsh, sabre-6quad:smp, sim:ostest
Signed-off-by: Masayuki Ishikawa <Masayuki.Ishikawa@jp.sony.com>
Summary:
- This commit introduces SP_WFE() and SP_SEV() to be used for spinlock
- Also, use wfe/sev instructions for ARMV7-A to reduce power consumption
Impact:
- ARMV7-a SMP only
Testing:
- sabre-6quad:smp (QEMU, dev board)
- maix-bit:smp, esp32-devkitc:smp, spresense:smp sim:smp (compile only)
Signed-off-by: Masayuki Ishikawa <Masayuki.Ishikawa@jp.sony.com>
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 <Masayuki.Ishikawa@jp.sony.com>
sched/signal/sig_timedwait.c:
* nxsig_timedwait(): Fix typo in comment:
"is store din" -> "is stored in"
include/nuttx/signal.h:
* Fix same typo in extern declaration of nxsig_timedwait().
Summary:
- I noticed that Cortex-A SGI can be masked
- We thought the SGI is not maskable
- Although I can not remember how I tested it before
- It actually works as expected now
- Also, fixed the number of remaining bugs in TODO
Impact:
- No impact
Testing:
- Tested with sabre-6quad:smp (QEMU and dev board)
- Add the following code in up_idle() before calling asm("WFI");
+ if (0 != up_cpu_index())
+ {
+ up_irq_save();
+ }
- Run the hello app, you can see "Hello, World!!"
- But nsh will freeze soon because arm_pause_handler is not called.
Signed-off-by: Masayuki Ishikawa <Masayuki.Ishikawa@jp.sony.com>
Summary:
- I noticed waitpid_test stops with lc823450-xgevk:rndis
- The condition was CONFIG_DEBUG_ASSERTION=y
- Actually, the child task sent SIGCHILD but the parent couldn't catch the signal
- Then, I found that nx_waitid(), nx_waitpid() use sched_lock()
- However, a parent task and a child task are running on different CPUs
- So, sched_lock() is not enough and need to use a critical section
- Also, signal handling in nxtask_exithook() must be done in a critical section
Impact:
- SMP only
Testing:
- Tested with ostest with the following configurations
- lc823450-xgevk:rndis (CONFIG_DEBUG_ASSERTION=y and n)
- spresense:smp
- spresense:wifi_smp (NCPUS=2 and 4)
- sabre-6quad:smp (QEMU)
- esp32-core:smp (QEMU)
- maix-bit:smp (QEMU)
- sim:smp
Signed-off-by: Masayuki Ishikawa <Masayuki.Ishikawa@jp.sony.com>
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 <Masayuki.Ishikawa@jp.sony.com>
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 <Masayuki.Ishikawa@jp.sony.com>
Reference here:
https://pubs.opengroup.org/onlinepubs/009695399/functions/timer_create.html
DESCRIPTION
...
Each implementation shall define a set of clocks that can be
used as timing bases for per-process timers. All implementations
shall support a clock_id of CLOCK_REALTIME.
*** If the Monotonic Clock option is supported, implementations shall
support a clock_id of CLOCK_MONOTONIC. ***
...
Change-Id: Ia8e7302ed4a7e9ec11a0059bd68e9674ea942001
Signed-off-by: chao.an <anchao@xiaomi.com>
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 <Masayuki.Ishikawa@jp.sony.com>
In file included from string/lib_strsignal.c:40:0:
string/lib_strsignal.c: In function ‘strsignal’:
string/lib_strsignal.c:142:12: error: ‘CONFIG_SIG_STOP’ undeclared (first use in this function); did you mean ‘CONFIG_SIG_PIPE’?
case SIGSTOP:
^
string/lib_strsignal.c:142:12: note: each undeclared identifier is reported only once for each function it appears in
string/lib_strsignal.c:147:12: error: ‘CONFIG_SIG_STP’ undeclared (first use in this function); did you mean ‘CONFIG_SIG_STOP’?
case SIGSTP:
Esnure all standard signal number is always defined
Change-Id: I3abce86079ebeba7bab038d7c770efc90b9cffd7
Signed-off-by: ligd <liguiding1@xiaomi.com>
Summary:
- I found a deadlock during Wi-Fi audio streaming test plus stress test
- The testing environment was spresense:wifi_smp (NCPUS=4)
- The deadlock happened because two CPUs called up_cpu_pause() almost simultaneously
- This situation should not happen, because up_cpu_pause() is called in a critical section
- Actually, the latter call was from nxsem_post() in an IRQ handler
- And when enter_critical_section() was called, irq_waitlock() detected a deadlock
- Then it called up_cpu_paused() to break the deadlock
- However, this resulted in setting g_cpu_irqset on the CPU
- Even though another CPU had held a g_cpu_irqlock
- This situation violates the critical section and should be avoided
- To avoid the situation, if a CPU sets g_cpu_irqset after calling up_cpu_paused()
- The CPU must release g_cpu_irqlock first
- Then retry irq_waitlock() to acquire g_cpu_irqlock
Impact:
- Affect SMP
Testing:
- Tested with spresense:wifi_smp (NCPUS=2 and 4)
- Tested with spresense:smp
- Tested with sim:smp
- Tested with sabre-6quad:smp (QEMU)
- Tested with maix-bit:smp (QEMU)
- Tested with esp32-core:smp (QEMU)
- Tested with lc823450-xgevk:rndis
Signed-off-by: Masayuki Ishikawa <Masayuki.Ishikawa@jp.sony.com>
Follow the POSIX description.
SIGTSTP should be sent when the Ctrl-Z characters is encountered, not SIGSTP.
Testing:
Built with hifive1-revb:nsh (CONFIG_SERIAL_TERMIOS=y, CONFIG_SIG_DEFAULT=y and CONFIG_TTY_SIGTSTP=y)
Summary:
- I noticed sched_lock() logic is different from sched_unlock()
- I think sched_lock() should use critical section
- Also, the code should be simple like sched_unlock()
- This commit fixes these issues
Impact:
- Affects SMP only
Testing:
- Tested with spresense:wifi_smp (both NCPUS=2 and 3)
- Tested with lc823450-xgevk:rndis
- Tested with maix-bit:smp (QEMU)
- Tested with esp32-core:smp (QEMU)
- Tested with sabre-6quad:smp (QEMU)
Signed-off-by: Masayuki Ishikawa <Masayuki.Ishikawa@jp.sony.com>
Summary:
- I noticed DEBUGASSERT() happens in sched_unlock()
- The test was Wi-Fi audio streaming stress test with spresense 3cores
- Actually, g_cpu_schedlock was locked but g_cpu_lockset was incorrect
- Finally, I found that cpu was obtained before enter_critical_section()
- And the task was moved from one cpu to another cpu
- However, that call should be done within the critical section
- This commit fixes this issue
Impact:
- Affects SMP only
Testing:
- Tested with spresense:wifi_smp (both NCPUS=2 and 3)
- Tested with lc823450-xgevk:rndis
- Tested with maix-bit:smp (QEMU)
- Tested with esp32-core:smp (QEMU)
- Tested with sabre-6quad:smp (QEMU)
Signed-off-by: Masayuki Ishikawa <Masayuki.Ishikawa@jp.sony.com>
to ensure the basic info(e.g. pid) setup correctly before call arch API
Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
Change-Id: I851cb0fdf22f45844938dafc5981b3f576100dba
Summary:
- During Wi-Fi audio streaming test, I found a deadlock in nxtask_exit()
- Actually, nxtask_exit() was called and tried to enter critical section
- In enter_critical_section(), there is a deadlock avoidance logic
- However, if switched to a new rtcb with irqcount=0, the logic did not work
- Because the 2nd critical section was treated as if it were the 1st one
- Actually, it tried to run the deadlock avoidance logic
- But nxtask_exit() was called with critical section (i.e. IRQ already disabled)
- So the logic did not work as expected because up_irq_restore() did not enable the IRQ.
- This commit fixes this issue by incrementing irqcount before calling nxtask_terminate()
- Also it adjusts g_cpu_irqlock and g_cpu_lockset
Impact:
- Affects SMP only
Testing:
- Tested with spresense:wifi_smp (smp, ostest, nxplayer, telnetd)
- Tested with sabre-6quad:smp with QEMU (smp, ostest)
- Tested with maix-bit:smp with QEMU (smp, ostest)
- Tested with esp32-core:smp with QEMU (smp, ostest)
Signed-off-by: Masayuki Ishikawa <Masayuki.Ishikawa@jp.sony.com>
since nxtask_startup will initialize c++ global variables which shouldn't
be done inside the kernel thread
Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
Move sched/task/task/task_gettid.c to libs/libc/unistd/lib_gettid.c. gettid() is a dumb wrapper around getpid(). It is wasteful of resources to support TWO systme calls, one for getpid() and one for gettid(). Instead, move gettid() in the C library where it calls the single sysgtem call, getpid(). Much cleaner.
1.Reduce the default size of task_group_s(~512B each task)
2.Scale better between simple and complex application
Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
Change-Id: Ia872137504fddcf64d89c48d6f0593d76d582710
Summary:
- ARCH_GLOBAL_IRQDISABLE was initially introduced for LC823450 SMP
- At that time, i.MX6 (quad Cortex-A9) did not use this config
- However, this option is now used for all CPUs which support SMP
- So it's good timing for refactoring the code
Impact:
- Should have no impact because the logic is the same for SMP
Testing:
- Tested with board: spresense:smp, spresense:wifi_smp
- Tested with qemu: esp32-core:smp, maix-bit:smp, sabre-6quad:smp
- Build only: lc823450-xgevk:rndis, sam4cmp-db:nsh
Signed-off-by: Masayuki Ishikawa <Masayuki.Ishikawa@jp.sony.com>
it is useful to pass the nonempty argument to change the init task behaviour
Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
Change-Id: I684e9c76b9eac54404d0e4e63ab78e51e039c9a8
to save the preserved space(1KB) and also avoid the heap overhead
Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
Change-Id: I694073f68e1bd63960cedeea1ddec441437be025
Change the preallocated message and descriptor from 32/24 to 4.
The total size is reduce from 1892 to 532
Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
Change-Id: I79d199465daef678986868f773876289859f42fc
1.Don't preallocate sigaction list since it's used only in the task context
2.Reduce the preserved item which is used only in the task context from 16 to 4
The total memory decrease from 1280B to 480B
Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
Change-Id: Ib5d5a7365c7d443fc0e99c0d3ea943e85f67ca8c
since the maximum number of argument pass to wd_start in the whole
code base is 2 and change CONFIG_MAX_WDOGPARMS in some defconfig
from 1 to 2 oherwise pthread_condclockwait will fail
Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
Change-Id: Ib6cb28b8c0722058849e7be916e164513431d21c
Found by clang-check:
sched/sched_waitpid.c:380:33: warning: Although the value stored to 'ret' is used in the enclosing expression, the value is never actually read from 'ret'
(pid != (pid_t)-1 && (ret = nxsig_kill(pid, 0)) < 0))
^ ~~~~~~~~~~~~~~~~~~
1 warning generated.
because nx_task_idle doesn't call sched_note_start. To avoid the
same error happen again in the furture, nx_task_idle is removed.
Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
Make.dep file should be updated by .config changed after first make.
There are 2 cases affected for this problem:
1) Add source files by config symbol
2) Include header files in #ifdef directive
These 2 cases may not be included in Make.dep and this may prevent the
differential build from working correctly.
Since up_release_stack auto detect whether the memory come from builtin heap
if (ttype == TCB_FLAG_TTYPE_KERNEL)
{
if (kmm_heapmember(dtcb->stack_alloc_ptr))
{
kmm_free(dtcb->stack_alloc_ptr);
}
}
else
{
/* Use the user-space allocator if this is a task or pthread */
if (umm_heapmember(dtcb->stack_alloc_ptr))
{
kumm_free(dtcb->stack_alloc_ptr);
}
}
This reverts commit 124e6ee53d.
add PR_SET_NAME_EXT/PR_GET_NAME_EXT extension to avoid semantic
conflicts, use extened version for pthread_setname_np/pthread_getname_np
Change-Id: I40404c737977a623130dcd37feb4061b5526e466
Signed-off-by: chao.an <anchao@xiaomi.com>
to avoid the similar code spread around each application
Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
Change-Id: I8967d647eaf2ecae47f29f83e7fa322ef1b42a02
change the stack pointer type from (uint32_t *) to (void *)
Change-Id: I90bb7d6a9cb0184c133578a1a2ae9a19c233ad30
Signed-off-by: chao.an <anchao@xiaomi.com>
If there's no child status available immediately,
return 0 without blocking as specified by the standards.
I checked the following version of the standard.
I believe it has always been this way though.
The Open Group Base Specifications Issue 7, 2018 edition
IEEE Std 1003.1-2017 (Revision of IEEE Std 1003.1-2008)
If there's no child status available immediately,
return 0 without blocking as specified by the standards.
The implementation for non CONFIG_SCHED_HAVE_PARENT case
seems ok in this regard.
I checked the following version of the standard.
I believe it has always been this way though.
The Open Group Base Specifications Issue 7, 2018 edition
IEEE Std 1003.1-2017 (Revision of IEEE Std 1003.1-2008)
Prohibit use of pthread_cleanup API's by kernel threads. The pthread pthread_cleanup functions MUST run in user mode, making them unusable for kernel threads.
See Issue #1263
since it is impossible that the current running thread is
in the waiting state and then need to wake up self.
Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
Change-Id: Ie2ba55c382eb3eb7c8d9f04bba1b9e294aaf6196
utilize the call inside nxtask_exit instead, also move
nxsched_suspend_scheduler to nxtask_exit for symmetry
Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
Change-Id: I219fc15faf0026e452b0db3906aa40b40ac677f3
In the FLAT build if CONFIG_LIB_SYSCALL=y, then the function task_spawn() will be duplicated.: One version in libs/libc/spawn and one version in sched/task.
The version of task_spawn in lib/libc/spawn exists only if CONFIG_LIB_SYSCALL is selected. In that case, the one in sched/task/task_spawn.c should be static, at least in the FLAT build.
The version of task_spawn.c in libs/libc/spawn simply marshals the parameters into a structure and calls nx_task_spawn(). If CONFIG_LIB_SYSCALL is defined then nx_task_spawn() will un-marshal the data can call the real task spawn. This nonsense is only necessary because task_spawn has 8 parameters and the maximum number of parameters in a system call is only 6.
Without syscalls: Application should call directly in task_spawn() in sched/task/task_spawn.c and, hence, it must not be static
With syscalls: Application should call the marshalling task_spawn() in libs/libc/spawn/lib_task_spawn.c -> That will call the autogenerated nx_task_spawn() proxy -> And generate a system call -> The system call will the unmarshalling nx_task_spawn() in sched/task/task_spawn.c -> Which will, finally, call the real task_spawn().
The side-effect of making task_spawn() static is that it then cannot be used within the OS. But as far as I can tell, nothing in the OS itself currently uses task_spawn() so I think it is safe to make it conditionally static. But that only protects from duplicate symbols in the useless case mentioned above.
Block and MTD drivers may be opened and managed as though they were character drivers. But this is really sleight of hand; there is a hidden character driver proxy that mediates the interface to the block and MTD drivers in this case.
fstat(), however, did not account for this. It would report the characteristics of the proxy character driver, not of the underlying block or MTD driver.
This change corrects that. fstat now checks if the character driver is such a proxy and, if so, reports the characteristics of the underlying block or MTD driver, not the proxy character driver.
sched_releasetcb() will normally free the stack allocated for a task. However, a task with a custom, user-managed stack may be created using nxtask_init() followed by nxtask_activer. If such a custom stack is used then it must not be free in this many or a crash will most likely result.
This chagne addes a flag call TCB_FLAG_CUSTOM_STACK that may be passed in the the pre-allocted TCB to nxtask_init(). This flag is not used internally anywhere in the OS except that if set, it will prevent sched_releasetcb() from freeing that custom stack.
Add trivial function nxtask_uninit(). This function will undo all operations on a TCB performed by task_init() and release the TCB by calling kmm_free(). This is intended primarily to support error recovery operations after a successful call to task_init() such was when a subsequent call to task_activate fails.
That error recovery is trivial but not obvious. This helper function should eliminate confusion about what to do to recover after calling nxtask_init()
-Move task_init() and task_activate() prototypes from include/sched.h to include/nuttx/sched.h. These are internal OS functions and should not be exposed to the user.
-Remove references to task_init() and task_activate() from the User Manual.
-Rename task_init() to nxtask_init() since since it is an OS internal function
-Rename task_activate() to nxtask_activate since it is an OS internal function
Functions within the OS must never set the errno value. fs_fdopen() was setting the errno value. Now, after some parameter changes, it reports errors via a negated errno integer return value as do most all other internal OS functions.
1. Internal scheduler functions should begin with nxsched_, not sched_
2. Follow the consistent naming patter of https://cwiki.apache.org/confluence/display/NUTTX/Naming+of+OS+Internal+Functions
# clock_systimer -> clock_systime_tick
# clock_systimespec -> clock_systime_timespec
sched_oneshot_extclk -> nxsched_oneshot_extclk
sched_period_extclk -> nxsched_period_extclk
# nxsem_setprotocol -> nxsem_set_protocol
# nxsem_getprotocol -> nxsem_get_protocol
# nxsem_getvalue -> nxsem_get_value
nxsem_initholders -> nxsem_initialize_holders
nxsem_addholder -> nxsem_add_holder
nxsem_addholder_tcb -> nxsem_add_holder_tcb
nxsem_boostpriority -> nxsem_boost_priority
nxsem_releaseholder -> nxsem_release_holder
nxsem_restorebaseprio -> nxsem_restore_baseprio
Some planned name changed were skipped for now because they effect too many files (and would require many hours of coding style fixups).
All complaints fixed except for those that were not possible to fix:
- Used of Mixed case identifier in ESP32 files. These are references to Expressif ROM functions which are outside of the scope of NuttX.
Linux Programmer's Manual:
SEM_POST(3)
NAME
sem_post - unlock a semaphore
...
ERRORS
...
EOVERFLOW
The maximum allowable value for a semaphore would be exceeded.
Change-Id: I57c1a797a5510df4290a10aa2f3106fd01754b37
Signed-off-by: chao.an <anchao@xiaomi.com>
1. Add missing conditional logic in include/sys/syscall_lookup.h
2. CONFIG_NPTHREAD_KEYS removed from code but was still in sched/Kconfig
3. Refresh all configurations affected by PR 1007
4. syscall/syscall_funclookup.c needs to include nuttx/tls.h
1. Move pthread-specific data files from sched/pthread/ to libs/libc/pthread.
2. Remove pthread-specific data functions from syscalls.
3. Implement tls_alloc() and tls_free() with system calls.
4. Reimplement pthread_key_create() and pthread_key_free() using tls_alloc() and tls_free().
5. Reimplement pthread_set_specific() and pthread_get_specicif() using tls_set_value() and tls_get_value()
- Remove per-thread errno from the TCB structure (pterrno)
- Remove get_errno() and set_errno() as functions. The macros are still available as stubs and will be needed in the future if we need to access the errno from a different address environment (KERNEL mode).
- Add errno value to the tls_info_s structure definitions
- Move sched/errno to libs/libc/errno. Replace old TCB access to the errno with TLS access to the errno.
Revert a portion of eca7059785 that
causes compiler warnings about unused variables if nx_start() is not
initializing any of the user-mode heap, kernel-mode heap, or page
allocator:
init/nx_start.c: In function 'nx_start':
init/nx_start.c:552:14: warning: unused variable 'heap_size'
[-Wunused-variable]
size_t heap_size;
^~~~~~~~~
init/nx_start.c:551:17: warning: unused variable 'heap_start'
[-Wunused-variable]
FAR void *heap_start;
^~~~~~~~~~
See dev@nuttx.apache.org mailing list discussion "New unused variables
warning in nx_start()" starting 6 May 2020, archived here:
https://lists.apache.org/thread.html/r3900727e6a06f4445d6eb881d065119ba6647daab89600c3d45d1424%40%3Cdev.nuttx.apache.org%3E
sched/init/nx_start.c:
* If none of MM_KERNEL_USRHEAP_INIT, CONFIG_MM_KERNEL_HEAP, or
CONFIG_MM_PGALLOC are defined, the variables heap_start and
heap_size were declared but never used.
* This change reinstates wrapping the block with a preprocessor
conditional to prevent the variables being declared if they will
not be used. This preprocessor condition was removed in the
above-mentioned commit.
There is a DEBUGPANIC in some logic. This happens if a a task exists at certain points with priority inheritance enabled. This event was not expected in the original design (although logic was provided to support it). Since, apparently, it does happen, the DEBUGPANIC must be removed.
Noted by Brennan Ashton.
The sched_get_stackinfo() interface was just added. However, it occurs to me that it is a dangerous feature and could lead to security problems. In FLAT and PROTECTED modes, if you get access to any other threads stack, you could do harm.
This commit adds some level of security. Basically, it implements these rules:
1. Any thread may query its own stack,
2. A kernel thread may query the stack of any other thread
3. Application threads, however, may query only the stacks of threads within the same task group, i.e., the main thread and any of the child pthreads created with the main thread as a parent or grandparent or great-grandpart ...
The new OS interface, sched_get_stackinfo() combines two pthread-specific interfaces into a single generic interface. The existing pthread_get_stackaddr_np() and pthread_get_stacksize_np() are moved from sched/pthread to libs/libc/pthread.
There are two motivations for this change: First, it reduces the number of system calls. Secondly, it adds a common hook that is going to used for a future implementation of TLS.
Sockets are created in two steps:
1. The socket is allocated, then
2. The socket is initialized.
In SMP mode, there is a possibility that a pthread executing one CPU may create a new task while a pthread on another CPU has allocated the socket but not yet initialized it. This commit updates the socket clone test to assure that the socket is both allocated and initailized.
Without the change, it is possible that uninitialized sockets could be cloned, leading to errors later in the newly started task.
These warnings fix a class of warnings that I saw during CI checks for macOS sim builds. For example:
devif/devif_callback.c:111:49: warning: for loop has empty body [-Wempty-body]
prev = curr, curr = curr->nxtdev);
^
devif/devif_callback.c:111:49: note: put the semicolon on a separate line to silence this warning
I did not put the semi-colon on a separate line, but used braces.
This commit resolves issue #620:
Remove CONFIG_CAN_PASS_STRUCTS #620
The configuration option CONFIG_CAN_PASS_STRUCTS was added many years ago to support an old version of the SDCC compiler. That compiler is currently used only with the Z80 and Z180 targets. The limitation of that old compiler was that it could not pass structures or unions as either inputs or outputs. For example:
#ifdef CONFIG_CAN_PASS_STRUCTS
struct mallinfo mallinfo(void);
#else
int mallinfo(FAR struct mallinfo *info);
#endif
And even leads to violation of a few POSIX interfaces like:
#ifdef CONFIG_CAN_PASS_STRUCTS
int sigqueue(int pid, int signo, union sigval value);
#else
int sigqueue(int pid, int signo, FAR void *sival_ptr);
#endif
This breaks the 1st INVIOLABLES rule:
Strict POSIX compliance
-----------------------
o Strict conformance to the portable standard OS interface as defined at
OpenGroup.org.
o A deeply embedded system requires some special support. Special
support must be minimized.
o The portable interface must never be compromised only for the sake of
expediency.
o Expediency or even improved performance are not justifications for
violation of the strict POSIX interface
Also, it appears that the current SDCC compilers have resolve this issue and so, perhaps, this is no longer a problem: z88dk/z88dk#1132
NOTE: This commit cannot pass the PR checks because it depends on matching changes to the apps/ directory.
Hello,
I am testing priority inversion feauture of Nuttx scheduler. I encounter with problem in tests.
Here is problem:
nxsched_readytorun_setpriority() function not works as expected when DEBUG_ASSERTION not enabled. Because sched_removereadytorun() and sched_addreadytorun() is not called in this case. These functions wrapped with DEBUGASSERT macro and with the effect of DEBUGASSERT macro, sched_removereadytorun() and sched_addreadytorun() not called when DEBUG_ASSERTION not enabled. Therefore priority inversion not works as expected.
Could you check this problem ? Is this a bug, if not what is the purpose of this code ?
Thanks,
Şükrü Bahadır Arslan
A mutex may be configured with rather exotic options such as recursive, unsafe, etc. The availability of these mutex options is controlled by configuation settings. When each option is enabled, additional fields are managed inside of the mutex structure.
pthread_cond_wait() and pthread_timed_wait() do the following atomically: (1) unlock the mutex, (2) wait for the condition, and (3) restore the mutex lock. When that lock is restored, pthread_cond_[timed]wait() must also restore the exact configuration of the mutex data structure if these "exotic" features are enabled.
Now that nxsem_wait_uninterruptible() returns an error when the thread is canceled, new logic is being executed. This revealed an existing error in pthread_cond_wait(). The nature of the error is as follows:
* pthread_cond_wait() must atomically (1) release the mutex, (2) wait on the condition, and (3) re-establish the mutex. Errors can occur on any of these steps. In this case the ECANCELED error was (very correctly) reported by nxsem_wait_uninterruptible().
* However, logic in step (3) had a bug; it re-acquired the mutex, but then a bug in existing logic cause the mutex to be improperly initialized. Essentially, the wrong error status value was being testing. This resulted in a nonsequitar and errors reported by the OS test.
This problem was found by apps/testing/ostest/pthread_cleanup.c
Resolution of Issue 619 will require multiple steps, this part of the first step in that resolution: Every call to nxsem_wait_uninterruptible() must handle the return value from nxsem_wait_uninterruptible properly. This commit is only for those files under graphics/, mm/, net/, sched/, wireless/bluetooth.
Still to do: Files under fs/, drivers/, and arch. The last is 116 files and will take some effort.
nxsem_timedwait_uninterruptible() must return -ECANCELED if the thread is canceled:
include/nuttx/semaphore.h: Return if nxsem_wait() returns ECANCELED meaning that the thread waiting for the semaphore has been canceled.
sched/semaphore/sem_timedwait.c: Same change (the inline version is in semaphore.h, the non-inlined version is in sem_tickwait.c).
drivers/sensors/lps25h.c and drivers/wireless/bluetooth/bt_uart_bcm4343x.c: Make sure that the caller deals correctly with the -ECANCELED return value.
Refer to issue 619.
* The appropriate size of stack varies among archs.
E.g. for 64-bit sim, 2048 is way too small, especially when the task
happens to use host OS functionalities.
I plan to allow an arch provide its own default.
* I plan to use this to replace hardcoded "STACKSIZE = 2048" in APPDIR.
Move the prototype for the internal OS from from sched/sched/sched.h to include/nuttx/sched.h. This was done because binfmt/binfmt/excecmodule.c requires the prototype for sched_releasetcb() and was illegally including sched/sched/sched.h. That is a blatant violation of the OS modular design and the person that did this should be hung up by their thumbs. Oh... I did that back in a bad moment in 2014. Now that is made right.
Update all files possible under sched/timer to Apache 2.0. These excludes two files that have a Xiaomi copyright. Also, run all modified files through nxstyle to assure that the conform to the coding standard.
PR #488 introduced an error in a file license header in file sched/mqueue/mqueue.h. This commit fixe that error and in the course of doing that, changing the license header to the standard Apache 2.0 header. I am the author and copyright hold of all modified files.
