sched/signal: reclaim sigaction blocks

[yf13]

Summary

Currently the sigaction objects are never reclaimed after use, thus after running ostest we will see used memory increasion.

This patch uses a configurable number of pre-allocated entries and reclaims dynamic entries so that to reduce leakage.

Impact

Used memory reduced after running ostest.

Testing

• Checked with rv-virt/nsh and rv-virt/nsh64
• CI checks

[yf13]

what's benefit to reclaim the signal action pool? if it's important to safe the memory, it's simpler and fast to alloc/free one action from heap.

@xiaoxiang781216 thanks for the comment. I watched 120*16=1920 bytes memory growth from ostest on rv-virt/nsh. This concerned me in the past and may scare new comers in the future. When reviewing the source, I saw comments in ostest like /* Just exit, the system should clean up the signal handlers */ in ostest and the system doesn't do reclamation, so I added it.

On the other side, can I learn the benefit of this pool over the direct alloc/free of single entries approach? how the pool size is controlled? Why this pool doesn't use flags sigalloc_e like other ones?

[xiaoxiang781216]

what's benefit to reclaim the signal action pool? if it's important to safe the memory, it's simpler and fast to alloc/free one action from heap.

@xiaoxiang781216 thanks for the comment. I watched 120*16=1920 bytes memory growth from ostest on rv-virt/nsh. This concerned me in the past and may scare new comers in the future. When reviewing the source, I saw comments in ostest like /* Just exit, the system should clean up the signal handlers */ in ostest and the system doesn't do reclamation, so I added it.

But the change makes nxsig_release_action is slower than kmm_free. If so, why not remove the pool and call kmm_malloc/kmm_free direcctly?

On the other side, can I learn the benefit of this pool over the direct alloc/free of single entries approach? how the pool size is controlled? Why this pool doesn't use flags sigalloc_e like other ones?

If you want to save the memory, please study how icmp manage the socket conn: https://github.com/apache/nuttx/blob/master/net/icmp/icmp_conn.c#L77-L204

[yf13]

But the change makes nxsig_release_action is slower than kmm_free. If so, why not remove the pool and call kmm_malloc/kmm_free direcctly? ... please study how icmp manage the socket conn:

@xiaoxiang781216 thanks for the pointers. I've updated it with both SIG_PREALLOC_ACTIONS and dynamic entries from kmm_malloc. Please take a look when free.

[yf13]

@xiaoxiang781216, sorry for my late response as I didn't see inline comments from gh command line.

would you teach a use case where static-plus-dynamic approach is improper but never return approach does a better job? I don't quite understand why we need never return designin upstream as it gives user apps chances to exhaust the system. If a system wants to avoid frequent small sized alloc/free operations from heap, pre-allocation shall help.

[xiaoxiang781216]

@xiaoxiang781216, sorry for my late response as I didn't see inline comments from gh command line.

would you teach a use case where static-plus-dynamic approach is improper but never return approach does a better job?

If the system hit a peak value, it may likely hit again in the future. So, it may fine to not return the memory to heap.

I don't quite understand why we need never return designin upstream as it gives user apps chances to exhaust the system. If a system wants to avoid frequent small sized alloc/free operations from heap, pre-allocation shall help.

but you mayn't know how much memory need be preserved in all case.

[yf13]

@acassis, it seems this job log complains about the stm3240g-eval/knxwm config:

arm-none-eabi-ld: /github/workspace/sources/nuttx/nuttx section `.bss' will not fit in region `ksram'
arm-none-eabi-ld: region `ksram' overflowed by 72 bytes

I am wondering if we have chances to enlarge the "ksram" region a little? Previously this patch builds well but after rebasing the bss usage just reaches the limit. The current layout reads like below in memory.ld:

  /* 112Kb of contiguous SRAM */

  ksram (rwx)      : ORIGIN = 0x20000000, LENGTH = 6K
  usram (rwx)      : ORIGIN = 0x20001800, LENGTH = 4K
  xsram (rwx)      : ORIGIN = 0x20002800, LENGTH = 102K

As I am unfamiliar with that chip, so instead I reduce the preallocated number of sigactions so that you can take time to review the linker script.

[yf13]

@xiaoxiang781216 thanks for the comments. when g_sigfreeactoin is merged to the private g_sigpool, then how to judge preallocated item from sig_action.c? here are a few options I see:

• still keep sigpool_s and g_sigpool private and add a function is_prealloc_action() in signal.h and sig_initialize.c to help sig_action.c .
• move sigpool_s to signal.h also make g_sigpool non-private, also keep IS_PREALLOC_ACTION in signal.h so that it is close to the struct sigpool_s and extern g_sigpool.
• just move sigpool_s to signal.h, then share g_sigpool between sig_initialize.c and sig_action.c, and put IS_PREALLOC_ACTION in sig_action.c.

shall we go with the last option?

[yf13]

@xiaoxiang781216, an update has been pushed, it moves g_sigactions[] and relevant macro and initialization to sig_action.c and makes them private. thus the sharing issue is gone.