arch/esp32_himemcdev: Replace file descriptor with node

[nuttxs]

Summary

arch/esp32_himemcdev:

• Multiple instances of struct file for the same file share the inode, ensuring that the mapping status is associated with the file entity rather than a single descriptor.
• Avoid redundant mapping operations caused by multiple descriptors operating on the same file, reducing performance overhead.

Why: The original file-pointer-based check caused incorrect mapping behavior for multiple opens of the same device, leading to memory corruption or leaks.
Consequences: Repeated calls on the same device could corrupt mappings or leave stale resources.

Impact

New Feature/Change:
Bug Fix: Corrects mapping logic for ESP32 himem (no new features).

User Impact:
Functional Change: Users accessing the same himem device from multiple threads/processes will no longer encounter mapping errors.
No Code Changes Required: Existing applications work correctly without modifications.

Build Impact:
No new Kconfig options or build system changes.

Hardware Impact:
ESP32-specific; other architectures unaffected.

Security:
Fixes potential memory corruption vulnerabilities in multi-threaded access.

Compatibility:
Backward-compatible; no breaking changes.

Testing

ESP32-DevKitC, NuttX with CONFIG_ESP32_SPIRAM_BANKSWITCH_ENABLE & CONFIG_TESTING_HIMEM_TEST and testing

nsh> esp32himem_cdev

Himem Cdev test: 1 MB
Memory Size: 1 KB
HimemSizeTest=1024
Data check OK=1 KB
Memory Size: 2 KB
HimemSizeTest=2048
......
Read and write multiple device files!
Memory Size: 16 KB
HimemSizeTest=16384
write HIMEM_DEV1 data:
write HIMEM_DEV2 data:
read HIMEM_DEV1 data:
Data check OK=16 KB
read HIMEM_DEV2 data:
Data check OK=16 KB
Memory Size: 1 MB
HimemSizeTest=1048576
write HIMEM_DEV1 data:
write HIMEM_DEV2 data:
read HIMEM_DEV1 data:
Data check OK=1024 KB
read HIMEM_DEV2 data:
Data check OK=1024 KB
Himemchardev test success!

[nuttxpr]

[Experimental Bot, please feedback here]

This PR appears to partially meet the NuttX requirements. While it provides a summary and some testing information, it lacks crucial details.

Here's a breakdown of what's missing and how to improve it:

• Summary: While the "what" is described, the "why" is missing. Why is sharing f_inode important? What specific problem does it solve? What were the negative consequences of the previous implementation? Mentioning a related NuttX issue would strengthen this section.

• Impact: The impact section is severely lacking. It only mentions "when esp32 device enable himem config," which doesn't explain the impact. Address all the points:

  • Is new feature added? Is existing feature changed?: Clarify this.
  • Impact on user: Will users notice any functional differences? Will their applications need modification?
  • Impact on build: Does enabling this feature require any build system changes? New Kconfig options?
  • Impact on hardware: It mentions esp32, but are other architectures affected?
  • Impact on documentation: Does this require documentation updates? If so, were they included in the PR?
  • Impact on security: Are there any potential security implications?
  • Impact on compatibility: Does this break any existing functionality or compatibility?

• Testing: While testing was performed, the information is insufficient.

  • Build Host(s): Specify the OS, CPU architecture, and compiler version used for building.
  • Target(s): Be more specific than just "esp32 device." Mention the specific board and configuration used.
  • Testing logs: The provided logs are placeholders. Include actual logs demonstrating the behavior before and after the change. Show how the change fixes a problem or improves performance. Ideally, the logs should be concise and highlight the relevant changes.

In short, the PR needs more detail and context to meet the NuttX requirements fully. Expanding the Summary, thoroughly addressing each point in the Impact section, and providing specific testing information will significantly improve the PR's quality and make it easier for reviewers to understand and approve.

[tmedicci]

[Experimental Bot, please feedback here]

This PR appears to partially meet the NuttX requirements. While it provides a summary and some testing information, it lacks crucial details.

Here's a breakdown of what's missing and how to improve it:

• Summary: While the "what" is described, the "why" is missing. Why is sharing f_inode important? What specific problem does it solve? What were the negative consequences of the previous implementation? Mentioning a related NuttX issue would strengthen this section.

• Impact: The impact section is severely lacking. It only mentions "when esp32 device enable himem config," which doesn't explain the impact. Address all the points:

  • Is new feature added? Is existing feature changed?: Clarify this.
  • Impact on user: Will users notice any functional differences? Will their applications need modification?
  • Impact on build: Does enabling this feature require any build system changes? New Kconfig options?
  • Impact on hardware: It mentions esp32, but are other architectures affected?
  • Impact on documentation: Does this require documentation updates? If so, were they included in the PR?
  • Impact on security: Are there any potential security implications?
  • Impact on compatibility: Does this break any existing functionality or compatibility?

• Testing: While testing was performed, the information is insufficient.

  • Build Host(s): Specify the OS, CPU architecture, and compiler version used for building.
  • Target(s): Be more specific than just "esp32 device." Mention the specific board and configuration used.
  • Testing logs: The provided logs are placeholders. Include actual logs demonstrating the behavior before and after the change. Show how the change fixes a problem or improves performance. Ideally, the logs should be concise and highlight the relevant changes.

In short, the PR needs more detail and context to meet the NuttX requirements fully. Expanding the Summary, thoroughly addressing each point in the Impact section, and providing specific testing information will significantly improve the PR's quality and make it easier for reviewers to understand and approve.

Hi @nuttxs , can you please adhere to these recommendations and improve the MR description?

[nuttxs]

Hi @tmedicci have updated the description of the current MR, please review again, thanks