Re: [PR] Reduce the size of tcb [nuttx]
xiaoxiang781216 commented on PR #15345: URL: https://github.com/apache/nuttx/pull/15345#issuecomment-2907555822 Ok, I understand the situation now: nxsig_deliver is only referenced by nxsig_queue_action before the change, which mean nxsig_deliver will be included only when some code really trigger signal. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Reduce the size of tcb [nuttx]
raiden00pl commented on PR #15345: URL: https://github.com/apache/nuttx/pull/15345#issuecomment-2906871526 @xiaoxiang781216 you are wrong. This change is exactly to blame. `nxsig_deliver()` can't be properly optimized by compiler now, even if the signals are not used by the system. Here is the output from `nm` before this change, showing 4 largest symbols: ``` 0184 T __start 0188 T _vectors 01a8 t group_leave 073c t nx_start ``` and here is the output after this change: ``` 0184 T __start 0188 T _vectors 0578 t nxsig_deliver 07f0 t nx_start ``` nxsig_deliver -> 0x578 = 1400B. The rest of wasted FLASH must come from some auxiliary functions which are also not optimized properly. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Reduce the size of tcb [nuttx]
xiaoxiang781216 commented on PR #15345: URL: https://github.com/apache/nuttx/pull/15345#issuecomment-2906859478 @raiden00pl this patch just change the pointer operation(assignment and test) to the related bit flag operation, I doubt that this change could increase 2KB flash size, so let's profile again to double confirm your result in Monday. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Reduce the size of tcb [nuttx]
acassis commented on PR #15345: URL: https://github.com/apache/nuttx/pull/15345#issuecomment-2904320155 @raiden00pl @lupyuen @xiaoxiang781216 @simbit18 how we could modify the CI to automatically detect subtle increase of Flash and RAM memory? I saw that the CI is installing the "bloaty" tool, but seems like it is not used to detect memory increases. @wagnzhi16 I propose revert this patch. Reducing 4 bytes for TCB at cost of 2KB flash is not a good option. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Reduce the size of tcb [nuttx]
raiden00pl commented on PR #15345: URL: https://github.com/apache/nuttx/pull/15345#issuecomment-2904217908 the difference for a minimalistic NuttX image on cortex-m4 (which basically does nothing) looks like this: 1. before ``` Memory region Used Size Region Size %age Used flash:6980 B64 KB 10.65% sram: 928 B16 KB 5.66% ``` 2. after ``` Memory region Used Size Region Size %age Used flash:9076 B64 KB 13.85% sram: 924 B16 KB 5.64% ``` 2KB of FLASH wasted for small systems. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Reduce the size of tcb [nuttx]
raiden00pl commented on PR #15345: URL: https://github.com/apache/nuttx/pull/15345#issuecomment-2904201084 @xiaoxiang781216 @wagnzhi16 this change has really bad impact on small systems in NuttX. It wastes about 2KB FLASH for a simple "hello world" application: 1. before this change ``` Memory region Used Size Region Size %age Used flash: 25140 B64 KB 38.36% sram:2824 B16 KB 17.24% ``` 2. after this change: ``` Memory region Used Size Region Size %age Used flash: 23392 B64 KB 35.69% sram:2824 B16 KB 17.24% ``` Here another the same "hello world" with more optimization: 1. before this change ``` Memory region Used Size Region Size %age Used flash: 18260 B64 KB 27.86% sram:1236 B16 KB 7.54% ``` 2. after this change ``` Memory region Used Size Region Size %age Used flash: 20056 B64 KB 30.60% sram:1232 B16 KB 7.52% ``` Reducing TCB size by 4B in exchange for almost 2KB FLASH is a very bad compromise! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Reduce the size of tcb [nuttx]
xiaoxiang781216 merged PR #15345: URL: https://github.com/apache/nuttx/pull/15345 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Reduce the size of tcb [nuttx]
wangzhi-art commented on code in PR #15345: URL: https://github.com/apache/nuttx/pull/15345#discussion_r1900886787 ## arch/z80/src/ez80/ez80_sigdeliver.c: ## @@ -102,7 +103,7 @@ void z80_sigdeliver(void) regs[XCPT_PC]= rtcb->xcp.saved_pc; Review Comment: Done. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Reduce the size of tcb [nuttx]
wangzhi-art commented on code in PR #15345:
URL: https://github.com/apache/nuttx/pull/15345#discussion_r1900886589
##
arch/avr/src/avr32/avr_sigdeliver.c:
##
@@ -104,7 +105,7 @@ void avr_sigdeliver(void)
regs[REG_PC] = rtcb->xcp.saved_pc;
regs[REG_SR] = rtcb->xcp.saved_sr;
Review Comment:
Done.
##
arch/ceva/src/common/ceva_sigdeliver.c:
##
@@ -63,17 +63,17 @@ void ceva_sigdeliver(void)
int saved_errno = rtcb->pterrno;
- sinfo("rtcb=%p sigdeliver=%p sigpendactionq.head=%p\n",
-rtcb, rtcb->sigdeliver, rtcb->sigpendactionq.head);
- DEBUGASSERT(rtcb->sigdeliver != NULL);
+ sinfo("rtcb=%p sigpendactionq.head=%p\n",
+rtcb, rtcb->sigpendactionq.head);
+ DEBUGASSERT((rtcb->flags & TCB_FLAG_SIGDELIVER) != 0);
/* Get a local copy of the sigdeliver function pointer. We do this so that
* we can nullify the sigdeliver function pointer in the TCB and accept
* more signal deliveries while processing the current pending signals.
*/
- sigdeliver = rtcb->sigdeliver;
- rtcb->sigdeliver = NULL;
+ sigdeliver = nxsig_deliver;
Review Comment:
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] Reduce the size of tcb [nuttx]
wangzhi-art commented on code in PR #15345: URL: https://github.com/apache/nuttx/pull/15345#discussion_r1900886410 ## arch/z80/src/z180/z180_sigdeliver.c: ## @@ -99,7 +100,7 @@ void z80_sigdeliver(void) regs[XCPT_PC]= rtcb->xcp.saved_pc; regs[XCPT_I] = rtcb->xcp.saved_i; Review Comment: Done. ## arch/avr/src/avr/avr_sigdeliver.c: ## @@ -104,7 +105,7 @@ void avr_sigdeliver(void) regs[REG_PC2]= rtcb->xcp.saved_pc2; #endif regs[REG_SREG] = rtcb->xcp.saved_sreg; Review Comment: Done. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Reduce the size of tcb [nuttx]
wangzhi-art commented on code in PR #15345: URL: https://github.com/apache/nuttx/pull/15345#discussion_r1900886247 ## arch/z80/src/z80/z80_sigdeliver.c: ## @@ -99,7 +100,7 @@ void z80_sigdeliver(void) regs[XCPT_PC]= rtcb->xcp.saved_pc; regs[XCPT_I] = rtcb->xcp.saved_i; Review Comment: Done ## arch/z80/src/z80/z80_sigdeliver.c: ## @@ -99,7 +100,7 @@ void z80_sigdeliver(void) regs[XCPT_PC]= rtcb->xcp.saved_pc; Review Comment: Done. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Reduce the size of tcb [nuttx]
wangzhi-art commented on code in PR #15345: URL: https://github.com/apache/nuttx/pull/15345#discussion_r1900885981 ## arch/misoc/src/minerva/minerva_sigdeliver.c: ## @@ -78,8 +78,8 @@ void minerva_sigdeliver(void) * more signal deliveries while processing the current pending signals. */ - sigdeliver = rtcb->sigdeliver; - rtcb->sigdeliver = NULL; + sigdeliver = nxsig_deliver; Review Comment: Done. ## include/nuttx/sched.h: ## @@ -134,6 +134,7 @@ #define TCB_FLAG_FORCED_CANCEL (1 << 13) /* Bit 13: Pthread cancel is forced */ #define TCB_FLAG_JOIN_COMPLETED(1 << 14) /* Bit 14: Pthread join completed */ #define TCB_FLAG_FREE_TCB (1 << 15) /* Bit 15: Free tcb after exit */ +#define TCB_FLAG_SIGDELIVER(1 << 16) /* Bit 16: Deliver pending signals */ Review Comment: Done. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Reduce the size of tcb [nuttx]
xiaoxiang781216 commented on code in PR #15345: URL: https://github.com/apache/nuttx/pull/15345#discussion_r1900790200 ## include/nuttx/sched.h: ## @@ -134,6 +134,7 @@ #define TCB_FLAG_FORCED_CANCEL (1 << 13) /* Bit 13: Pthread cancel is forced */ #define TCB_FLAG_JOIN_COMPLETED(1 << 14) /* Bit 14: Pthread join completed */ #define TCB_FLAG_FREE_TCB (1 << 15) /* Bit 15: Free tcb after exit */ +#define TCB_FLAG_SIGDELIVER(1 << 16) /* Bit 16: Deliver pending signals */ Review Comment: either is fine to me -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Reduce the size of tcb [nuttx]
wangzhi-art commented on code in PR #15345: URL: https://github.com/apache/nuttx/pull/15345#discussion_r1900499104 ## include/nuttx/sched.h: ## @@ -134,6 +134,7 @@ #define TCB_FLAG_FORCED_CANCEL (1 << 13) /* Bit 13: Pthread cancel is forced */ #define TCB_FLAG_JOIN_COMPLETED(1 << 14) /* Bit 14: Pthread join completed */ #define TCB_FLAG_FREE_TCB (1 << 15) /* Bit 15: Free tcb after exit */ +#define TCB_FLAG_SIGDELIVER(1 << 16) /* Bit 16: Deliver pending signals */ Review Comment: Currently, bit 2 is not used. I don't know if there are other reasons why bit 2 was not used before. If there is no other impact, wouldn't it be better to arrange it more compactly, such as letting TCB_FLAG_POLICY use bits 2-3, TCB_FLAG_CPU_LOCKED use bit 4, and so on? I wonder if this is okay? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Reduce the size of tcb [nuttx]
hujun260 commented on code in PR #15345: URL: https://github.com/apache/nuttx/pull/15345#discussion_r1900502395 ## include/nuttx/sched.h: ## @@ -134,6 +134,7 @@ #define TCB_FLAG_FORCED_CANCEL (1 << 13) /* Bit 13: Pthread cancel is forced */ #define TCB_FLAG_JOIN_COMPLETED(1 << 14) /* Bit 14: Pthread join completed */ #define TCB_FLAG_FREE_TCB (1 << 15) /* Bit 15: Free tcb after exit */ +#define TCB_FLAG_SIGDELIVER(1 << 16) /* Bit 16: Deliver pending signals */ Review Comment: Can the "scheduling policy" change to the placeholder to 2-3 bits? And then arrange the rest in order. I feel it's easier to understand that way. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Reduce the size of tcb [nuttx]
xiaoxiang781216 commented on code in PR #15345:
URL: https://github.com/apache/nuttx/pull/15345#discussion_r1900157910
##
arch/z80/src/z80/z80_sigdeliver.c:
##
@@ -99,7 +100,7 @@ void z80_sigdeliver(void)
regs[XCPT_PC]= rtcb->xcp.saved_pc;
regs[XCPT_I] = rtcb->xcp.saved_i;
Review Comment:
```suggestion
regs[XCPT_I] = rtcb->xcp.saved_i;
```
##
include/nuttx/sched.h:
##
@@ -134,6 +134,7 @@
#define TCB_FLAG_FORCED_CANCEL (1 << 13) /* Bit 13:
Pthread cancel is forced */
#define TCB_FLAG_JOIN_COMPLETED(1 << 14) /* Bit 14:
Pthread join completed */
#define TCB_FLAG_FREE_TCB (1 << 15) /* Bit 15:
Free tcb after exit */
+#define TCB_FLAG_SIGDELIVER(1 << 16) /* Bit 16:
Deliver pending signals */
Review Comment:
```suggestion
#define TCB_FLAG_SIGDELIVER(1 << 2) /* Bit 2:
Deliver pending signals */
```
move before line 120
##
arch/z80/src/ez80/ez80_sigdeliver.c:
##
@@ -102,7 +103,7 @@ void z80_sigdeliver(void)
regs[XCPT_PC]= rtcb->xcp.saved_pc;
Review Comment:
```suggestion
regs[XCPT_PC] = rtcb->xcp.saved_pc;
```
##
arch/avr/src/avr32/avr_sigdeliver.c:
##
@@ -104,7 +105,7 @@ void avr_sigdeliver(void)
regs[REG_PC] = rtcb->xcp.saved_pc;
regs[REG_SR] = rtcb->xcp.saved_sr;
Review Comment:
```suggestion
regs[REG_SR] = rtcb->xcp.saved_sr;
```
##
arch/z80/src/z80/z80_sigdeliver.c:
##
@@ -99,7 +100,7 @@ void z80_sigdeliver(void)
regs[XCPT_PC]= rtcb->xcp.saved_pc;
Review Comment:
ditto
##
arch/misoc/src/minerva/minerva_sigdeliver.c:
##
@@ -78,8 +78,8 @@ void minerva_sigdeliver(void)
* more signal deliveries while processing the current pending signals.
*/
- sigdeliver = rtcb->sigdeliver;
- rtcb->sigdeliver = NULL;
+ sigdeliver = nxsig_deliver;
Review Comment:
remove sigdeliver, call nxsig_deliver directly
##
arch/z80/src/z180/z180_sigdeliver.c:
##
@@ -99,7 +100,7 @@ void z80_sigdeliver(void)
regs[XCPT_PC]= rtcb->xcp.saved_pc;
regs[XCPT_I] = rtcb->xcp.saved_i;
Review Comment:
```suggestion
regs[XCPT_I] = rtcb->xcp.saved_i;
```
##
arch/avr/src/avr/avr_sigdeliver.c:
##
@@ -104,7 +105,7 @@ void avr_sigdeliver(void)
regs[REG_PC2]= rtcb->xcp.saved_pc2;
#endif
regs[REG_SREG] = rtcb->xcp.saved_sreg;
Review Comment:
```suggestion
regs[REG_SREG] = rtcb->xcp.saved_sreg;
```
and other lines
##
arch/ceva/src/common/ceva_sigdeliver.c:
##
@@ -63,17 +63,17 @@ void ceva_sigdeliver(void)
int saved_errno = rtcb->pterrno;
- sinfo("rtcb=%p sigdeliver=%p sigpendactionq.head=%p\n",
-rtcb, rtcb->sigdeliver, rtcb->sigpendactionq.head);
- DEBUGASSERT(rtcb->sigdeliver != NULL);
+ sinfo("rtcb=%p sigpendactionq.head=%p\n",
+rtcb, rtcb->sigpendactionq.head);
+ DEBUGASSERT((rtcb->flags & TCB_FLAG_SIGDELIVER) != 0);
/* Get a local copy of the sigdeliver function pointer. We do this so that
* we can nullify the sigdeliver function pointer in the TCB and accept
* more signal deliveries while processing the current pending signals.
*/
- sigdeliver = rtcb->sigdeliver;
- rtcb->sigdeliver = NULL;
+ sigdeliver = nxsig_deliver;
Review Comment:
remove sigdeliver
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] Reduce the size of tcb. [nuttx]
xiaoxiang781216 commented on PR #15391: URL: https://github.com/apache/nuttx/pull/15391#issuecomment-2566397416 why close this pr? @wangzhi-art -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Reduce the size of tcb [nuttx]
wangzhi-art closed pull request #15345: Reduce the size of tcb URL: https://github.com/apache/nuttx/pull/15345 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Reduce the size of tcb. [nuttx]
wangzhi-art closed pull request #15391: Reduce the size of tcb. URL: https://github.com/apache/nuttx/pull/15391 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Reduce the size of tcb [nuttx]
wangzhi-art closed pull request #15345: Reduce the size of tcb URL: https://github.com/apache/nuttx/pull/15345 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Reduce the size of tcb. [nuttx]
nuttxpr commented on PR #15391: URL: https://github.com/apache/nuttx/pull/15391#issuecomment-2566068294 [**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) This PR description, while providing some information, needs significant improvement to meet the NuttX requirements fully. Here's a breakdown and suggestions: **Weaknesses:** * **Summary is insufficient:** While it explains the *why*, it lacks crucial details about *what* was changed and *how*. It mentions `tcb->sigdeliver` and a new flag `TCB_FLAG_SIGDELIVER`, but doesn't specify the code modifications. Imagine someone reviewing this without seeing the code diff; they wouldn't understand the change. * **"Impact: None" is too brief:** Even seemingly small changes can have impacts. This needs more thorough consideration. At minimum, justify *why* there's no impact. Was the removed code truly unused? * **Testing is inadequate:** Saying "ostest is executed without failure" isn't enough. Provide specific commands used, expected output, and actual output. "smp environment of armv7a and armv8a" is also vague; specify the boards and configurations used. Crucially, the "Testing logs before change" and "Testing logs after change" sections are empty. * **No Issue references:** Even if there isn't a directly related issue, consider whether this addresses a broader problem that might have been discussed previously. * **Drawback section misplaced and insufficient:** This belongs in the "Impact" section. Also, "There are no drawback so far" is weak. More robust testing and reasoning are needed to confidently assert this. **Suggested Improvements:** **Summary:** * **What functional part is changed?** Specifically name the files and functions modified. (e.g., "The signal delivery mechanism in `sched/signal/sig_deliver.c` and the TCB structure in `sched/include/nuttx/sched.h` are modified.") * **How does the change exactly work?** Provide a concise, technical description. (e.g., "The `tcb->sigdeliver` pointer, previously used to indicate pending signal delivery, is removed. A new flag, `TCB_FLAG_SIGDELIVER`, in the `tcb->flags` field now serves this purpose. Calls to assign and check `tcb->sigdeliver` are replaced with setting/checking this flag. This reduces memory usage and simplifies the logic.") **Impact:** * **Impact on user:** Even if there's no direct impact, state it explicitly and justify it. (e.g., "NO. This is an internal optimization and does not affect user-facing APIs or behavior.") * **Impact on build:** Likely NO, but confirm. * **Impact on hardware:** Specify the architectures and boards tested. Justify why other architectures are *not* affected, if applicable. * **Impact on documentation:** If no documentation changes are needed, state it explicitly. (e.g., "NO. This change is internal and does not require documentation updates.") * **Impact on security:** Analyze potential security implications. Even if none are apparent, explicitly state your reasoning. (e.g., "NO. This change simplifies the signal delivery logic, potentially reducing the attack surface, but introduces no new vulnerabilities.") * **Impact on compatibility:** Address backward compatibility. Does this change affect applications compiled against older NuttX versions? * **Anything else to consider?** Mention any performance improvements observed. **Testing:** * **Build Host(s):** Provide full details (e.g., "Linux Ubuntu 20.04, x86_64, GCC 9.4.0") * **Target(s):** Be specific (e.g., "ARMv7-A: STM32F429I-DISC1, `nsh` configuration; ARMv8-A: QEMU virt, `nsh` configuration") * **Testing logs:** Include relevant logs demonstrating the behavior before and after the change. Focus on the signal delivery functionality. Show how the new flag is being used. Even simple logs demonstrating successful signal handling are better than nothing. Use commands like `dmesg` or application-specific logging. If using `ostest`, provide specific test case outputs. By addressing these points, you'll create a much stronger PR that's easier to review and more likely to be accepted. Remember to also follow the NuttX Contributing Guidelines closely. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Reduce the size of tcb [nuttx]
wangzhi-art closed pull request #15345: Reduce the size of tcb URL: https://github.com/apache/nuttx/pull/15345 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Reduce the size of tcb [nuttx]
wangzhi-art commented on PR #15345: URL: https://github.com/apache/nuttx/pull/15345#issuecomment-2565066102 > @wangzhi-art please include more details in the git commit log message to explain why the old code is not needed anymore. Is there some drawback after removing these lines of code? Done -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Reduce the size of tcb [nuttx]
wangzhi-art commented on code in PR #15345:
URL: https://github.com/apache/nuttx/pull/15345#discussion_r1899254062
##
arch/arm/src/arm/arm_sigdeliver.c:
##
@@ -60,8 +60,8 @@ void arm_sigdeliver(void)
board_autoled_on(LED_SIGNAL);
sinfo("rtcb=%p sigdeliver=%p sigpendactionq.head=%p\n",
-rtcb, rtcb->sigdeliver, rtcb->sigpendactionq.head);
- DEBUGASSERT(rtcb->sigdeliver != NULL);
+rtcb, nxsig_deliver, rtcb->sigpendactionq.head);
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] Reduce the size of tcb [nuttx]
wangzhi-art commented on PR #15345: URL: https://github.com/apache/nuttx/pull/15345#issuecomment-2564987955 > @wangzhi-art please include more details in the git commit log message to explain why the old code is not needed anymore. Is there some drawback after removing these lines of code? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Reduce the size of tcb [nuttx]
wangzhi-art closed pull request #15345: Reduce the size of tcb URL: https://github.com/apache/nuttx/pull/15345 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Reduce the size of tcb [nuttx]
hujun260 commented on PR #15345: URL: https://github.com/apache/nuttx/pull/15345#issuecomment-2564278983 fix compile error -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Reduce the size of tcb [nuttx]
hujun260 commented on code in PR #15345:
URL: https://github.com/apache/nuttx/pull/15345#discussion_r1898879690
##
arch/arm/src/arm/arm_sigdeliver.c:
##
@@ -60,8 +60,8 @@ void arm_sigdeliver(void)
board_autoled_on(LED_SIGNAL);
sinfo("rtcb=%p sigdeliver=%p sigpendactionq.head=%p\n",
-rtcb, rtcb->sigdeliver, rtcb->sigpendactionq.head);
- DEBUGASSERT(rtcb->sigdeliver != NULL);
+rtcb, nxsig_deliver, rtcb->sigpendactionq.head);
Review Comment:
remove print nxsig_deliver
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] Reduce the size of tcb [nuttx]
hujun260 commented on code in PR #15345:
URL: https://github.com/apache/nuttx/pull/15345#discussion_r1898877417
##
arch/z16/src/common/z16_sigdeliver.c:
##
@@ -62,8 +62,8 @@ void z16_sigdeliver(void)
board_autoled_on(LED_SIGNAL);
sinfo("rtcb=%p sigdeliver=%p sigpendactionq.head=%p\n",
-rtcb, rtcb->sigdeliver, rtcb->sigpendactionq.head);
- DEBUGASSERT(rtcb->sigdeliver != NULL);
+rtcb, nxsig_deliver, rtcb->sigpendactionq.head);
Review Comment:
remove
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] Reduce the size of tcb [nuttx]
hujun260 commented on code in PR #15345:
URL: https://github.com/apache/nuttx/pull/15345#discussion_r1898877417
##
arch/z16/src/common/z16_sigdeliver.c:
##
@@ -62,8 +62,8 @@ void z16_sigdeliver(void)
board_autoled_on(LED_SIGNAL);
sinfo("rtcb=%p sigdeliver=%p sigpendactionq.head=%p\n",
-rtcb, rtcb->sigdeliver, rtcb->sigpendactionq.head);
- DEBUGASSERT(rtcb->sigdeliver != NULL);
+rtcb, nxsig_deliver, rtcb->sigpendactionq.head);
Review Comment:
remove
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] Reduce the size of tcb [nuttx]
nuttxpr commented on PR #15345: URL: https://github.com/apache/nuttx/pull/15345#issuecomment-2562134672 [**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) No, this PR description does not fully meet the NuttX requirements. While it provides a summary and testing information, it lacks crucial details required for proper review. Here's a breakdown of what's missing and how to improve it: **Missing Information & Suggested Improvements:** * **Summary:** * **Why is this change necessary?** The summary states *what* the change does, but not *why*. Is `nxsig_deliver` causing a performance bottleneck? Is it a maintainability issue? Explain the motivation for this change. * **What functional part of the code is being changed?** Be more specific than "tcb". Which files are modified? Mentioning specific modules or subsystems within NuttX would be helpful. * **How *exactly* does the change work?** The summary is a bit vague. Describe the mechanism of using a flag bit. What flag bit is used? How does the logic change for signal delivery with this new flag? * **Related Issues:** Are there any related NuttX or NuttX Apps issues? If not, explicitly state "None." * **Impact:** While stating "Code modification is imperceptible to users" is a good start, you need to specifically address *all* the impact points with either "NO" or "YES (and explanation)": * **New/Changed Feature:** NO * **Impact on User:** NO * **Impact on Build:** Likely NO, but confirm. If there are any changes to Kconfig options or build dependencies, explain them. * **Impact on Hardware:** Specify the architectures tested (armv7a, armv8a). Were any specific boards used? State YES and list them if applicable. Otherwise, if it's architecture-generic, explain why. * **Impact on Documentation:** NO (unless documentation needs to be updated to reflect the internal code changes, in which case say YES and provide details). * **Impact on Security:** Carefully consider this! Does changing the signal handling mechanism introduce any potential security vulnerabilities? Justify your answer (NO/YES). * **Impact on Compatibility:** NO (presumably, but explain why you think so). Does this change affect any existing drivers or applications? * **Anything else to consider:** Mention any potential risks or edge cases that reviewers should be aware of. * **Testing:** * **Build Host(s):** Provide details about your build host environment: OS, CPU architecture, compiler version (e.g., "Linux x86_64, GCC 12.2.0"). * **Target(s):** Be more specific: "sim:qemu-system-arm", "board:stm32f4discovery:default", etc. * **Testing Logs:** The provided logs are placeholders. Include *actual* logs or output demonstrating the test results before and after the change. If the output is too long, consider attaching a file or linking to a CI run. Show that the change works as expected and doesn't introduce regressions. What specific aspects of `ostest` were tested? By addressing these points, you'll create a much more comprehensive and reviewable PR, increasing its chances of being accepted. Remember to follow the contributing guidelines carefully. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
