Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sched/wdog: use small lock to protect g_wdactivelist #15130

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

hujun260
Copy link
Contributor

@hujun260 hujun260 commented Dec 11, 2024

Summary

sched/wdog: use small lock to protect g_wdactivelist
reason:
We would like to replace the critical section with a small lock.

This PR requires #15129 to be merged first.

Impact

wdog

Testing

ci ostest(qemu-armv8a:nsh_smp)

@github-actions github-actions bot added Area: Drivers Drivers issues Area: OS Components OS Components issues Size: S The size of the change in this PR is small labels Dec 11, 2024
@nuttxpr
Copy link

nuttxpr commented Dec 11, 2024

[Experimental Bot, please feedback here]

No, this PR does not fully meet the NuttX requirements. While it provides a summary and testing information, it lacks crucial details.

Here's what's missing:

  • Summary: Lacks sufficient detail. How does using a small lock work? What specific code changes are made? Simply stating "replace critical section with small lock" is insufficient. What benefits does this change offer (e.g., performance, reduced interrupt latency)?
  • Impact: The impact section is woefully incomplete. It only mentions "wdog" which is not helpful. All "NO/YES" questions need to be answered explicitly, and a description provided for any "YES" answers. Consider impacts like:
    • Performance: Does this change improve or degrade performance?
    • Memory usage: Does this change affect memory usage?
    • Real-time behavior: Are there any changes to real-time guarantees?
  • Testing: While it mentions "ci ostest," it lacks specific details about the results of the tests. Providing "before" and "after" logs is mandatory, even if they are identical (to demonstrate no regressions). The provided logs are empty placeholders. Information about the build host (OS, compiler version) is also missing.

The PR needs substantial revision to meet the NuttX requirements. It needs to provide a more thorough explanation of the changes, a detailed impact assessment, and actual testing logs demonstrating the effect of the change.

drivers/note/note_driver.c Outdated Show resolved Hide resolved
sched/wdog/wdog.h Outdated Show resolved Hide resolved
reason:
We would like to replace the critical section with a small lock.

Signed-off-by: hujun5 <[email protected]>
@github-actions github-actions bot added Size: M The size of the change in this PR is medium and removed Size: S The size of the change in this PR is small labels Dec 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Drivers Drivers issues Area: OS Components OS Components issues Size: M The size of the change in this PR is medium
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants