Repository navigation
Conversation
…boot The freefile is created once, on the largest free block of the card and at most 4 GB (the largest FAT32 file), and reused as it is at every later boot. Its size is a whole number of superclusters plus one cluster, so once the logs have taken the superclusters one cluster is left: the card reads as full and the blackbox stops logging until the card is formatted, although the rest of a card larger than 4 GB and any log deleted from a PC are free. When the freefile is below 2 GB at boot, search the FAT for the largest free block as on the first boot, and if that is larger, free the old freefile's clusters and move the freefile there. Larger freefiles are reused without a search, as before.
…w it Logs take their clusters from the start of the freefile, so it only ever shrank until the next format, and one log could never grow past what was left of it at boot. On a card larger than 4 GB the clusters that follow the freefile are normally free, so at boot extend it over them, up to the largest file size, leaving AFATFS_FREEFILE_LEAVE_CLUSTERS for regular files as a new freefile does. Only a freefile that is still below 2 GB afterwards is moved to the largest free block. The new clusters are chained and flushed before the freefile's last cluster points at them, and the new size is saved after that, so a power cut leaves at worst unreferenced clusters, never a chain that runs into free ones.
After the skip the search went on reading the FAT sector of the freefile's first cluster while the cluster number had moved past it, so the entries it examined belonged to other clusters. It could only matter when those sectors were already cached, and the search at boot now runs with the freefile in place.
afatfs_FATFillWithPattern() counted a final FAT sector it only writes part of among the sectors to pre-erase. That sector has to be read first, and on an SD card on SPI the read ends the multiple-block write started with ACMD23 before it reaches that block; the SD specification leaves an unwritten pre-erased block undefined, erased or old. Its other entries, which can belong to the next file, could then be read back erased. A new freefile ends that way (whole superclusters plus one cluster), and so does every extension of it.
|
ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing |
PR Summary by QodoRegrow the afatfs blackbox freefile at boot
AI Description
Diagram
High-Level Assessment
Files changed (1)
|
Code Review by Qodo
1. Interrupted extension consumes card space
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
| if (afatfs_flush() && !afatfs.cacheFlushInProgress) { | ||
| const uint32_t oldEnd = afatfs.freeFile.firstCluster + afatfs_freeFileClusters(); | ||
|
|
||
| status = afatfs_FATSetNextCluster(oldEnd - 1, oldEnd); |
There was a problem hiding this comment.
1. Interrupted extension consumes card space 🐞 Bug ☼ Reliability
The extension path flushes the new FAT chain before linking it to the freefile or saving the larger directory size. If power is lost between those writes, reboot uses the old directory size, finds occupied clusters where extension would begin, and cannot reclaim the space.
Agent Prompt
## Issue description
An interrupted extension can leave allocated clusters beyond the freefile's saved size, permanently consuming card space.
## Fix Focus Areas
- src/main/io/asyncfatfs/asyncfatfs.c[3613-3637]
- src/main/io/asyncfatfs/asyncfatfs.c[3649-3686]
## Recommended Fix
Add a recoverable record of the extension or boot-time reconciliation of the FAT chain and directory size. Handle both an unlinked new chain and a linked chain whose larger size was not saved before allowing normal freefile initialization to continue.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
The order is deliberate. The new clusters are chained and flushed before anything points at them, then linked, then the size is saved. A power cut in between can only leave clusters marked in use that no file owns, which a PC's disk check gives back. The other order would leave the freefile's entry covering clusters the FAT still calls free, and the next allocation could hand them to a second file. The window is the few milliseconds the extension takes at mount.
|
Test firmware build ready — commit Download firmware for PR #12056 251 targets built. Find your board's
|
|
RAM / Flash usage vs. base commit
See RAM/flash optimization guide for techniques to reduce usage. |
9f908bf to
cb337c7
Compare
cb337c7 to
4d7f2b3
Compare
…rectory entry each step A power cut while the freefile was regrown or moved at boot left whatever chain was in flight as a lost FAT chain: up to 4 GB of new chain, and up to 2 GB of the old freefile when moving it. Three cuts on a TBS Lucid H7 Wing left three lost chains of 650 MB on average. The new chain now grows in 256 MB blocks, each one chained, linked, saved and flushed before the next. A moved freefile gives its old clusters back the same way from its end: the smaller size is saved, then the chain is cut there, then the tail is freed. A cut strands at most one block either way.
|
I added 0cf3c99: the freefile now grows, and is given back when it moves, 256 MB at a time. Before it, a power cut while afatfs regrew or moved the freefile at boot left the chain in flight as a lost FAT chain: up to 4 GB of new chain, or up to 2 GB of the old freefile during a move. The logs and the freefile were fine after it, but that space stayed lost until chkdsk on a PC. Now each block is chained, linked to the freefile, saved in its directory entry and flushed before the next one. A moved freefile goes back the same way from its end: the smaller size is saved, then its chain is cut there, then the tail is freed. A cut leaves at most one block behind. I tested it on a second TBS Lucid H7 Wing (H743, SDIO). To land a cut at a known point, a bench build resets the MCU at a set millisecond of the next boot and keeps the init phase it was in.
Cost: one directory entry write and one flush per block. On this card the chain was written at 18.4 µs per cluster against 16.9 µs before, about 0.2 s more for 4 GB, only at the boots that build a new chain; a usual boot extends by less than a block and costs the same as before. Flash: +296 B on MATEKF722SE (98.69%, 6,461 B free, the tightest of the 17 F722 targets with an SD card), +344 B on MATEKF405SE, +400 B on the H743. KAKUTEF7 and KAKUTEF7HDV use the same ITCM as before. |
… end and freeing from the front
|
I added 99dc078: the freefile's FAT is now written one sector at a time, each sector on the card before the next one starts. A block that grows the freefile goes from its last FAT sector back to its first, and the tail of a moved freefile is freed from its first sector forward. This is for the cross-linked chain chkdsk found in my earlier tests. A cut while a block's FAT entries were being written left the last written entry pointing at the next cluster, which was still free. When a later boot moved the freefile there, that lost chain ran into the freefile. The block also went through the cache as one multiple-sector write, so the order in which its sectors reached the card wasn't fixed either. Written backwards, a cut leaves a chain that ends in a terminator. I tested it on a TBS Lucid H7 Wing with a bench build that resets the MCU at a set millisecond of the boot. After each cut and the boot that follows, it counts the FAT entries, over the whole card, that point at a free cluster or at the freefile's first cluster:
Cost: none I could measure. The regrow took 9.7 µs per cluster with both orders on this card. Flash: +152 B on MATEKF722SE (98.72%), +240 B on MATEKF405SE, +224 B on KAKUTEF7 and KAKUTEF7HDV, +128 B on the H743; ITCM is unchanged on KAKUTEF7 and KAKUTEF7HDV. |
2 participants
The SD blackbox writes into
FREESPAC.E, a file afatfs preallocates on the largest free block of the card, at most 4 GB (the largest FAT32 file). Each log takes its clusters from the start of it.Nothing ever gives clusters back to the freefile, so it only shrinks until the card is formatted:
So a 16 or 32 GB card stops logging after about 4 GB of logs in total, however many of them get deleted in between. On my board, logging at 1 kHz, that is about 14 hours. In betaflight/betaflight#2947 (same afatfs) the expectation was that the freefile stays at 4 GB until the card is three quarters full; this change makes that true.
The change
Both parts run at boot only, while afatfs opens the freefile.
AFATFS_FREEFILE_LEAVE_CLUSTERSfree for regular files as a new freefile does. On a card larger than 4 GB those clusters are normally free, so every boot starts again with a 4 GB freefile and one log can reach the FAT32 limit. The new clusters are added 256 MB at a time: each block is chained and flushed, the freefile's last cluster is pointed at it, and the new size is saved before the next block, so a power cut leaves at worst one block of unreferenced clusters, and the freefile's chain never runs into free ones.Two afatfs bugs that were already there become easier to reach once the freefile is rebuilt at boot, so this fixes them as well. I found both reading the code for this change, not on the bench:
afatfs_findClusterWithCondition()skips the freefile by moving the cluster number past it, but went on reading the FAT sector of the freefile's first cluster. If that sector was already in the cache, as it tends to be right after the freefile has been written, the entries it examined belonged to other clusters, and in the worst case a regular allocation could get a cluster whose FAT entry it never looked at. The FAT position now follows the cluster number after the skip.afatfs_FATFillWithPattern()asked the card to pre-erase (ACMD23) a last FAT sector it only writes part of. That sector has to be read first, and on an SD card on SPI the read ends the multiple-block write before it gets there; the SD specification leaves a pre-erased block that was never written undefined, erased or old. A new freefile ends that way (whole superclusters plus one cluster), and so does every extension, so the other entries of that sector, which can belong to the next file, could read back as free. Only the sectors it overwrites whole are pre-erased now.I kept FAT32. exFAT would remove the 4 GB limit on a single file, but it is a second filesystem to fit in flash (MATEKF722SE is at 98.5%), and with this change the whole card gets used anyway, 4 GB at a time.
Cost
Measured on a 16 GB card (32 KB clusters) on the H743's SDIO:
Flash, against maintenance-10.x: +948 B on MATEKF722SE (98.49% to 98.69%), +1008 B on MATEKF405SE, +1128 B on TBS_LUCID_H7_WING.
Testing
TBS Lucid H7 Wing (H743, SDIO), 16 GB card, with two bench-only CLI commands: one shrinks the freefile in RAM, one walks its chain in the FAT. I ran these with the threshold at 512 MB and raised it to 2 GB afterwards; the freefiles below are under or over both.
Repeated after the two fixes above, with my other SD PRs on top: at each of three boots the freefile went back to 4,190,240 KB with a contiguous chain, and every log decoded.
Built for MATEKF405SE, MATEKF722SE, MATEKH743, TBS_LUCID_H7_WING and SITL.
Not tested:
If anyone has an F4 or F7 board with the SD on SPI and a card on which the Configurator shows no free space for logs, or just a large card, I'd like to know how long the first boot after flashing takes and whether logging comes back.
After review
The code that runs at mount is kept out of line (
NOINLINE): LTO had pulled it throughafatfs_poll()intoscheduler(), which is in ITCM on F7, and KAKUTEF7 no longer fit its ITCM.The freefile grows, and is given back when it moves, 256 MB at a time, each step saved before the next. A power cut during that work used to leave the whole chain in flight as a lost FAT chain, up to 4 GB; now it leaves at most one block. The power-cut tests are in this comment.
Built on F4, F7, H7, AT32 and SITL; KAKUTEF7 and KAKUTEF7HDV fit their ITCM. On the TBS Lucid H7 Wing after review: two rounds of logging and a reboot, the freefile back to 4 GB each time, its FAT chain contiguous.
With the block-wise steps: built for the 17 F722 targets with an SD card, MATEKF405SE, TBS_LUCID_H7_WING, BLUEBERRYF435WING_SD and ORBITF435_SD; KAKUTEF7 and KAKUTEF7HDV use the same ITCM as before.
Related PRs
asyncfatfs.cor the blackbox; each merges cleanly with this one.