Rename atomic_fetch_add/sub/or/and/xor to atomic_add/sub/or/and/xor
to avoid conflicts with the C/C++ standard library naming. The
atomic_fetch_xxx naming is reserved by the standard; keeping it causes
function name conflicts when source files indirectly include both
<nuttx/atomic.h> and <atomic>/<stdatomic.h>.
Signed-off-by: zhangyu117 <zhangyu117@xiaomi.com>
A mutex records its holder as a task id in the low 31 bits of a word
whose top bit means "someone is blocked on this". The id was stored
without masking, so an id with its top bit set became a holder with the
blocking bit raised.
Task ids are normally small and positive, but not always.
nxsched_gettid() reports -ESRCH for a context that no longer maps to a
running task, and there is a window where that is exactly what the
running context is: nxtask_exit() marks the next task ready to run
while the dying task is still executing on its own stack, and only then
releases the TCB. Freeing the group inside that release takes and
drops the group's mutexes, so the lock stores 0xfffffffd and the unlock
compares 0x7ffffffd, which are not equal.
With assertions enabled the unlock trips its holder check, and every
exit of a process that frees memory panics. In a kernel build that is
every exit, so no program could be run twice, and running one at all
took the shell down with it. Without assertions the failure is silent:
the accidental blocking bit sends the unlock looking for a waiter that
never existed.
Encode the id the same way everywhere it is stored or compared, so that
a lock and an unlock from one context agree whatever the id's sign.
The masked forms of -1 and -2 would alias the "no holder" and "reset"
values, but nxsched_gettid() yields only valid ids and -ESRCH.
mm_lock() already sidesteps this window with a note that gettid() may
return -ESRCH during a context switch; this gives the generic mutex the
same footing rather than a second special case.
Test case, on the EIC7700 EVB, which is a kernel build with assertions:
nsh> hello
Hello, World!!
Before, that printed and then panicked in sem_post, taking the shell
with it, every time. After, five runs in a row complete and the shell
survives. ps over telnet still completes.
Assisted-by: Claude:claude-opus-5
Signed-off-by: Justin Hammond <justin@dynam.ac>
Clear NXSEM_MBLOCKING_BIT in nxsem_wait_irq() when a mutex waiter is
removed and the wait queue becomes empty.
Signed-off-by: Martin Krasula <mkrasula@elektroline.cz>
Correct build errors when CONFIG_ENABLE_ALL_SIGNALS is not defined
- sched makefiles: Move pending-signal helpers from the ENABLE_ALL_SIGNALS-only
list to the !DISABLE_ALL_SIGNALS list so signal dispatch is available in
PARTIAL builds sched: make SIG_PREALLOC_ACTIONS, SIG_ALLOC_ACTIONS and
SIG_DEFAULT depend on ENABLE_ALL_SIGNALS
- sched: fix ifdefs around pending-signal queue access and signal-mask for
PARTIAL/DISABLE modes
- arch: gate SYS_signal_handler / _return calls and SYSCALL_LOOKUP(signal)
with ENABLE_ALL_SIGNALS
Signed-off-by: Jukka Laitinen <jukka.laitinen@tii.ae>
debug.h is a NuttX-specific, non-POSIX header. Placing it in the
top-level include/ directory creates naming conflicts with external
projects that define their own debug.h.
This commit moves the canonical header to include/nuttx/debug.h,
following the NuttX convention for non-POSIX/non-standard headers,
and updates all in-tree references.
A backward-compatibility shim is left at include/debug.h that
emits a deprecation #warning and re-includes <nuttx/debug.h>,
allowing out-of-tree code to continue building while migrating.
Signed-off-by: Piyush Patle <piyushpatle228@gmail.com>
A signal can arrive before sem_wait transitions the task
to TSTATE_WAIT_SEM. In that window, the wait cannot yet
be aborted by sem_wait_irq(). If sem_wait then blocks
without re-checking unmasked pending signals, it can
sleep indefinitely and miss the interrupt.
Check for unmasked pending signals before touching the
semaphore count and return -EINTR if one is already pending.
Signed-off-by: Shunchao Hu <ankohuu@gmail.com>
Add DEBUGASSERT in nxsem_wait_slow() to catch illegal mutex
recursion attempts. This helps identify bugs where a task tries
to lock a mutex it already holds, which is not allowed.
Signed-off-by: anjiahao <anjiahao@xiaomi.com>
This change replaces goto-based control flow with structured if-else blocks
in the up_read() function to comply with MISRA HIS coding standards while
maintaining identical functional behavior.
Signed-off-by: hujun5 <hujun5@xiaomi.com>
This change optimizes the reader-writer semaphore release process by consolidating
redundant context switch operations and improving code efficiency, reducing
unnecessary wake-ups during concurrent access scenarios.
Signed-off-by: Yongrong Wang <wangyongrong@xiaomi.com>
This change consolidates multiple return statements in down_read_trylock() into
a single exit point and replaces goto with if-else structure to reduce cyclomatic
complexity and comply with MISRA HIS coding standards.
Signed-off-by: hujun5 <hujun5@xiaomi.com>
This change replaces goto-based control flow with structured if-else blocks
in the down_read() function to comply with MISRA HIS coding standards while
maintaining identical functional behavior.
Signed-off-by: hujun5 <hujun5@xiaomi.com>
This change consolidates multiple return statements in init_rwsem() into a
single exit point by inverting error conditions and restructuring nested
if-else blocks for MISRA HIS compliance.
Signed-off-by: hujun5 <hujun5@xiaomi.com>
This change consolidates multiple return statements in down_write_trylock()
into a single exit point to reduce cyclomatic complexity and comply with MISRA
HIS coding standards for safety-critical embedded systems.
Signed-off-by: hujun5 <hujun5@xiaomi.com>
Refactor nxsem_post_slow() to consolidate multiple return statements into a single exit point
by introducing a result variable and wrapping semaphore operations in an error-check condition block.
This improves code structure and resolves Coverity HIS_metric_violation defect for MISRA HIS standards compliance.
Signed-off-by: hujun5 <hujun5@xiaomi.com>
Refactor nxsem_wait_irq() to consolidate multiple return statements into a single
exit point by inverting the mutex condition check. This improves code maintainability
and resolves Coverity HIS_metric_violation defect for better compliance with MISRA HIS standards.
Signed-off-by: hujun5 <hujun5@xiaomi.com>
Refactor sem_trywait.c to replace goto statements with structured control
flow to comply with MISRA HIS coding standards. This improves code clarity
and maintainability while preserving all functional behavior and performance
characteristics.
Signed-off-by: hujun5 <hujun5@xiaomi.com>
Consolidate CONFIG_PRIORITY_PROTECT checks by moving macro definitions to
their actual usage locations and removing duplicate/dead code in semaphore
implementation. This reduces code duplication and improves maintainability
across sem_wait, sem_trywait, and sem_post functions.
Signed-off-by: p-gaoxiang43 <p-gaoxiang43@xiaomi.com>
Refactor sem_reset.c to reduce nested function call depth by consolidating
internal helper functions and optimizing call chains. This addresses Coverity's
HIS_metric_violation(HIS_CALLS) warning to comply with MISRA HIS coding
standards for embedded systems safety and maintainability.
Signed-off-by: hujun5 <hujun5@xiaomi.com>
Remove redundant leave_critical_section() call in nxsem_trywait_slow() as the
critical section is properly released at the exit point via 'out' label, making
the intermediate call unnecessary and eliminating code duplication.
Signed-off-by: hujun5 <hujun5@xiaomi.com>
Curernt implementation default semaphore max allowed value to SEM_VALUE_MAX.
In some cases, user may want to change this, so provide a function to do this: sem_setmaxvalue
Signed-off-by: Chengdong Wang <wangchengdong@lixiang.com>
Change every occurence of up_switch_context to use this_task() as the first parameter.
"nxsched_add_readytorun" returns "true" if context switch is required. "nxsched_add_readytorun"
typically could only switch the assigned/running task to the one which is passed in as parameter.
But this is not strictly guaranteed in SMP; if other CPUs tweak with affinities or priorities,
it may be possible that the running task after the call is changed, but is some other
task from the readytorun list (and it should be, if there is higher priority one available or the
affinity of the added task prevents it to be scheduled in, but the previous head of the readytorun
list should run.
this_task() is always the correct one to switch to, since it always points to the tcb which was
just switched in by the nxsched_add_readytorun.
This is also a precursor to re-writing the SMP queue logic to remove pending lists for SMP.
Signed-off-by: Jukka Laitinen <jukka.laitinen@tii.ae>
There is no need to check the holder structure "counts". There are cases
where the counts may be greater than 1 when several tasks block
on the mutex, but there is always just one holder, which must be freed.
Signed-off-by: Jukka Laitinen <jukka.laitinen@tii.ae>
This is not a bug, but unnecessary code. If the mutex is no longer blocking,
the released thread will set the holder and clear the blocking bit in the end
of nxsem_wait.
Signed-off-by: Jukka Laitinen <jukka.laitinen@tii.ae>
Checking only adds a race condition. The checking if the wdog is active
or not must be done inside wd_cancel, where the proper spinlock is held
Signed-off-by: Jukka Laitinen <jukka.laitinen@tii.ae>
This commit added a macro function clock_delay2abstick to calculate the
absolute tick after the delay.
Signed-off-by: ouyangxiangzhen <ouyangxiangzhen@xiaomi.com>
This puts the mutex support fully inside nxsem, allowing
locking the mutex and setting the holder with single atomic
operation.
This enables fast mutex locking from userspace, avoiding taking
critical_sections, which may be heavy in SMP and cleanup
of nxmutex library in the future.
Signed-off-by: Jukka Laitinen <jukka.laitinen@tii.ae>
The task which is deleted should be removed from the semaphores waitlist,
if the task happens to be blocked.
Signed-off-by: Jukka Laitinen <jukka.laitinen@tii.ae>
This avoids unnecessary syscalls in memory protected builds, when mutex
lock/unlock can be done with only atomic counter access
Signed-off-by: Jukka Laitinen <jukka.laitinen@tii.ae>
When the semaphore priority flags is set to NONE, and the semaphore
is a mutex, the fast locking path can be used, even when
priority inheritance or priority protect are enabled globally.
Signed-off-by: Jukka Laitinen <jukka.laitinen@tii.ae>
The kernel mapping should be performed in sem_wait (thread level) as
virtual memory mappings cannot be added from interrupt, at least for now.
The reason?
kmm_map() depends on mm_map_add(), which in turn uses a mutex for mutual
exclusion. Using mutexes from interrupt level is not permitted.
Mapping tcb->waitobj into kernel virtual memory directly in sem_wait()
makes sense, since accessing tcb->waitobj via a user virtual address can
lead to unexpected results (the wrong mappings can be in place).
1. remove up_interrupt_context() check, which should be safe in interrupt context
2. remove sem instance check will be handle in nxsem_trywait()
Signed-off-by: chao an <anchao@lixiang.com>
Otherwise the free holder list will leak, causing either a crash due to
holder->htcb = NULL, or the free holder list becomes (erroneously) empty
even though most of the holder entries are free.
The holder list can be modified via interrupt so using addrenv_select is
not safe. Access the semaphore by mapping it into kernel virtual memory
instead.
The temporary mappings via addrenv_select() and addrenv_restore() simply
do not work from interrupt, so remove its usage and replace with kmap
which is safe.
Add sem_wait fast operations, use atomic to ensure
atomicity of semcount operations, and do not depend
on critical section.
Test with robot:
before modify:
nxmutex_lock cost: 78 ns
nxmutex_unlock cost: 82 ns
after modify:
nxmutex_lock cost: 28 ns
nxmutex_unlock cost: 14 ns
Signed-off-by: zhangyuan29 <zhangyuan29@xiaomi.com>
This reverts commit befe29801f.
Because a few regressions have been reported and
it likely will take some time to fix them:
* for some configurations, semaphore can be used on the special
memory region, where atomic access is not available.
cf. https://github.com/apache/nuttx/pull/14625
* include/nuttx/lib/stdatomic.h is not compatible with
the C11 semantics, which the change in question relies on.
cf. https://github.com/apache/nuttx/pull/14755
Move CONFIG_SEM_PREALLOCHOLDERS to include/semaphore.h to avoid undefined issues from occurring in other places as well.
Signed-off-by: cuiziwei <cuiziwei@xiaomi.com>
Add sem_wait fast operations, use atomic to ensure
atomicity of semcount operations, and do not depend
on critical section.
Test with robot:
before modify:
nxmutex_lock cost: 78 ns
nxmutex_unlock cost: 82 ns
after modify:
nxmutex_lock cost: 28 ns
nxmutex_unlock cost: 14 ns
Signed-off-by: zhangyuan29 <zhangyuan29@xiaomi.com>
set CONFIG_PRIORITY_INHERITANCE=y
set CONFIG_SEM_PREALLOCHOLDERS=0 or CONFIG_SEM_PREALLOCHOLDERS=8
#24 0x4dcab71 in __assert assert/lib_assert.c:37
#25 0x4d6b0e9 in nxsem_destroyholder semaphore/sem_holder.c:602
#26 0x4d80cf7 in nxsem_destroy semaphore/sem_destroy.c:80
#27 0x4d80db9 in sem_destroy semaphore/sem_destroy.c:120
#28 0x4dcb077 in nxmutex_destroy misc/lib_mutex.c:122
#29 0x4dc6611 in pipecommon_freedev pipes/pipe_common.c:117
#30 0x4dc7fdc in pipecommon_close pipes/pipe_common.c:397
#31 0x4ed4f6d in file_close vfs/fs_close.c:78
#32 0x6a91133 in local_free local/local_conn.c:184
#33 0x6a92a9c in local_release local/local_release.c:129
#34 0x6a91d1a in local_subref local/local_conn.c:271
#35 0x6a75767 in local_close local/local_sockif.c:797
#36 0x4e978f6 in psock_close socket/net_close.c:102
#37 0x4eed1b9 in sock_file_close socket/socket.c:115
#38 0x4ed4f6d in file_close vfs/fs_close.c:78
#39 0x4ed1459 in nx_close_from_tcb inode/fs_files.c:754
#40 0x4ed1501 in nx_close inode/fs_files.c:781
#41 0x4ed154a in close inode/fs_files.c:819
#42 0x6bcb9ce in property_get kvdb/client.c:307
#43 0x6bcd465 in property_get_int32 kvdb/common.c:270
#44 0x5106c9a in tz_offset_restore app/miwear_bluetooth.c:745
#45 0x510893f in miwear_bluetooth_main app/miwear_bluetooth.c:1033
#46 0x4dcf5c8 in nxtask_startup sched/task_startup.c:70
#47 0x4d70873 in nxtask_start task/task_start.c:134
#48 0x4e04a07 in pre_start sim/sim_initialstate.c:52
Signed-off-by: ligd <liguiding1@xiaomi.com>
set CONFIG_PRIORITY_INHERITANCE=y
set CONFIG_SEM_PREALLOCHOLDERS=0
semaphore/sem_holder.c:320:34: runtime error: member access within null pointer of type 'struct tcb_s'
#0 0xd8b540 in nxsem_boostholderprio semaphore/sem_holder.c:320
#1 0xd8c1cf in nxsem_boost_priority semaphore/sem_holder.c:703
#2 0xda5dfa in nxsem_wait semaphore/sem_wait.c:145
#3 0xda61d9 in nxsem_wait_uninterruptible semaphore/sem_wait.c:248
#4 0x12f2477 in media_service_thread0 /home/ligd/platform/dev/apps/examples/hello/hello_main.c:44
#5 0x1204154 in pthread_startup pthread/pthread_create.c:59
#6 0x1cd906f in pthread_start pthread/pthread_create.c:139
#7 0xe72fcb in pre_start sim/sim_initialstate.c:52
Signed-off-by: ligd <liguiding1@xiaomi.com>
If the write lock is already held by oneself and sine the write
lock can be recursively held, so, this operation can be converted to a write
lock to avoid deadlock.
Signed-off-by: dongjiuzhu1 <dongjiuzhu1@xiaomi.com>
