apache
Support disable signal
Summary
Added support for signal decoupling. In scenarios with strict requirements for runtime memory and code size, this feature can be enabled by setting CONFIG_DISABLE_SIGNALS=y, which will disable the core functionality of signals.
It should be noted that CONFIG_DISABLE_SIGNALS=y does not completely disable all signal functionalities, as some operations such as kill and sleep remain essential in certain contexts. Therefore, only the core queue component of signals is disabled, while basic operations like sleep and kill continue to be supported.
Impact
This feature does not modify the functional logic of existing capabilities. It only decouples the signal functionality.
When CONFIG_DISABLE_SIGNALS=y, it can reduce runtime memory usage and decrease code size.
Testing
- has passed ostest on CONFIG_DISABLE_SIGNALS=y
- Here are the test results from our ARMv7-A platform:
When CONFIG_DISABLE_SIGNALS=n:
Binary size = 1,295,424 bytes, Used RAM = 37,980 bytes
When CONFIG_DISABLE_SIGNALS=y:
Binary size = 1,262,624 bytes, Used RAM = 37,852 bytes
This shows a reduction of 32,800 bytes in binary size and 128 bytes in RAM usage.
- Please refer to #17352 as the primary commit; @wangchdo submitted this change first.
- cc NuttX PMC and commiterrs, Please do not merge this PR, especially @xiaoxiang781216
The previous submission at https://github.com/apache/nuttx/pull/17352 had an overly broad scope when disabling signals, as it also disabled functionalities like slee, kill etc. The current patchset narrows this scope, ensuring that APIs such as sleep and kill remain available while only certain signal functionalities are disabled.
Alternatively, we could introduce a concept like "disable levels" to differentiate the extent of signal functionality being disabled.
- Please refer to sched/signal: Add support to disable signals #17352 as the primary commit; @wangchdo submitted this change first.
- cc NuttX PMC and commiterrs, Please do not merge this PR, especially @xiaoxiang781216
The previous submission at #17352 had an overly broad scope when disabling signals, as it also disabled functionalities like slee, kill etc. The current patchset narrows this scope, ensuring that APIs such as sleep and kill remain available while only certain signal functionalities are disabled.
Alternatively, we could introduce a concept like "disable levels" to differentiate the extent of signal functionality being disabled.
For sleep()/usleep(), compatibility with the CONFIG_DISABLE_SIGNALS scenario should be considered in the implementation. For other APIs that have mandatory dependencies on signals (such as kill()/pkill()), scenario-based optimization can be performed—even the interfaces can be retained with error code returns supported.
- Please refer to sched/signal: Add support to disable signals #17352 as the primary commit; @wangchdo submitted this change first.
- cc NuttX PMC and commiterrs, Please do not merge this PR, especially @xiaoxiang781216
The previous submission at #17352 had an overly broad scope when disabling signals, as it also disabled functionalities like slee, kill etc. The current patchset narrows this scope, ensuring that APIs such as sleep and kill remain available while only certain signal functionalities are disabled. Alternatively, we could introduce a concept like "disable levels" to differentiate the extent of signal functionality being disabled.
For sleep()/usleep(), compatibility with the CONFIG_DISABLE_SIGNALS scenario should be considered in the implementation. For other APIs that have mandatory dependencies on signals (such as kill()/pkill()), scenario-based optimization can be performed—even the interfaces can be retained with error code returns supported.
Basically, signal has two reaction:
- Call the signal handler function in the target process/task
- Wake up the targe thread just like sem post
The major memory/code size contribution come from the first item, the second item consume the small code/memory, but could keep many frequent used function(e.g. sleep/usleep/wait etc.). So, the best approach is disable signal by level to make it more useful in the different case.
- Please refer to #17352 as the primary commit; @wangchdo submitted this change first.
- cc NuttX PMC and commiterrs, Please do not merge this PR, especially @xiaoxiang781216
The detailed implementation is different, both approach could benefit in the different usage. It's better to introduce the disable level concept, so both pr could be merged.
- Please refer to sched/signal: Add support to disable signals #17352 as the primary commit; @wangchdo submitted this change first.
- cc NuttX PMC and commiterrs, Please do not merge this PR, especially @xiaoxiang781216
The previous submission at #17352 had an overly broad scope when disabling signals, as it also disabled functionalities like slee, kill etc. The current patchset narrows this scope, ensuring that APIs such as sleep and kill remain available while only certain signal functionalities are disabled. Alternatively, we could introduce a concept like "disable levels" to differentiate the extent of signal functionality being disabled.
For sleep()/usleep(), compatibility with the CONFIG_DISABLE_SIGNALS scenario should be considered in the implementation. For other APIs that have mandatory dependencies on signals (such as kill()/pkill()), scenario-based optimization can be performed—even the interfaces can be retained with error code returns supported.
Basically, signal has two reaction:
- Call the signal handler function in the target process/task
- Wake up the targe thread just like sem post
The major memory/code size contribution come from the first item, the second item consume the small code/memory, but could keep many frequent used function(e.g. sleep/usleep/wait etc.). So, the best approach is disable signal by level to make it more useful in the different case.
- Please refer to sched/signal: Add support to disable signals #17352 as the primary commit; @wangchdo submitted this change first.
- cc NuttX PMC and commiterrs, Please do not merge this PR, especially @xiaoxiang781216
The detailed implementation is different, both approach could benefit in the different usage. It's better to introduce the disable level concept, so both pr could be merged.
Why do we need two PRs for this? I already have a PR that disables signals. If the level-control approach works, it would be straightforward to integrate it into #17352. Opening a new PR would only consume additional reviewer time. I think we should focus on improving the existing PR instead.
The detailed implementation is different, both approach could benefit in the different usage. It's better to introduce the disable level concept, so both pr could be merged.
Why do we need two PRs for this? I already have a PR that disables signals. If the level-control approach works, it would be straightforward to integrate it into #17352. Opening a new PR would only consume additional reviewer time. I think we should focus on improving the existing PR instead.
It's fine to use one pr to do the work, but someone need help to merge them. For example, for example, this pr contain more clean than https://github.com/apache/nuttx/pull/17352: https://github.com/apache/nuttx/pull/17357/files#diff-9e6b14f89a1f89e46914da0086d8e74e4d511ce0907333ca592dfda73368207fR132-R138
Bascially, we can arrange the pr into three part:
- Skip the signal dispatch in each arch. Both pr do the same thing, but we need merge the change to ensure the skip completeness.
- One patch skip the signal dispatch in the common code
- Final patch skip all signal related function
Who volunteers to complete this work? @wangchdo or @extinguish.
The detailed implementation is different, both approach could benefit in the different usage. It's better to introduce the disable level concept, so both pr could be merged.
Why do we need two PRs for this? I already have a PR that disables signals. If the level-control approach works, it would be straightforward to integrate it into #17352. Opening a new PR would only consume additional reviewer time. I think we should focus on improving the existing PR instead.
It's fine to use one pr to do the work, but someone need help to merge them. For example, for example, this pr contain more clean than #17352: https://github.com/apache/nuttx/pull/17357/files#diff-9e6b14f89a1f89e46914da0086d8e74e4d511ce0907333ca592dfda73368207fR132-R138
Bascially, we can arrange the pr into three part:
- Skip the signal dispatch in each arch. Both pr do the same thing, but we need merge the change to ensure the skip completeness.
- One patch skip the signal dispatch in the common code
- Final patch skip all signal related function
Who volunteers to complete this work? @wangchdo or @extinguish.
I don't think this PR is better because if signal support is disabled, then the related APIs should also be disabled. eg. the implementation of sleep/usleep needs to be redirected to the sched_sleep() version, and doesn't need to support signal handling.
The detailed implementation is different, both approach could benefit in the different usage. It's better to introduce the disable level concept, so both pr could be merged.
Why do we need two PRs for this? I already have a PR that disables signals. If the level-control approach works, it would be straightforward to integrate it into #17352. Opening a new PR would only consume additional reviewer time. I think we should focus on improving the existing PR instead.
It's fine to use one pr to do the work, but someone need help to merge them. For example, for example, this pr contain more clean than #17352: https://github.com/apache/nuttx/pull/17357/files#diff-9e6b14f89a1f89e46914da0086d8e74e4d511ce0907333ca592dfda73368207fR132-R138
Bascially, we can arrange the pr into three part:
- Skip the signal dispatch in each arch. Both pr do the same thing, but we need merge the change to ensure the skip completeness.
- One patch skip the signal dispatch in the common code
- Final patch skip all signal related function
Who volunteers to complete this work? @wangchdo or @extinguish.
I can take care of all the works related to disabling signals , and I'm happy to do so.
By the way, since this is still under VOTING on the mailing list, I think this work is not very urgent.
I can take care of this, and I'm happy to do so.
By the way, since this is still under VOTING on the mailing list, I think this work is not very urgent.
I think DISABLE_SIGNAL is unlikely to be affected by voting. Voting only affects whether to introduce PES-related definitions to limit kernel capabilities. Compared to DISABLE_PTHREAD/TIMER, signal is designed to achieve a smaller memory footprint.
The detailed implementation is different, both approach could benefit in the different usage. It's better to introduce the disable level concept, so both pr could be merged.
Why do we need two PRs for this? I already have a PR that disables signals. If the level-control approach works, it would be straightforward to integrate it into #17352. Opening a new PR would only consume additional reviewer time. I think we should focus on improving the existing PR instead.
It's fine to use one pr to do the work, but someone need help to merge them. For example, for example, this pr contain more clean than #17352: https://github.com/apache/nuttx/pull/17357/files#diff-9e6b14f89a1f89e46914da0086d8e74e4d511ce0907333ca592dfda73368207fR132-R138 Bascially, we can arrange the pr into three part:
- Skip the signal dispatch in each arch. Both pr do the same thing, but we need merge the change to ensure the skip completeness.
- One patch skip the signal dispatch in the common code
- Final patch skip all signal related function
Who volunteers to complete this work? @wangchdo or @extinguish.
I don't think this PR is better because if signal support is disabled, then the related APIs should also be disabled.
wherer do I say this patch is better? I just claim that both approach can be used in the different case. I just said that some dead fields forget to remove from https://github.com/apache/nuttx/pull/17352, need merge the change in this pr to make it complete.
eg. the implementation of sleep/usleep needs to be redirected to the sched_sleep() version, and doesn't need to support signal handling.
Could you review the patch carefully before making the comment? In this patch, you can still interrupt the target thread from sleep, but you can't trigger the signal handle.
Could you review the patch carefully before making the comment? In this patch, you can still interrupt the target thread from sleep, but you can't trigger the signal handle.
I just think this PR isn't perfect enough, because the sleep process still uses the implementation of nxsig_clockwait()
Could you review the patch carefully before making the comment? In this patch, you can still interrupt the target thread from sleep, but you can't trigger the signal handle.
I just think this PR isn't perfect enough, because the sleep process still uses the implementation of nxsig_clockwait()
This patch try to keep the signal wake up functionality, from this context, the change is consistent and perfect.
The detailed implementation is different, both approach could benefit in the different usage. It's better to introduce the disable level concept, so both pr could be merged.
Why do we need two PRs for this? I already have a PR that disables signals. If the level-control approach works, it would be straightforward to integrate it into #17352. Opening a new PR would only consume additional reviewer time. I think we should focus on improving the existing PR instead.
It's fine to use one pr to do the work, but someone need help to merge them. For example, for example, this pr contain more clean than #17352: https://github.com/apache/nuttx/pull/17357/files#diff-9e6b14f89a1f89e46914da0086d8e74e4d511ce0907333ca592dfda73368207fR132-R138
Bascially, we can arrange the pr into three part:
- Skip the signal dispatch in each arch. Both pr do the same thing, but we need merge the change to ensure the skip completeness.
- One patch skip the signal dispatch in the common code
- Final patch skip all signal related function
Who volunteers to complete this work? @wangchdo or @extinguish.
Well, the initial split is now complete:
- The arch-related parts have been merged into a unified patch
- The signal handling portion is now a separate patch(may need further review to make modification)
- The signal-related function dependencies, currently mainly needed by projects under apps, have been placed in another pull request: https://github.com/apache/nuttx-apps/pull/3217
The "One patch skip the signal dispatch in the common code" part may need clarification regarding the disable level.
I can take care of this, and I'm happy to do so. By the way, since this is still under VOTING on the mailing list, I think this work is not very urgent.
I think
DISABLE_SIGNALis unlikely to be affected by voting. Voting only affects whether to introduce PES-related definitions to limit kernel capabilities. Compared toDISABLE_PTHREAD/TIMER,signalis designed to achieve a smaller memory footprint
The detailed implementation is different, both approach could benefit in the different usage. It's better to introduce the disable level concept, so both pr could be merged.
Why do we need two PRs for this? I already have a PR that disables signals. If the level-control approach works, it would be straightforward to integrate it into #17352. Opening a new PR would only consume additional reviewer time. I think we should focus on improving the existing PR instead.
It's fine to use one pr to do the work, but someone need help to merge them. For example, for example, this pr contain more clean than #17352: https://github.com/apache/nuttx/pull/17357/files#diff-9e6b14f89a1f89e46914da0086d8e74e4d511ce0907333ca592dfda73368207fR132-R138
Bascially, we can arrange the pr into three part:
- Skip the signal dispatch in each arch. Both pr do the same thing, but we need merge the change to ensure the skip completeness.
- One patch skip the signal dispatch in the common code
- Final patch skip all signal related function
Who volunteers to complete this work? @wangchdo or @extinguish.
Upon reconsideration, I prefer to only allow disabling all signal-related functionality, and re-implement the signal-dependent functions such as sleep()/usleep() using the newly added scheduler-based sleep APIs.
-
This approach is clearer and safer. If we allow disabling only part of the signal subsystem, we would need to modify the implementations of the remaining signal functions. At the same time, we cannot guarantee that those functions will continue to behave exactly as before. Even worse, it becomes harder for users to understand the actual impact of partially disabling signal features.
-
With PR https://github.com/apache/nuttx/pull/17200 introducing scheduler-based sleep support, and PR https://github.com/apache/nuttx/pull/17204 replacing all signal-based sleep implementations in drivers and the filesystem with scheduler-based versions, the overall dependency on signals has already been significantly reduced.
-
We can re-implement the libc sleep()/usleep() functions using the new API added by PR https://github.com/apache/nuttx/pull/17368. This will even be more lightweight compared to the current signal-based wait mechanism.