Skip to content

Change disk format to make fixes and enhancements #147

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.

Sign up for GitHub

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

Open
diondokter wants to merge 3 commits into from
Open

Change disk format to make fixes and enhancements #147

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
02f5e7f
Fix #145
diondokter Oct 13, 2026
e38fcc4
Fix #138
diondokter Oct 13, 2026
cf9c664
Be more strict about (partially) closing pages to avoid extra reads
diondokter Oct 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,11 +90,11 @@ These numbers are taken from the test cases in the cache module:
| Name | RAM bytes | Map # flash reads | Map flash bytes read | Queue # flash reads | Queue flash bytes read |
| --------------------------: | ---------------------------: | -------------------: | -------------------: | ------------------: | ---------------------: |
| All `Uncached` | 0 | 100% | 100% | 100% | 100% |
| Only `CalculatedPageStates` | 24 | 78% | 97% | 44% | 85% |
| Only `ArrayPageStates` | 1 * num pages | 77% | 97% | 41% | 85% |
| Only `ArrayPagePointers` | 8 * num pages | 92% | 92% | 65% | 28% |
| Only `ArrayKeyPointers` | (sizeof(KEY) + 4) * num keys | half: 82%, full: 22% | half: 80%, full: 17% | - | - |
| All array caches together | | 6% | 8% | 6% | 14% |
| Only `CalculatedPageStates` | 24 | 87% | 97% | 59% | 86% |
| Only `ArrayPageStates` | 1 * num pages | 87% | 97% | 59% | 86% |
| Only `ArrayPagePointers` | 8 * num pages | 91% | 92% | 49% | 28% |
| Only `ArrayKeyPointers` | (sizeof(KEY) + 4) * num keys | half: 82%, full: 20% | half: 80%, full: 17% | - | - |
| All array caches together | | 7% | 8% | 9% | 14% |

#### Takeaways

Expand Down
2 changes: 1 addition & 1 deletion src/cache/page_pointers.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ impl SealedPagePointersCache for TrackedPagePointers<'_> {

// Either the item we point to or the first item on the page
let next_unerased_item = self.first_item_after_erased(page_index).unwrap_or_else(|| {
calculate_page_address::<S>(flash_range, page_index) + S::WORD_SIZE as u32
calculate_page_address::<S>(flash_range, page_index) + S::WORD_SIZE as u32 * 2
});

