-
Notifications
You must be signed in to change notification settings - Fork 1.5k
drivers/mtd/ftl/bch: Add direct writing method for FTL #16642
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
please remove the merge patch, @jingfei195887 |
92fd5a1 to
83d4a23
Compare
2ac9bff to
bba763f
Compare
jerpelea
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please change commit title to
drivers/bch: Add direct writing method for FTL
thanks for adding the commit message
michallenc
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested with example application in #16624, nxboot update and custom logging library.
cederom
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @jingfei195887 looks interesting :-)
- @michallenc is out expert in this area so I follow all his remarks :-)
- What happens if we use SYNC flag assuming the memory is erased but it is not erased? Is this scenario handled properly with returning error code?
- Please update commits topic prefix to
bch: ...andfs/mtd: .... - Should we mark this change with
[BREAKING]tag in the commit topic too? It changes filesystem / mount / mtd in a non-backward compatible way and this should be clearly marked if someone bumps the release and gets a broken code it will help find a fix quickly. Maybe it would be possible to create new function calls with additional flags as new functionality while old API remains untouched (could call new functions with default parameters)? What other folks think? :-) - Should we also update the documentation on the new behavior (i.e. https://nuttx.apache.org/docs/latest/components/drivers/special/mtd.html)?
We have adjusted the registration method for MTD devices in nuttx/boards, replacing the previous approach using ftl_initialize() and bchdev_register() with register_mtddriver(). When registering MTD devices via register_mtddriver(), FTL and BCH wrappers will be added during the open() process: 1. Character Device Mode: When accessing the MTD device node via the open() interface, the device will be automatically converted to a character device. Both FTL and BCH wrappers will be implicitly added, provided that BCH support is enabled in the configuration. 2. Block Device Mode: When accessing the MTD device node via open_blockdriver(), the device will be treated as a block device, with only the FTL wrapper automatically applied. Due to the automatic wrapping of MTD devices during the open() process, the legacy registration methods ftl_initialize() and bchdev_register() are no longer required for MTD device registration for user code and should be used only internally within fs and driver code. Signed-off-by: jingfei <jingfei@xiaomi.com>
This reverts commit 20fcdcf. The reason is that the erase buffer isn't always used in most cases. Signed-off-by: jingfei <jingfei@xiaomi.com>
…open process. To save more space (equivalent to the size of one erase sector of MTD device) and to achieve faster read and write speeds, a method for direct writing was introduced at the FTL layer. This can be accomplished simply by using the following oflags during the open operation: 1. O_DIRECT. when this flag is passed in, ftl internally uses the direct write strategy and no read cache is used in ftl; otherwise, each write will be executed with the minimum granularity of flash erase sector size which means a "sector read back - erase sector - write sector" operation is performed by using a read cache buffer in heap. 2. O_SYNC. When this flag is passed in, we assume that the flash has been erased in advance and no erasure operation will be performed internally within ftl. O_SYNC will take effect only when both O_DIRECT and O_SYNC are passed in simultaneously. 3. For uniformity, we remapped the mount flag in mount.h and unified it with the open flag in fcntl.h. The repetitive parts of their definitions were reused, and the remaining part of the mount flag redefine to the unused bit of open flags. Signed-off-by: jingfei <jingfei@xiaomi.com>
…Control Documentation: 1. Updates the registration process description and flag usage guidelines 2. Includes the MTD open sequence diagram (mtd_open_flow.png)
da13aba to
9e1177a
Compare
@xiaoxiang781216 @jingfei195887 It's today's (additional!) task. FYI - when I set up the test environment yesterday my existing code seemed to work OK without me yet having changed to the new methodology. So it's not a "breaking" change thankfully and I see the documentation recommends the new method, not demands it. Thank you :-) |
@xiaoxiang781216 @jingfei195887 I have just tested this on my board, using NXboot to flash a recovery image from /dev/mtd0 to /dev/mtd2 and an "ota" update image from /dev/mtd1 to /dev/mtd0 having changed my startup code to directly register /dev/mtd%d instead of using FTL and BCH steps/ I can confirm it works for me. If you want to invite me as a reviewer I'm happy to approve it: if not, I am equally happy to see it merged as soon as you're both done here. I very much appreciate being kept in the loop and being given the opportunity to check it out before it just happened 👍 👍 |
@TimJTi Thank you very much for helping test this feature and really appreciate your work! |
|
Hi @xiaoxiang781216 there is a CI build error in xtensa-01 for
but another problem happened and I can't finish the build (even without my patch): maybe this is for a wrong version of xtensa gcc |
|
@tmedicci @eren-terzioglu could you look at the above problem? |
|
let's ignore esp_tool ci error since it's unrelated to the change in this pr. |
Hi all, seems we still have this issue. I have run a git bisect since this CI error is still happening. |
|
it's strange why host esptool depend on the code change inside fs/mtd:(, @fdcavalcanti . |
|
@xiaoxiang781216 @jingfei195887 Hi guys, I have now tested the patch in my environment and all seems to work as expected. Thanks for all the work and effort with fixing the issues! And thanks to @TimJTi for the quick review with nxboot. |
thankyou |
Thanks for the helping, any process please let me know~ |
So the issue is not related to this MR specifically. Its an alignment issue on ESP32 and it was a coincidence that happened on this PR. Upgrading to esptool v5.0.x fixes the problem and we should keep discussing on #16723. |
since ftl_initialize isn't used anymore after: apache#16642 apache#16738 Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
since ftl_initialize isn't used anymore after: apache#16642 apache#16738 Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
since ftl_initialize isn't used anymore after: apache#16642 apache#16738 Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>




Summary
The registration method for MTD devices in
nuttx/boardshave been adjusted , replacing the previous approach usingftl_initialize()andbchdev_register()withregister_mtddriver().When registering MTD devices viaregister_mtddriver(), FTL and BCH wrappers will be added during the open() process:When accessing the MTD device node via the
open()interface, the device will be automatically converted to a character device. Both FTL and BCH wrappers will be implicitly added, provided that BCH support is enabled in the configuration.When accessing the MTD device node via
open_blockdriver(), the device will be treated as a block device, with only the FTL wrapper automatically applied.Due to the automatic wrapping of MTD devices during the
open()process, the legacy registration methodsftl_initialize()andbchdev_register()are no longer required for MTD device registration for user code and shouldbe used only internally within fs and driver code.
The functions
register_partition_with_mtd()andregister_mtdpartition()are actually wrappers built on top ofregister_mtddriver(), and they can be used to create sub-partition devices for MTD devices.To save more space (equivalent to the size of one erase sector of MTD device) and to achieve faster read and write speeds, a method for direct writing was introduced at the FTL layer. This can be accomplished simply by using the following oflags during the open operation:
O_DIRECT.when this flag is passed in, ftl internally uses the direct write strategy and no read cache is used in ftl;otherwise, each write will be executed with the minimum granularity of flash erase sector size which means a
"sector read back - erase sector - write sector" operation is performed by using a read cache buffer in heap.
O_SYNC.When this flag is passed in, we assume that the flash has been erased in advance and no erase operationwill be performed internally within ftl. O_SYNC will take effect only when both O_DIRECT and O_SYNC are passed in
simultaneously.
For uniformity, we remapped the mount flag in mount.h and unified it with the open flag in fcntl.h. The repetitive
parts of their definitions were reused, and the remaining part of the mount flag redefine to the unused bit of open
flags.**
Impact
The
ftl_initialize()andbchdev_register()methods, originally used to wrap MTD device inodes, are no longer recommended for creating MTD device nodes. Instead, developers are advised to use theregister_mtddriver(),register_partition_with_mtd(), orregister_mtdpartition()functions. If these older methods were previously used in your code, please update your implementation to follow the new recommendations.When an application wants to bypass the following logic in the FTL layer during the process of writing to Flash: using the sector size as the buffer for "read-back - erase - write", the MTD device node can be opened with the O_DIRECT flag. At this point, the FTL will directly write to the MTD device. The open method is as follows:
fd = open("/dev/mtd_device", O_RDWR | O_DIRECT);If the application ensures that the flash has been erased in advance, then adding the O_SYNC logic can control the FTL not to perform erasure. The open method is as follows:
fd = open("/dev/mtd_device", O_RDWR | O_DIRECT | O_SYNC);