AP_HAL_SITL: serialize storage writes and flushing

A writer could update a line before the flusher cleared its dirty bit,
leaving the new value only in RAM. Snapshot and dequeue under the buffer
semaphore, then perform backend IO under a separate flush semaphore so
parameter and mission reads remain accessible. Use a separate flash image
for compaction, and requeue failed writes without losing concurrent updates.
This commit is contained in:
Andrew Tridgell
2026-09-28 11:35:44 +10:00
parent d589336d4e
commit 131d83de4b
2 changed files with 67 additions and 62 deletions
+63 -60
View File
@@ -120,11 +120,9 @@ void Storage::_storage_open(void)
}
/*
mark some lines as dirty. Note that there is no attempt to avoid
the race condition between this code and the _timer_tick() code
below, which both update _dirty_mask. If we lose the race then the
result is that a line is written more than once, but it won't result
in a line not being written.
Mark lines dirty while holding _sem. The buffer update and dirty-bit
changes must be serialized with flushing so a concurrent write cannot
be lost when _timer_tick() takes a snapshot and clears its dirty bit.
*/
void Storage::_mark_dirty(uint16_t loc, uint16_t length)
{
@@ -141,6 +139,7 @@ void Storage::_mark_dirty(uint16_t loc, uint16_t length)
void Storage::read_block(void *dst, uint16_t loc, size_t n)
{
WITH_SEMAPHORE(_sem);
if (loc >= sizeof(_buffer)-(n-1)) {
return;
}
@@ -150,6 +149,7 @@ void Storage::read_block(void *dst, uint16_t loc, size_t n)
void Storage::write_block(uint16_t loc, const void *src, size_t n)
{
WITH_SEMAPHORE(_sem);
if (loc >= sizeof(_buffer)-(n-1)) {
return;
}
@@ -162,57 +162,68 @@ void Storage::write_block(uint16_t loc, const void *src, size_t n)
void Storage::_timer_tick(void)
{
if (_initialisedType == StorageBackend::None) {
return;
}
if (_dirty_mask.empty()) {
_last_empty_ms = AP_HAL::millis();
return;
}
// write out the first dirty line. We don't write more
// than one to keep the latency of this call to a minimum
// Serialize flushes without blocking buffer access during backend IO.
WITH_SEMAPHORE(_timer_sem);
uint8_t snapshot[STORAGE_LINE_SIZE];
uint16_t i;
for (i=0; i<STORAGE_NUM_LINES; i++) {
if (_dirty_mask.get(i)) {
break;
}
}
if (i == STORAGE_NUM_LINES) {
// this shouldn't be possible
return;
}
#if STORAGE_USE_FRAM
if (fram.write(STORAGE_LINE_SIZE*i, &_buffer[STORAGE_LINE_SIZE*i], STORAGE_LINE_SIZE)) {
_dirty_mask.clear(i);
StorageBackend backend;
{
WITH_SEMAPHORE(_sem);
backend = _initialisedType;
if (backend == StorageBackend::None) {
return;
}
#endif
#if STORAGE_USE_POSIX
if (hal.get_storage_posix_enabled()) {
if (log_fd != -1) {
const off_t offset = STORAGE_LINE_SIZE*i;
if (lseek(log_fd, offset, SEEK_SET) != offset) {
return;
}
if (write(log_fd, &_buffer[offset], STORAGE_LINE_SIZE) != STORAGE_LINE_SIZE) {
return;
}
_dirty_mask.clear(i);
if (_dirty_mask.empty()) {
_last_empty_ms = AP_HAL::millis();
return;
}
}
#endif
// Write out one dirty line per tick.
for (i=0; i<STORAGE_NUM_LINES; i++) {
if (_dirty_mask.get(i)) {
break;
}
}
if (i == STORAGE_NUM_LINES) {
return;
}
memcpy(snapshot, &_buffer[STORAGE_LINE_SIZE*i], sizeof(snapshot));
#if STORAGE_USE_FLASH
if (hal.get_storage_flash_enabled()) {
// save to storage backend
_flash_write(i);
return;
}
if (backend == StorageBackend::Flash) {
// Compaction and error recovery may write the whole storage image.
memcpy(_flash_buffer, _buffer, sizeof(_buffer));
}
#endif
// A concurrent write must queue the line again, even while IO is pending.
_dirty_mask.clear(i);
}
bool written = false;
switch (backend) {
#if STORAGE_USE_FRAM
case StorageBackend::FRAM:
written = fram.write(STORAGE_LINE_SIZE*i, snapshot, sizeof(snapshot));
break;
#endif
#if STORAGE_USE_POSIX
case StorageBackend::SDCard:
written = log_fd != -1 &&
pwrite(log_fd, snapshot, sizeof(snapshot), STORAGE_LINE_SIZE*i) == sizeof(snapshot);
break;
#endif
#if STORAGE_USE_FLASH
case StorageBackend::Flash:
written = _flash.write(STORAGE_LINE_SIZE*i, STORAGE_LINE_SIZE);
break;
#endif
default:
break;
}
if (!written) {
WITH_SEMAPHORE(_sem);
_dirty_mask.set(i);
}
}
#if STORAGE_USE_FLASH
@@ -225,20 +236,9 @@ void Storage::_flash_load(void)
if (!_flash.init()) {
AP_HAL::panic("unable to init flash storage");
}
memcpy(_buffer, _flash_buffer, sizeof(_buffer));
}
/*
write one storage line. This also updates _dirty_mask.
*/
void Storage::_flash_write(uint16_t line)
{
if (_flash.write(line*STORAGE_LINE_SIZE, STORAGE_LINE_SIZE)) {
// mark the line clean
_dirty_mask.clear(line);
}
}
/*
emulate writing to flash
*/
@@ -382,6 +382,7 @@ bool Storage::_flash_erase_ok(void)
*/
bool Storage::healthy(void)
{
WITH_SEMAPHORE(_sem);
if (_initialisedType == StorageBackend::None) {
return false;
}
@@ -393,9 +394,11 @@ bool Storage::healthy(void)
*/
bool Storage::get_storage_ptr(void *&ptr, size_t &size)
{
WITH_SEMAPHORE(_sem);
if (_initialisedType==StorageBackend::None) {
return false;
}
// The caller receives a live buffer, not a snapshot protected by _sem.
ptr = _buffer;
size = sizeof(_buffer);
return true;
+4 -2
View File
@@ -48,6 +48,8 @@ private:
void _mark_dirty(uint16_t loc, uint16_t length);
uint8_t _buffer[HAL_STORAGE_SIZE] __attribute__((aligned(4)));
Bitmask<STORAGE_NUM_LINES> _dirty_mask;
HAL_Semaphore _sem;
HAL_Semaphore _timer_sem;
uint32_t _last_empty_ms;
@@ -60,7 +62,8 @@ private:
bool _flash_failed;
uint32_t _last_re_init_ms;
AP_FlashStorage _flash{_buffer,
uint8_t _flash_buffer[HAL_STORAGE_SIZE] __attribute__((aligned(4)));
AP_FlashStorage _flash{_flash_buffer,
HAL_FLASH_SECTOR_SIZE,
FUNCTOR_BIND_MEMBER(&Storage::_flash_write_data, bool, uint8_t, uint32_t, const uint8_t *, uint16_t),
FUNCTOR_BIND_MEMBER(&Storage::_flash_read_data, bool, uint8_t, uint32_t, uint8_t *, uint16_t),
@@ -68,7 +71,6 @@ private:
FUNCTOR_BIND_MEMBER(&Storage::_flash_erase_ok, bool)};
void _flash_load(void);
void _flash_write(uint16_t line);
#endif
#if STORAGE_USE_POSIX