apache
sched/sched_lock: remove null pointer check of rtcb
Summary
Reverts apache/nuttx#17300
sched/sched_lock: remove null pointer check of rtcb
In previous commits to xiaomi, runtime null pointer checks for RTCB were removed.
This could cause sched_lock/unlock exceptions when DEBUG_ASSERTIONS was disabled.
In this commit, we removed all RTCB checks to improve performance by 1 comparison cycle,
References to null pointers in the RTCB will rely on hardware exception or MPU/MMU protection.
| commit d94cb53d6cde6a999d862ae45e1fcd3692675cb4 (HEAD, origin/master, origin/HEAD)
| Author: hujun5 <[email protected]>
| Date: Thu Feb 6 15:06:00 2025 +0800
|
| sched_lock: remove the check for whether tcb is NULL
|
| Remove Redundant Checks
|
| Signed-off-by: hujun5 <[email protected]>
Signed-off-by: chao an [email protected]
Testing
sim/nsh ostest ; sabre-6quad/smp
I submitted a change request in PR https://github.com/apache/nuttx/pull/17300 , but @xiaoxiang781216 was ignored and the commit was forcibly merged. Please revert this change.
REF: https://github.com/apache/nuttx/pull/17300#discussion_r2513526376
Hi @anchao , I support reverting the commit until the review comments are satisfied. But, this revert PR looks like it has modifications and isn't a direct revert. I think we should revert the exact commit changes and then reopen for discussion.
What's the point of fighting for performance in debug code? It's normal for debug code to have no performance. Null pointer debug assertions are useful because they sometimes allow you to find bugs that are hard to find, like stack/buffers overflows. I remember that this type of assertions helped me to find bugs in completely unrelated parts of the kernel.
Also I very much doubt that all architectures support null pointer detection, the first thing that comes to my mind is STM32F0 (cortex-m0). For all STM32 0x0000:0000 is the correct address, which means that by default there will be no exception even for chips with MPU. This can lead to bugs that are difficult to detect and debug.
What's the point of fighting for performance in debug code? It's normal for debug code to have no performance. Null pointer debug assertions are useful because they sometimes allow you to find bugs that are hard to find, like stack/buffers overflows. I remember that this type of assertions helped me to find bugs in completely unrelated parts of the kernel.
Also I very much doubt that all architectures support null pointer detection, the first thing that comes to my mind is STM32F0 (cortex-m0). For all STM32 0x0000:0000 is the correct address, which means that by default there will be no exception even for chips with MPU. This can lead to bugs that are difficult to detect and debug.
I agree. I wonder if someone could explain though why we don't need the runtime check out of debug mode? Why is the rtcb never null?
What's the point of fighting for performance in debug code? It's normal for debug code to have no performance. Null pointer debug assertions are useful because they sometimes allow you to find bugs that are hard to find, like stack/buffers overflows. I remember that this type of assertions helped me to find bugs in completely unrelated parts of the kernel.
I disagree with your point. In fact, most Nuttx devices have DEBUG_ASSERTIONS enabled by default, including those already on the market, because this is very helpful for locating and resolving certain issues.
Therefore, performance is also very important in the enable assertion mode.
Also I very much doubt that all architectures support null pointer detection, the first thing that comes to my mind is STM32F0 (cortex-m0). For all STM32 0x0000:0000 is the correct address, which means that by default there will be no exception even for chips with MPU. This can lead to bugs that are difficult to detect and debug.
@raiden00pl Have you actually seen the changes in #17300 reviewed? This commit removed the check for rtcb validity. If, as you claim, some hardware lacks 0-address protection, then #17300 needs to be reverted.