Skip to content

Conversation

@hujun260
Copy link
Contributor

Summary

In x86_64, g_current_regs is still used for context switching.

This commit fixes the regression from #13616

Impact

x86_64

Testing

qemu-intel64:smp (signest_test will Occasional failure not caused by #13616)

@github-actions github-actions bot added Area: Tooling Arch: x86_64 Issues related to the x86_64 architecture Size: L The size of the change in this PR is large labels Sep 30, 2024
@nuttxpr
Copy link

nuttxpr commented Sep 30, 2024

[Experimental Bot, please feedback here]

The PR summary is missing some information:

  • What functional part of the code is being changed? Be specific. What modules/files are affected?
  • How does the change exactly work (what will change and how)? What was the problem caused by the previous PR and how does this PR fix it?

The Impact section is also lacking:

  • Is new feature added? Is existing feature changed? This is a bug fix, so clearly state that an existing feature (context switching) was broken and is now fixed.
  • Impact on user (will user need to adapt to change)? While likely "NO", you should still state it explicitly.
  • Impact on build (will build process change)? Explicitly state "NO".
  • Impact on hardware (will arch(s) / board(s) / driver(s) change)? This specifically impacts x86_64, so state that.
  • Impact on documentation (is update required / provided)? "NO" is likely correct.
  • Impact on security (any sort of implications)? Bug fixes often have security implications. Analyze if this applies here.
  • Impact on compatibility (backward/forward/interoperability)? Consider if this fix could impact compatibility.

The Testing section is insufficient:

  • Build Host(s): Be specific about your build host OS and version.
  • Target(s): You mention qemu-intel64:smp, but be specific about the NuttX configuration used.
  • Testing logs before change: These are required to show the problem existed.
  • Testing logs after change: These are required to show the problem is resolved.
  • Occasional failure: This is concerning. A PR should ideally completely resolve an issue, not just reduce its frequency.

In conclusion, this PR does not yet meet the NuttX requirements. It needs a more complete summary, a fleshed-out impact assessment, and significantly improved testing information, including logs demonstrating the problem and the fix.

reason:
In x86_64, g_current_regs is still used for context switching.

This commit fixes the regression from apache#13616

Signed-off-by: hujun5 <hujun5@xiaomi.com>
@github-actions github-actions bot added Size: XS The size of the change in this PR is very small and removed Size: L The size of the change in this PR is large labels Sep 30, 2024
@masayuki2009 masayuki2009 merged commit 0e87a47 into apache:master Oct 1, 2024
@hujun260 hujun260 deleted the apache_10 branch October 10, 2024 05:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Arch: x86_64 Issues related to the x86_64 architecture Size: XS The size of the change in this PR is very small

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants