Fix-Point commented on PR #3936: URL: https://github.com/apache/nuttx-apps/pull/3936#issuecomment-6512946413
Thanks for the detailed review. I've addressed points 2-4 plus the sign-off comment in the force-pushed branch: - `testing/ostest/spinlock.c`: use `sched_setaffinity(0, ...)` so the calling test thread is pinned (the main-ostest-thread side effect is gone); - `testing/ostest/Kconfig`: the `TEST_LOOP_SCALE` help now reads `"scale * 10000"`, matching `LOOP_TIMES`; - `testing/ostest/spinlock.c`: the `rspinlock_t` comment now says "recursive spinlock" instead of "read-write spinlock"; - the two commits previously authored by the vela automation accounts are now authored and signed off by a human (`ouyangxiangzhen <[email protected]>`), with the `Co-authored-by` markers kept. Regarding point 1 (the thread-priority option), I'd respectfully like to keep `CONFIG_TESTING_OSTEST_SPINLOCK_PRIORITY`. `spinlock_test` is not only a functional test (the final counters) but also a **performance test**: it reports throughput in op/s and total execution time. If the test threads stay at the caller's default priority, higher-priority threads (e.g. the shell or other ostest sub-tests) can preempt them, which corrupts the reported throughput figures. Boosting the test threads to a high priority is therefore required to keep the performance measurements meaningful, and the Kconfig option lets watchdog-monitored platforms (e.g. BMP + WdgM) choose a lower value when needed. Could you re-review? Happy to adjust if you still disagree. -- 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]