if item_address == next_unerased_item {
Expand Down
28 changes: 14 additions & 14 deletions src/cache/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@ mod queue_tests {
run_test(Cache::new_uncached()).await,
FlashStatsResult {
erases: 149,
reads: 165009,
reads: 114520,
writes: 6299,
bytes_read: 651212,
bytes_read: 647214,
bytes_written: 53299
}
);
Expand All @@ -41,7 +41,7 @@ mod queue_tests {
.await,
FlashStatsResult {
erases: 149,
reads: 68111,
reads: 68070,
writes: 6299,
bytes_read: 554314,
bytes_written: 53299
Expand All @@ -60,7 +60,7 @@ mod queue_tests {
.await,
FlashStatsResult {
erases: 149,
reads: 68037,
reads: 68033,
writes: 6299,
bytes_read: 554240,
bytes_written: 53299
Expand All @@ -79,9 +79,9 @@ mod queue_tests {
.await,
FlashStatsResult {
erases: 149,
reads: 106931,
reads: 56442,
writes: 6299,
bytes_read: 186588,
bytes_read: 182590,
bytes_written: 53299
}
);
Expand All @@ -98,7 +98,7 @@ mod queue_tests {
.await,
FlashStatsResult {
erases: 149,
reads: 9959,
reads: 9955,
writes: 6299,
bytes_read: 89616,
bytes_written: 53299
Expand Down Expand Up @@ -170,7 +170,7 @@ mod map_tests {
run_test(Cache::new_uncached()).await,
FlashStatsResult {
erases: 198,
reads: 233786,
reads: 207470,
writes: 5201,
bytes_read: 1837101,
bytes_written: 50401
Expand All @@ -189,7 +189,7 @@ mod map_tests {
.await,
FlashStatsResult {
erases: 198,
reads: 181262,
reads: 181208,
writes: 5201,
bytes_read: 1784577,
bytes_written: 50401
Expand All @@ -208,7 +208,7 @@ mod map_tests {
.await,
FlashStatsResult {
erases: 198,
reads: 181162,
reads: 181158,
writes: 5201,
bytes_read: 1784477,
bytes_written: 50401
Expand All @@ -227,7 +227,7 @@ mod map_tests {
.await,
FlashStatsResult {
erases: 198,
reads: 215897,
reads: 189581,
writes: 5201,
bytes_read: 1693989,
bytes_written: 50401
Expand All @@ -246,7 +246,7 @@ mod map_tests {
.await,
FlashStatsResult {
erases: 198,
reads: 191876,
reads: 170630,
writes: 5201,
bytes_read: 1484871,
bytes_written: 50401
Expand All @@ -265,7 +265,7 @@ mod map_tests {
.await,
FlashStatsResult {
erases: 198,
reads: 52403,
reads: 42397,
writes: 5201,
bytes_read: 313708,
bytes_written: 50401
Expand All @@ -284,7 +284,7 @@ mod map_tests {
.await,
FlashStatsResult {
erases: 198,
reads: 14510,
reads: 14506,
writes: 5201,
bytes_read: 150592,
bytes_written: 50401
Expand Down
44 changes: 31 additions & 13 deletions src/item.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -556,10 +556,10 @@ impl<S: NorFlash, C: CacheImpl<KEY>, KEY> GenericStorage<S, C, KEY> {
page_index: usize,
) -> Result<bool, Error<S::Error>> {
let page_data_start_address =
calculate_page_address::<S>(self.flash_range.clone(), page_index) + S::WORD_SIZE as u32;
calculate_page_address::<S>(self.flash_range.clone(), page_index)
+ S::WORD_SIZE as u32 * 2;
let page_data_end_address =
calculate_page_end_address::<S>(self.flash_range.clone(), page_index)
- S::WORD_SIZE as u32;
calculate_page_end_address::<S>(self.flash_range.clone(), page_index);

Ok(ItemHeaderIter::new(
self.cache
Expand Down Expand Up @@ -610,15 +610,17 @@ impl ItemIter {
}

pub(crate) struct ItemHeaderIter {
current_address: u32,
next_address: u32,
end_address: u32,
item_corrupted: bool,
}

impl ItemHeaderIter {
pub(crate) fn new(start_address: u32, end_address: u32) -> Self {
Self {
current_address: start_address,
next_address: start_address,
end_address,
item_corrupted: false,
}
}

Expand All @@ -640,22 +642,38 @@ impl ItemHeaderIter {
callback: impl Fn(&ItemHeader, u32) -> bool,
) -> Result<(Option<ItemHeader>, u32), Error<S::Error>> {
loop {
match ItemHeader::read_new(flash, self.current_address, self.end_address).await {
let read_result =
ItemHeader::read_new(flash, self.next_address, self.end_address).await;

let last_item_corrupted = core::mem::replace(
&mut self.item_corrupted,
matches!(read_result, Err(Error::Corrupted { .. })),
);

match read_result {
Ok(Some(header)) => {
let next_address = header.next_item_address::<S>(self.current_address);
if callback(&header, self.current_address) {
self.current_address = next_address;
let next_address = header.next_item_address::<S>(self.next_address);
if callback(&header, self.next_address) {
self.next_address = next_address;
} else {
let current_address = self.current_address;
self.current_address = next_address;
let current_address = self.next_address;
self.next_address = next_address;
return Ok((Some(header), current_address));
}
}
Ok(None) => {
return Ok((None, self.current_address));
return Ok((None, self.next_address));
}
Err(Error::Corrupted { .. }) => {
self.current_address += S::WORD_SIZE as u32;
self.next_address += if last_item_corrupted {
// We skip a full header so we can't start writing into what could be a possible header.
// If we did write into a corrupted header, it might suddenly (wrongly) be deemed valid again.
round_up_to_alignment::<S>(ItemHeader::LENGTH as u32)
} else {
// We've already skipped a full header, so now we have no information anymore except that
// a header must be word aligned, so look at every word
S::WORD_SIZE as u32
};
}
Err(e) => return Err(e),
}
Expand Down
74 changes: 22 additions & 52 deletions src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,37 +201,18 @@ impl<S: NorFlash, C: CacheImpl<KEY>, KEY> GenericStorage<S, C, KEY> {
/// So only half of the byte needs to be zero.
const HALF_MARKER_BITS: u32 = 4;

let mut buffer = [0; MAX_WORD_SIZE];
let mut buffer = [0; MAX_WORD_SIZE * 2];
self.flash
.read(page_address, &mut buffer[..S::READ_SIZE])
.read(page_address, &mut buffer[..S::WORD_SIZE * 2])
.await
.map_err(|e| Error::Storage {
value: e,
#[cfg(feature = "_test")]
backtrace: std::backtrace::Backtrace::capture(),
})?;
let start_marked = buffer[..S::READ_SIZE]
.iter()
.map(|marker_byte| marker_byte.count_zeros())
.sum::<u32>()
>= HALF_MARKER_BITS;

self.flash
.read(
page_address + (S::ERASE_SIZE - S::READ_SIZE) as u32,
&mut buffer[..S::READ_SIZE],
)
.await
.map_err(|e| Error::Storage {
value: e,
#[cfg(feature = "_test")]
backtrace: std::backtrace::Backtrace::capture(),
})?;
let end_marked = buffer[..S::READ_SIZE]
.iter()
.map(|marker_byte| marker_byte.count_zeros())
.sum::<u32>()
>= HALF_MARKER_BITS;
let start_marked = buffer[0].count_zeros() >= HALF_MARKER_BITS;
let end_marked = buffer[S::WORD_SIZE].count_zeros() >= HALF_MARKER_BITS;

let discovered_state = match (start_marked, end_marked) {
(true, true) => PageState::Closed,
Expand Down Expand Up @@ -272,22 +253,20 @@ impl<S: NorFlash, C: CacheImpl<KEY>, KEY> GenericStorage<S, C, KEY> {

/// Fully closes a page by writing both the start and end marker
async fn close_page(&mut self, page_index: usize) -> Result<(), Error<S::Error>> {
let current_state = self.partial_close_page(page_index).await?;

if current_state != PageState::PartialOpen {
return Ok(());
#[cfg(debug_assertions)]
if self.get_page_state_cached(page_index).await? != PageState::PartialOpen {
panic!("Not open");
}

self.cache
.notice_page_state(page_index, PageState::Closed, true);

let buffer = AlignedBuf([MARKER; MAX_WORD_SIZE]);
let page_end_address =
calculate_page_end_address::<S>(self.flash_range.clone(), page_index)
- S::WORD_SIZE as u32;
let second_word_address =
calculate_page_address::<S>(self.flash_range.clone(), page_index) + S::WORD_SIZE as u32;
// Close the end marker
self.flash
.write(page_end_address, &buffer[..S::WORD_SIZE])
.write(second_word_address, &buffer[..S::WORD_SIZE])
.await
.map_err(|e| Error::Storage {
value: e,
Expand All @@ -299,22 +278,14 @@ impl<S: NorFlash, C: CacheImpl<KEY>, KEY> GenericStorage<S, C, KEY> {
}

/// Partially close a page by writing the start marker
async fn partial_close_page(
&mut self,
page_index: usize,
) -> Result<PageState, Error<S::Error>> {
let current_state = self.get_page_state_cached(page_index).await?;

if current_state != PageState::Open {
return Ok(current_state);
async fn partial_close_page(&mut self, page_index: usize) -> Result<(), Error<S::Error>> {
#[cfg(debug_assertions)]
if self.get_page_state_cached(page_index).await? != PageState::Open {
panic!("Not open");
}

let new_state = match current_state {
PageState::Closed => PageState::Closed,
PageState::PartialOpen | PageState::Open => PageState::PartialOpen,
};

self.cache.notice_page_state(page_index, new_state, true);
self.cache
.notice_page_state(page_index, PageState::PartialOpen, true);

let buffer = AlignedBuf([MARKER; MAX_WORD_SIZE]);
let page_start_address = calculate_page_address::<S>(self.flash_range.clone(), page_index);
Expand All @@ -328,7 +299,7 @@ impl<S: NorFlash, C: CacheImpl<KEY>, KEY> GenericStorage<S, C, KEY> {
backtrace: std::backtrace::Backtrace::capture(),
})?;

Ok(new_state)
Ok(())
}

#[cfg(any(test, feature = "std", fuzzing))]
Expand All @@ -355,10 +326,9 @@ impl<S: NorFlash, C: CacheImpl<KEY>, KEY> GenericStorage<S, C, KEY> {
.unwrap();
let page_data_start =
crate::calculate_page_address::<S>(self.flash_range.clone(), page_index)
+ S::WORD_SIZE as u32;
+ S::WORD_SIZE as u32 * 2;
let page_data_end =
crate::calculate_page_end_address::<S>(self.flash_range.clone(), page_index)
- S::WORD_SIZE as u32;
crate::calculate_page_end_address::<S>(self.flash_range.clone(), page_index);

let mut it = crate::item::ItemHeaderIter::new(page_data_start, page_data_end);
while let (Some(header), item_address) =
Expand Down Expand Up @@ -465,7 +435,7 @@ const MARKER: u8 = 0;
/// The state of a page
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
#[cfg_attr(feature = "defmt", derive(defmt::Format))]
pub enum PageState {
pub(crate) enum PageState {
/// This page was fully written and has now been sealed
Closed,
/// This page has been written to, but may have some space left over
Expand Down Expand Up @@ -689,14 +659,14 @@ mod tests {
write_aligned(&mut flash, 0x000, &[MARKER, 0, 0, 0])
.await
.unwrap();
write_aligned(&mut flash, 0x100 - 4, &[0, 0, 0, MARKER])
write_aligned(&mut flash, 0x000 + 4, &[MARKER, 0, 0, 0])
.await
.unwrap();
// Page 1 markers
write_aligned(&mut flash, 0x100, &[MARKER, 0, 0, 0])
.await
.unwrap();
write_aligned(&mut flash, 0x200 - 4, &[0, 0, 0, MARKER])
write_aligned(&mut flash, 0x100 + 4, &[MARKER, 0, 0, 0])
.await
.unwrap();
// Page 2 markers
Expand Down
Loading
Loading