Skip to content

Conversation

@hujun260
Copy link
Contributor

Summary

fix compile error

riscv-none-elf-ld: warning: /home/hujun5/downloads1/vela_sim/nuttx/nuttx has a LOAD segment with RWX permissions
riscv-none-elf-ld: /home/hujun5/downloads1/vela_sim/nuttx/staging/libarch.a(riscv_modifyreg32.o): in function spin_lock_irqsave_wo_note': /home/hujun5/downloads1/vela_sim/nuttx/include/nuttx/spinlock.h:534:(.text.modifyreg32+0x1e): undefined reference to this_cpu'
riscv-none-elf-ld: /home/hujun5/downloads1/vela_sim/nuttx/staging/libarch.a(riscv_modifyreg32.o): in function spin_unlock_irqrestore_wo_note': /home/hujun5/downloads1/vela_sim/nuttx/include/nuttx/spinlock.h:701:(.text.modifyreg32+0x82): undefined reference to this_cpu'
riscv-none-elf-ld: /home/hujun5/downloads1/vela_sim/nuttx/staging/libarch.a(bl602_serial.o): in function spin_lock_irqsave_wo_note': /home/hujun5/downloads1/vela_sim/nuttx/include/nuttx/spinlock.h:534:(.text.up_putc+0x12): undefined reference to this_cpu'
riscv-none-elf-ld: /home/hujun5/downloads1/vela_sim/nuttx/staging/libarch.a(bl602_serial.o): in function spin_unlock_irqrestore_wo_note': /home/hujun5/downloads1/vela_sim/nuttx/include/nuttx/spinlock.h:701:(.text.up_putc+0x84): undefined reference to this_cpu'
riscv-none-elf-ld: /home/hujun5/downloads1/vela_sim/nuttx/staging/libsched.a(irq_attach.o): in function spin_lock_irqsave_wo_note': /home/hujun5/downloads1/vela_sim/nuttx/include/nuttx/spinlock.h:534:(.text.irq_attach+0x26): undefined reference to this_cpu'
riscv-none-elf-ld: /home/hujun5/downloads1/vela_sim/nuttx/staging/libsched.a(irq_attach.o):/home/hujun5/downloads1/vela_sim/nuttx/include/nuttx/spinlock.h:701: more undefined references to `this_cpu' follow

Impact

none

Testing

ci

riscv-none-elf-ld: warning: /home/hujun5/downloads1/vela_sim/nuttx/nuttx has a LOAD segment with RWX permissions
riscv-none-elf-ld: /home/hujun5/downloads1/vela_sim/nuttx/staging/libarch.a(riscv_modifyreg32.o): in function `spin_lock_irqsave_wo_note':
/home/hujun5/downloads1/vela_sim/nuttx/include/nuttx/spinlock.h:534:(.text.modifyreg32+0x1e): undefined reference to `this_cpu'
riscv-none-elf-ld: /home/hujun5/downloads1/vela_sim/nuttx/staging/libarch.a(riscv_modifyreg32.o): in function `spin_unlock_irqrestore_wo_note':
/home/hujun5/downloads1/vela_sim/nuttx/include/nuttx/spinlock.h:701:(.text.modifyreg32+0x82): undefined reference to `this_cpu'
riscv-none-elf-ld: /home/hujun5/downloads1/vela_sim/nuttx/staging/libarch.a(bl602_serial.o): in function `spin_lock_irqsave_wo_note':
/home/hujun5/downloads1/vela_sim/nuttx/include/nuttx/spinlock.h:534:(.text.up_putc+0x12): undefined reference to `this_cpu'
riscv-none-elf-ld: /home/hujun5/downloads1/vela_sim/nuttx/staging/libarch.a(bl602_serial.o): in function `spin_unlock_irqrestore_wo_note':
/home/hujun5/downloads1/vela_sim/nuttx/include/nuttx/spinlock.h:701:(.text.up_putc+0x84): undefined reference to `this_cpu'
riscv-none-elf-ld: /home/hujun5/downloads1/vela_sim/nuttx/staging/libsched.a(irq_attach.o): in function `spin_lock_irqsave_wo_note':
/home/hujun5/downloads1/vela_sim/nuttx/include/nuttx/spinlock.h:534:(.text.irq_attach+0x26): undefined reference to `this_cpu'
riscv-none-elf-ld: /home/hujun5/downloads1/vela_sim/nuttx/staging/libsched.a(irq_attach.o):/home/hujun5/downloads1/vela_sim/nuttx/include/nuttx/spinlock.h:701: more undefined references to `this_cpu' follow

Signed-off-by: hujun5 <hujun5@xiaomi.com>
@github-actions github-actions bot added the Size: S The size of the change in this PR is small label Oct 13, 2024
@nuttxpr
Copy link

nuttxpr commented Oct 13, 2024

[Experimental Bot, please feedback here]

The PR summary needs to be more descriptive. Here's a breakdown of the issues and how to improve the PR:

Issues:

  • Vague Summary: "fix compile error" doesn't explain what compile error or how it's being fixed.
  • Missing Information: The "Impact" section is overly simplistic. While the user impact might be minimal, there are likely impacts on the build (at least for the RISC-V architecture).
  • Insufficient Testing: "ci" isn't enough detail. You need to provide:
    • Specific build hosts and target configurations used for testing.
    • Relevant snippets of logs from before and after the change, showing the error being resolved.

Improved PR Summary:

## Summary

This PR fixes a compile-time error encountered on the RISC-V architecture due to missing `this_cpu` symbol references within spinlock functions. The issue arises from the use of `spin_lock_irqsave_wo_note` and `spin_unlock_irqrestore_wo_note` in specific code paths where the `this_cpu` variable is not defined. 

This PR resolves the issue by [briefly explain your solution, e.g., "ensuring the `this_cpu` definition is available in the affected files/modules" or "refactoring the code to avoid direct use of `this_cpu` in those specific instances."]. 

**Related Issue:** [If applicable, link to a NuttX issue]

## Impact

* **Is new feature added? Is existing feature changed?** No.
* **Impact on user (will user need to adapt to change)?** NO 
* **Impact on build (will build process change)?** YES. This fixes a build error specifically affecting the RISC-V architecture.
* **Impact on hardware (will arch(s) / board(s) / driver(s) change)?** NO
* **Impact on documentation (is update required / provided)?** NO 
* **Impact on security (any sort of implications)?** NO 
* **Impact on compatibility (backward/forward/interoperability)?** NO 
* **Anything else to consider?**  N/A

## Testing

I confirm that changes are verified on the following setups:

* **Build Host:** [e.g., Ubuntu 20.04, GCC 10.2.1]
* **Target:**  [e.g., RISC-V simulator, QEMU, specific RISC-V board:config]

**Testing logs before change:**

[Paste relevant log snippet showing the compile errors]


**Testing logs after change:**

[Paste relevant log snippet showing successful compilation]

Key Points:

  • Be clear, concise, and provide specific details.
  • Focus on what the change is, why it's needed, and how it was implemented.
  • Provide concrete evidence of testing to demonstrate the fix.

int me;

#ifdef CONFIG_SMP
me = up_cpu_index();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

define the up cupu index to zero when not in SMP mode

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

up_cpu_index may not zero when in AMP mode

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not revert #14161? or even #14190 since #14200 fix the dead lock.

@hujun260 hujun260 closed this Oct 13, 2024
@hujun260 hujun260 deleted the apache_16 branch October 13, 2024 23:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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