Skip to content

{bp-15163} use atomic to protect f_refs#15213

Merged
xiaoxiang781216 merged 2 commits intoapache:releases/12.8from
jerpelea:bp-15163
Dec 17, 2024
Merged

{bp-15163} use atomic to protect f_refs#15213
xiaoxiang781216 merged 2 commits intoapache:releases/12.8from
jerpelea:bp-15163

Conversation

@jerpelea
Copy link
Contributor

Summary

use atomic to protect f_refs
fix regresion from #14801

includes
#15202

Impact

RELEASE

Testing

CI

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

nuttxpr commented Dec 16, 2024

[Experimental Bot, please feedback here]

No, this PR description does not fully meet the NuttX requirements. While it provides some information, it lacks crucial details. Here's a breakdown:

Missing/Insufficient Information:

  • Summary:

  • Impact:

    • "RELEASE" is not descriptive enough. Does this mean it affects a release branch? Which one? The Impact section requires "YES/NO" answers for each category followed by descriptions if "YES." At minimum, it needs to explicitly address each of the impact categories (user, build, hardware, documentation, security, compatibility). Even if the answer is NO, it should state "NO."
  • Testing:

    • "CI" is also insufficient. While Continuous Integration testing is valuable, the requirements specifically ask for details about local testing. What host and target platforms were tested? What specific tests were run? The provided log sections are empty. Real testing logs are needed to demonstrate the fix and that it doesn't introduce new issues.

How to Improve:

  1. Elaborate on the Summary: Explain what f_refs represents and its function. Describe the specific issue the regression introduced (e.g., race conditions, crashes, data corruption). Explain how using atomic operations resolves the problem. Clarify the relationship with PR fs: fix comment in https://github.com/apache/nuttx/pull/15163 #15202.

  2. Complete the Impact Assessment: Address all impact categories with "YES/NO" and provide explanations for any "YES" responses. If there is no impact in a category, explicitly state "NO." For "RELEASE," specify which release is affected.

  3. Provide Detailed Testing Information: List the specific build host(s) and target(s) used for testing, including OS, CPU architecture, compiler, and board/configuration details. Include actual testing logs (or relevant snippets) demonstrating the problem before the change and the correct behavior after the change. Even if CI tests pass, demonstrating local testing is essential for review.

By providing this missing information, the PR will meet the NuttX requirements and be easier for reviewers to understand and evaluate.

@jerpelea jerpelea closed this Dec 17, 2024
@jerpelea jerpelea deleted the bp-15163 branch December 17, 2024 02:24
@jerpelea jerpelea restored the bp-15163 branch December 17, 2024 02:47
@jerpelea jerpelea reopened this Dec 17, 2024
fix regresion from apache#14801

Signed-off-by: hujun5 <hujun5@xiaomi.com>
Signed-off-by: hujun5 <hujun5@xiaomi.com>
@xiaoxiang781216 xiaoxiang781216 merged commit 9c3fc91 into apache:releases/12.8 Dec 17, 2024
27 checks passed
@jerpelea jerpelea deleted the bp-15163 branch June 23, 2025 08:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: File System File System issues Size: S The size of the change in this PR is small

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants