nuttxpr commented on PR #13943: URL: https://github.com/apache/nuttx/pull/13943#issuecomment-2400066235
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) This PR summary is missing crucial information. Here's a breakdown of what's lacking and how to improve it: **Summary - What's Missing:** * **Specificity:** * **What functional part?** Be precise. Is this changing the mutex implementation itself, adding a debugging function, etc.? * **How does it work?** Don't just say "dump support". Explain *how* the backtrace is captured and made available. Is it logged somewhere? Printed on error? Available through a new API call? * **Context:** * **Why is this necessary?** Briefly explain the problem this solves. "Debugging..." isn't enough. Describe the specific debugging scenario (e.g., "Difficult to diagnose deadlocks because..."). * **References:** * **Issues:** If this PR addresses a specific issue in the NuttX repo, link it! **Impact - What's Missing:** * **You MUST address each point:** Just saying "test by ci and real devices" isn't sufficient. Consider each impact category: * **New Feature:** Clearly, this is a YES. Describe the feature briefly. * **User Impact:** Will users need to do anything differently? Enable a config option? Understand new output? * **Build Impact:** Does this change the build process in any way (new dependencies, config options, etc.)? * **Hardware Impact:** Likely NO, but confirm. * **Documentation Impact:** Does this require documentation updates? If so, confirm they're included in the PR. * **Security Impact:** Could this have security implications (e.g., leaking information in backtraces)? Think carefully! * **Compatibility Impact:** Does this break compatibility with any existing code or APIs? **Testing - What's Missing:** * **Specifics:** * **Build Hosts:** List the *specific* OS, CPU architectures, and compiler versions you used for testing. * **Targets:** List the *specific* target architectures, boards, and NuttX configurations you tested on. * **Logs:** * **"Before" and "After" are essential.** Show what the output/behavior was BEFORE your change and how it's different AFTER. This demonstrates the PR's effect. **Example of an Improved Summary:** ``` ## Summary This PR addresses the difficulty in debugging situations where a task exits while holding a mutex, potentially leading to deadlocks. It adds support for dumping the backtrace of the task that last acquired a mutex. This information is made available through a new system call, `nx_mutex_get_holder_backtrace()`, which can be called from a debugger or diagnostic tool. This change modifies the mutex implementation within `nuttx/kernel/pthread/pthread_mutex.c`. This PR resolves [Issue #XYZ](link-to-issue). ## Impact * **Is new feature added?** YES - Adds a new system call (`nx_mutex_get_holder_backtrace()`) for debugging mutex contention. * **Impact on user?** YES - Users will need to be aware of the new system call and how to use it for debugging. * **Impact on build?** NO * **Impact on hardware?** NO * **Impact on documentation?** YES - This PR includes updates to the kernel documentation to describe the new system call and its usage. * **Impact on security?** YES - The backtrace may contain sensitive information about the task's state. This should only be accessible in privileged contexts. * **Impact on compatibility?** NO * **Anything else to consider?** N/A ## Testing **Build Hosts:** * Linux (Ubuntu 20.04), x86_64, GCC 9.3.0 * macOS 11.6.1, Apple M1, Clang 13.0.0 **Targets:** * Simulator (sim), x86_64 * STM32F4 Discovery board, stm32f4discovery:nsh **Testing logs before change:** ``` [Previous output without backtrace information] ``` **Testing logs after change:** ``` [Output demonstrating the new backtrace information] ``` ``` By providing more specifics, context, and evidence of testing, you make it much easier for maintainers to review and merge your PR. -- 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]
