Repository navigation
Conversation
afatfs_getFreeBufferSpace() counted every unlocked sector that was empty or in sync, but afatfs_allocateCacheSector() never evicts one with a retain count: a contiguous file keeps its directory entry retained for as long as it is open, and the blackbox log and the free file are both contiguous. With a log open, two of the 8 sectors were reported as space the blackbox could write into, and could not be. Measured on a TBS Lucid H7 while the SD card held a write for 54 ms: afatfs_fwrite() refused bytes while this function still reported 1024 bytes free.
An SD card can hold a write for tens of milliseconds (the standard
allows 250). The blackbox keeps writing into the other cache sectors
meanwhile, and once none is left it drops what it writes. With 8
sectors, and 3 of them never free while a log is open, the cache held
about 30 ms of a 1 kHz log.
Measured on a TBS Lucid H7 whose card holds a write for 54 ms every
768 KB once it runs out of prepared blocks, 120 s logs:
sectors 1 kHz log (85 kB/s) 2 kHz log (170 kB/s)
rows lost rows lost
8 379 2178
12 8 1282
16 0 703
24 0 0
32 0 0 (also over a 20 minute log)
Now 32 sectors on H7 and 24 on F7, where RAM is plenty. On F4 with the
card on SPI the cache moves to CCM with 24 sectors: RAM is short there,
CCM mostly unused, and the CPU writes the card, so no DMA needs to
reach it. Elsewhere (F4 with SDIO, AT32) it stays at 8. A target can
set AFATFS_NUM_CACHE_SECTORS itself. Targets without an SD card do not
link the cache.
Walking the larger cache costs afatfs_poll() about 8 ns per sector on
the H7, in the scheduler's idle loop.
|
ⓘ 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 QodoCorrect afatfs free-space accounting and size the SD cache by MCU family
AI Description
Diagram
High-Level Assessment
Files changed (1)
|
Code Review by Qodo
1.
|
|
RAM / Flash usage vs. base branch — commit
See RAM/flash optimization guide for techniques to reduce usage. |
|
Test firmware build ready — commit Download firmware for PR #12057 251 targets built. Find your board's
|
69326cc to
4430500
Compare
4430500 to
430bc19
Compare
|
One more run on the TBS Lucid H7 Wing, heavier than the table above: 8 motors and 18 servos in the mixer, every field group on, logging every PID loop (about 1980 Hz, 240 KB/s), 60 s, same card. A bench build of #12058, which carries this PR and so the 32 sectors of H7: 119 025 frames, no iteration missing, and the pause of #12058 never triggered. maintenance-10.x with its 8 sectors: 1808 iterations missing (911 ms) in 18 gaps, and 19 corrupt frames inside the log, which make 419 iterations unreadable. |
|
Also on another TBS Lucid H7 Wing with its own card, at a lighter load (1 kHz, 63 kB/s): three 3-minute logs from maintenance-10.x with 8 sectors lost 1.9 to 2.4% of their iterations, frames cut short at 512-byte sector boundaries; four logs from a #12058 bench build, with this PR's 32 sectors, lost none. Longer runs on that card still hit an occasional write stall of 0.2 to 0.6 s with 32 sectors (#12060's tests), which is #12058's part. Those logs have all nine optional P fields, so they decode only with iNavFlight/blackbox-tools#102. |
The SD blackbox drops rows whenever the card takes long over a write. SD cards may hold a write for up to 250 ms by the standard; the one I test with holds one for 54 ms every 768 KB, once it runs out of prepared blocks. Meanwhile the blackbox keeps writing into afatfs's sector cache, and when no sector is left
afatfs_fwrite()drops what does not fit.Two things made that happen sooner than it should:
afatfs_getFreeBufferSpace()counted as free the sectors that have a retain count, butafatfs_allocateCacheSector()never evicts those. A contiguous file keeps its directory sector retained while it is open, and the blackbox log and the free file are both contiguous, so with a log open two of the 8 sectors were reported as space that wasn't there. I caughtafatfs_fwrite()refusing bytes while this function still reported 1024 bytes free.The change
afatfs_getFreeBufferSpace()skips retained sectors.AFATFS_NUM_CACHE_SECTORScan be set per target, and defaults by family: 32 sectors on H7 and 24 on F7, where RAM is plenty; 24 on F4 with the card on SPI, in CCM (RAM is short there, CCM mostly unused, and the CPU writes the card so no DMA has to reach it); 8 elsewhere (F4 with SDIO, AT32), as today. Targets without an SD card don't link the cache. Betaflight's cache is 11 sectors by default and overridable per target.Results
TBS Lucid H7 Wing (H743, SDIO), 120 s logs, rows lost:
RAM: +12 KB on H7, +8.4 KB on MATEKF722SE (of 192 KB), and on MATEKF405SE (SD on SPI) 3.6 KB less main RAM and 12 KB more CCM. Walking the larger cache costs
afatfs_poll()about 8 ns per sector on the H7, in the scheduler's idle loop.Built for F405 with SD on SPI and on SDIO, F722, H743, AT32, a target without SD, and SITL.
Not tested:
If anyone logs to SD on an F4 or F7, a before/after log at the same settings would help, especially at 1/1 logging rate.
After review
Built on F4, F7, H7, AT32 and SITL; KAKUTEF7 and KAKUTEF7HDV fit their ITCM. On the TBS Lucid H7 Wing after review: a 60 s log at every PID loop (1983 Hz), no iteration missing.
Related PRs