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.
