You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "anchao (via GitHub)" <gi...@apache.org> on 2025/03/16 06:31:36 UTC
[PR] [Draft]libs/instrument: add instrument level check to avoid nesting loops [nuttx]
anchao opened a new pull request, #15997:
URL: https://github.com/apache/nuttx/pull/15997
## Summary
libs/instrument: add instrument level check to avoid nesting loops
Signed-off-by: chao an <an...@bytedance.com>
## Impact
N/A
## Testing
sim/nsh, enable vfs instrument functions
```
diff --git a/fs/vfs/Make.defs b/fs/vfs/Make.defs
index 30292d787f..e00b361407 100644
--- a/fs/vfs/Make.defs
+++ b/fs/vfs/Make.defs
@@ -22,6 +22,8 @@
# Common file/socket descriptor support
+CFLAGS += -finstrument-functions
+
CSRCS += fs_chstat.c fs_close.c fs_dup.c fs_dup2.c fs_fcntl.c fs_epoll.c
CSRCS += fs_fchstat.c fs_fstat.c fs_fstatfs.c fs_ioctl.c fs_lseek.c
CSRCS += fs_mkdir.c fs_open.c fs_poll.c fs_pread.c fs_pwrite.c fs_read.c
```
```
$ git diff .
diff --git a/boards/sim/sim/sim/configs/nsh/defconfig b/boards/sim/sim/sim/configs/nsh/defconfig
index 8517209444..496336d268 100644
--- a/boards/sim/sim/sim/configs/nsh/defconfig
+++ b/boards/sim/sim/sim/configs/nsh/defconfig
@@ -24,6 +24,7 @@ CONFIG_DEBUG_FEATURES=y
CONFIG_DEBUG_SYMBOLS=y
CONFIG_DEV_GPIO=y
CONFIG_DEV_LOOP=y
+CONFIG_DRIVERS_NOTE=y
CONFIG_ETC_FATDEVNO=2
CONFIG_ETC_ROMFS=y
CONFIG_ETC_ROMFSDEVNO=1
@@ -64,6 +65,14 @@ CONFIG_READLINE_TABCOMPLETION=y
CONFIG_SCHED_BACKTRACE=y
CONFIG_SCHED_EVENTS=y
CONFIG_SCHED_HAVE_PARENT=y
+CONFIG_SCHED_INSTRUMENTATION=y
+CONFIG_SCHED_INSTRUMENTATION_DUMP=y
+CONFIG_SCHED_INSTRUMENTATION_FILTER=y
+CONFIG_SCHED_INSTRUMENTATION_FUNCTION=y
+CONFIG_SCHED_INSTRUMENTATION_HEAP=y
+CONFIG_SCHED_INSTRUMENTATION_PREEMPTION=y
+CONFIG_SCHED_INSTRUMENTATION_SWITCH=y
+CONFIG_SCHED_INSTRUMENTATION_WDOG=y
CONFIG_SCHED_WAITPID=y
CONFIG_SIM_HOSTFS=y
CONFIG_SIM_WALLTIME_SIGNAL=y
```
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Draft]libs/instrument: add instrument level check to avoid nesting loops [nuttx]
Posted by "nuttxpr (via GitHub)" <gi...@apache.org>.
nuttxpr commented on PR #15997:
URL: https://github.com/apache/nuttx/pull/15997#issuecomment-2727224462
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues)
This PR does NOT fully meet the NuttX requirements. Here's why:
* **Summary is insufficient:** While it states *what* was changed, it doesn't explain *why* this change is necessary. What problem does the nesting of instrument loops cause? What are the symptoms? Why is this a bug that needs fixing? It also lacks a reference to a related NuttX issue.
* **Impact is incomplete:** Simply stating "N/A" is not acceptable. While some sections *might* be N/A, the author needs to explicitly confirm that for each section. For example, the added compiler flag `-finstrument-functions` certainly has a build impact and needs to be documented. Furthermore, enabling various `CONFIG_SCHED_INSTRUMENTATION_*` options also impacts the build and runtime behavior. These need to be explained.
* **Testing is inadequate:** The provided testing only shows the configuration changes, not the actual *results* of the testing. What were the observed problematic behaviors *before* the change? How do the logs *after* the change demonstrate that the issue is fixed? The provided diffs don't prove anything about functionality. The logs should showcase the issue and the resolution. Furthermore, testing only on `sim/nsh` with VFS instrumentation is likely insufficient. Ideally, testing should cover other scenarios where instrumentation might be used to ensure the fix doesn't introduce regressions elsewhere. Finally, the build host information is missing.
**In short, the PR needs to provide more context, explain the impact thoroughly, and offer concrete evidence that the change works as intended.**
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Draft]libs/instrument: add instrument level check to avoid nesting loops [nuttx]
Posted by "anchao (via GitHub)" <gi...@apache.org>.
anchao closed pull request #15997: [Draft]libs/instrument: add instrument level check to avoid nesting loops
URL: https://github.com/apache/nuttx/pull/15997
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org