Skip to content

Conversation

@hujun260
Copy link
Contributor

Summary

In the kernel, we are planning to remove all occurrences of up_cpu_pause as one of the steps to simplify the implementation of critical sections. The goal is to enable spin_lock_irqsave to encapsulate critical sections, thereby facilitating the replacement of critical sections(big lock) with smaller spin_lock_irqsave(small lock)

Impact

setpriority

Testing

Configuring NuttX and compile:
$ ./tools/configure.sh -l qemu-armv8a:nsh_smp
$ make
Running with qemu
$ qemu-system-aarch64 -cpu cortex-a53 -smp 4 -nographic -machine virt,virtualization=on,gic-version=3
-net none -chardev stdio,id=con,mux=on -serial chardev:con -mon chardev=con,mode=readline -kernel ./nuttx

reason:
In the kernel, we are planning to remove all occurrences of up_cpu_pause as one of the steps to
simplify the implementation of critical sections. The goal is to enable spin_lock_irqsave to encapsulate critical sections,
thereby facilitating the replacement of critical sections(big lock) with smaller spin_lock_irqsave(small lock)

Configuring NuttX and compile:
$ ./tools/configure.sh -l qemu-armv8a:nsh_smp
$ make
Running with qemu
$ qemu-system-aarch64 -cpu cortex-a53 -smp 4 -nographic
-machine virt,virtualization=on,gic-version=3
-net none -chardev stdio,id=con,mux=on -serial chardev:con
-mon chardev=con,mode=readline -kernel ./nuttx

Signed-off-by: hujun5 <hujun5@xiaomi.com>
@github-actions github-actions bot added Area: OS Components OS Components issues Size: S The size of the change in this PR is small labels Sep 29, 2024
@nuttxpr
Copy link

nuttxpr commented Sep 29, 2024

[Experimental Bot, please feedback here]

The provided PR information does not meet the NuttX Requirements. Here's why and what's missing:

Missing Information:

  • Summary:
    • What functional part of the code is being changed? Be specific. Mention the files, functions, or modules affected by the removal of up_cpu_pause.
    • How does the change exactly work? Provide a technical explanation of how removing up_cpu_pause contributes to simplifying critical sections and enabling spin_lock_irqsave.
  • Impact:
    • The word "setpriority" seems out of place. You need to systematically address all impact points:
      • Is new feature added? Is existing feature changed? Clarify if this is purely an internal refactoring or if it impacts user-facing functionality.
      • Impact on user: Will any application code using NuttX APIs or behavior be affected?
      • Impact on build: Will any build system changes be required (new configs, dependencies)?
      • Impact on hardware: Are any architectures or boards specifically affected?
      • Impact on documentation: Does any documentation need updates due to code changes or the removal of up_cpu_pause?
      • Impact on security: Could this change introduce any potential security vulnerabilities?
      • Impact on compatibility: Will this affect compatibility with older NuttX versions, applications, or drivers?
  • Testing:
    • Build Host(s): Specify the operating system, CPU architecture, and compiler used for building NuttX.
    • Target(s): Be specific about the target architecture (qemu-armv8a is a good start) and the NuttX configuration used (nsh_smp).
    • Testing logs before change: Provide actual logs demonstrating the behavior before your change.
    • Testing logs after change: Provide logs demonstrating how the behavior is different or improved after your change. The logs should showcase the impact of removing up_cpu_pause.

Recommendations:

  1. Expand on the technical details: Clearly explain the role of up_cpu_pause and how its removal achieves the stated goals.
  2. Thoroughly address impact: Consider all potential consequences of the change, even if the answer is "NO" – provide a brief justification.
  3. Provide concrete testing evidence: Include relevant log snippets that demonstrate the change's effect.

By providing comprehensive information, you'll make it easier for reviewers to understand and approve your PR.

@xiaoxiang781216 xiaoxiang781216 merged commit 0b0f535 into apache:master Oct 5, 2024
@hujun260 hujun260 deleted the apache_7 branch October 10, 2024 05:32
@hujun260 hujun260 mentioned this pull request Oct 16, 2024
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: OS Components OS Components 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.

3 participants