Skip to content

Commit

Permalink
Fix wrongful cache invalidation issue
Browse files Browse the repository at this point in the history
  • Loading branch information
@diondokter
diondokter committed Mar 26, 2024
1 parent 95d39f6 commit 342ca4f
Show file tree
Hide file tree
Showing 6 changed files with 102 additions and 62 deletions.
24 changes: 24 additions & 0 deletions src/cache/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,12 @@ impl NoCache {
}
}

impl Default for NoCache {
fn default() -> Self {
Self::new()
}
}

impl PrivateCacheImpl for NoCache {
type PSC = UncachedPageStates;
type PPC = UncachedPagePointers;
Expand Down Expand Up @@ -279,6 +285,12 @@ impl<const PAGE_COUNT: usize> PageStateCache<PAGE_COUNT> {
}
}

impl<const PAGE_COUNT: usize> Default for PageStateCache<PAGE_COUNT> {
fn default() -> Self {
Self::new()
}
}

impl<const PAGE_COUNT: usize> PrivateCacheImpl for PageStateCache<PAGE_COUNT> {
type PSC = CachedPageStates<PAGE_COUNT>;
type PPC = UncachedPagePointers;
Expand Down Expand Up @@ -346,6 +358,12 @@ impl<const PAGE_COUNT: usize> PagePointerCache<PAGE_COUNT> {
}
}

impl<const PAGE_COUNT: usize> Default for PagePointerCache<PAGE_COUNT> {
fn default() -> Self {
Self::new()
}
}

impl<const PAGE_COUNT: usize> PrivateCacheImpl for PagePointerCache<PAGE_COUNT> {
type PSC = CachedPageStates<PAGE_COUNT>;
type PPC = CachedPagePointers<PAGE_COUNT>;
Expand Down Expand Up @@ -418,6 +436,12 @@ impl<const PAGE_COUNT: usize, KEY: Eq, const KEYS: usize> KeyPointerCache<PAGE_C
}
}

impl<const PAGE_COUNT: usize, KEY: Eq, const KEYS: usize> Default for KeyPointerCache<PAGE_COUNT, KEY, KEYS> {
fn default() -> Self {
Self::new()
}
}

impl<const PAGE_COUNT: usize, KEY: Eq, const KEYS: usize> PrivateCacheImpl
for KeyPointerCache<PAGE_COUNT, KEY, KEYS>
{
Expand Down
57 changes: 37 additions & 20 deletions src/cache/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,9 @@ mod map_tests {
run_test(&mut NoCache::new()).await,
FlashStatsResult {
erases: 198,
reads: 224161,
reads: 233786,
writes: 5201,
bytes_read: 1770974,
bytes_read: 1837101,
bytes_written: 50401
}
);
Expand All @@ -139,9 +139,9 @@ mod map_tests {
run_test(&mut PageStateCache::<NUM_PAGES>::new()).await,
FlashStatsResult {
erases: 198,
reads: 171543,
reads: 181162,
writes: 5201,
bytes_read: 1718356,
bytes_read: 1784477,
bytes_written: 50401
}
);
Expand All @@ -153,9 +153,9 @@ mod map_tests {
run_test(&mut PagePointerCache::<NUM_PAGES>::new()).await,
FlashStatsResult {
erases: 198,
reads: 153667,
reads: 163273,
writes: 5201,
bytes_read: 1575348,
bytes_read: 1641365,
bytes_written: 50401
}
);
Expand All @@ -164,12 +164,12 @@ mod map_tests {
#[test]
async fn key_pointer_cache_half() {
assert_eq!(
run_test(&mut KeyPointerCache::<NUM_PAGES, u8, 12>::new()).await,
run_test(&mut KeyPointerCache::<NUM_PAGES, u16, 12>::new()).await,
FlashStatsResult {
erases: 198,
reads: 130523,
reads: 131503,
writes: 5201,
bytes_read: 1318479,
bytes_read: 1299275,
bytes_written: 50401
}
);
Expand All @@ -178,18 +178,18 @@ mod map_tests {
#[test]
async fn key_pointer_cache_full() {
assert_eq!(
run_test(&mut KeyPointerCache::<NUM_PAGES, u8, 24>::new()).await,
run_test(&mut KeyPointerCache::<NUM_PAGES, u16, 24>::new()).await,
FlashStatsResult {
erases: 198,
reads: 14506,
reads: 14510,
writes: 5201,
bytes_read: 150566,
bytes_read: 150592,
bytes_written: 50401
}
);
}

async fn run_test(cache: &mut impl KeyCacheImpl<u8>) -> FlashStatsResult {
async fn run_test(cache: &mut impl KeyCacheImpl<u16>) -> FlashStatsResult {
let mut flash =
mock_flash::MockFlashBase::<NUM_PAGES, 1, 256>::new(WriteCountCheck::Twice, None, true);
const FLASH_RANGE: Range<u32> = 0x00..0x400;
Expand All @@ -202,19 +202,36 @@ mod map_tests {
let start_snapshot = flash.stats_snapshot();

for _ in 0..100 {
for i in 0..24 {
store_item(&mut flash, FLASH_RANGE, cache, &mut data_buffer, i as u8, &vec![i as u8; LENGHT_PER_KEY[i]].as_slice())
.await
.unwrap();
const WRITE_ORDER: [usize; 24] = [
15, 0, 4, 22, 18, 11, 19, 8, 14, 23, 5, 1, 16, 10, 6, 12, 20, 17, 3, 9, 7, 13, 21,
2,
];

for i in WRITE_ORDER {
store_item(
&mut flash,
FLASH_RANGE,
cache,
&mut data_buffer,
i as u16,
&vec![i as u8; LENGHT_PER_KEY[i]].as_slice(),
)
.await
.unwrap();
}

for i in 0..24 {
let item = fetch_item::<u8, &[u8], _>(
const READ_ORDER: [usize; 24] = [
8, 22, 21, 11, 16, 23, 13, 15, 19, 7, 6, 2, 12, 1, 17, 4, 20, 14, 10, 5, 9, 3, 18,
0,
];

for i in READ_ORDER {
let item = fetch_item::<u16, &[u8], _>(
&mut flash,
FLASH_RANGE,
cache,
&mut data_buffer,
i as u8,
i as u16,
)
.await
.unwrap()
Expand Down
32 changes: 14 additions & 18 deletions src/item.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@
//! and has some other modifications to make corruption less likely to happen.
//!
use core::ops::Range;
use core::num::NonZeroU32;
use core::ops::Range;

use embedded_storage_async::nor_flash::{MultiwriteNorFlash, NorFlash};

Expand Down Expand Up @@ -54,7 +54,7 @@ impl ItemHeader {
flash: &mut S,
address: u32,
end_address: u32,
) -> Result<Option<Self>, Error< S::Error>> {
) -> Result<Option<Self>, Error<S::Error>> {
let mut buffer = [0; MAX_WORD_SIZE];
let header_slice = &mut buffer[..round_up_to_alignment_usize::<S>(Self::LENGTH)];

Expand Down Expand Up @@ -104,7 +104,7 @@ impl ItemHeader {
data_buffer: &'d mut [u8],
address: u32,
end_address: u32,
) -> Result<MaybeItem<'d>, Error< S::Error>> {
) -> Result<MaybeItem<'d>, Error<S::Error>> {
match self.crc {
None => Ok(MaybeItem::Erased(self, data_buffer)),
Some(header_crc) => {
Expand Down Expand Up @@ -141,11 +141,7 @@ impl ItemHeader {
}
}

async fn write<S: NorFlash>(
&self,
flash: &mut S,
address: u32,
) -> Result<(), Error< S::Error>> {
async fn write<S: NorFlash>(&self, flash: &mut S, address: u32) -> Result<(), Error<S::Error>> {
let mut buffer = AlignedBuf([0xFF; MAX_WORD_SIZE]);

buffer[Self::DATA_CRC_FIELD]
Expand Down Expand Up @@ -174,7 +170,7 @@ impl ItemHeader {
flash_range: Range<u32>,
cache: &mut impl PrivateCacheImpl,
address: u32,
) -> Result<Self, Error< S::Error>> {
) -> Result<Self, Error<S::Error>> {
self.crc = None;
cache.notice_item_erased::<S>(flash_range.clone(), address, &self);
self.write(flash, address).await?;
Expand Down Expand Up @@ -223,7 +219,7 @@ impl<'d> Item<'d> {
cache: &mut impl PrivateCacheImpl,
address: u32,
data: &'d [u8],
) -> Result<ItemHeader, Error< S::Error>> {
) -> Result<ItemHeader, Error<S::Error>> {
let header = ItemHeader {
length: data.len() as u16,
crc: Some(adapted_crc32(data)),
Expand All @@ -241,7 +237,7 @@ impl<'d> Item<'d> {
header: &ItemHeader,
data: &[u8],
address: u32,
) -> Result<(), Error< S::Error>> {
) -> Result<(), Error<S::Error>> {
cache.notice_item_written::<S>(flash_range, address, header);
header.write(flash, address).await?;

Expand Down Expand Up @@ -282,7 +278,7 @@ impl<'d> Item<'d> {
flash_range: Range<u32>,
cache: &mut impl PrivateCacheImpl,
address: u32,
) -> Result<(), Error< S::Error>> {
) -> Result<(), Error<S::Error>> {
Self::write_raw(
flash,
flash_range,
Expand Down Expand Up @@ -340,7 +336,7 @@ pub async fn find_next_free_item_spot<S: NorFlash>(
start_address: u32,
end_address: u32,
data_length: u32,
) -> Result<Option<u32>, Error< S::Error>> {
) -> Result<Option<u32>, Error<S::Error>> {
let page_index = calculate_page_index::<S>(flash_range, start_address);

let free_item_address = match cache.first_item_after_written(page_index) {
Expand Down Expand Up @@ -392,7 +388,7 @@ impl<'d> core::fmt::Debug for MaybeItem<'d> {
}

impl<'d> MaybeItem<'d> {
pub fn unwrap<E>(self) -> Result<Item<'d>, Error< E>> {
pub fn unwrap<E>(self) -> Result<Item<'d>, Error<E>> {
match self {
MaybeItem::Corrupted(_, _) => Err(Error::Corrupted {
#[cfg(feature = "_test")]
Expand Down Expand Up @@ -476,7 +472,7 @@ pub async fn is_page_empty<S: NorFlash>(
cache: &mut impl PrivateCacheImpl,
page_index: usize,
page_state: Option<PageState>,
) -> Result<bool, Error< S::Error>> {
) -> Result<bool, Error<S::Error>> {
let page_state = match page_state {
Some(page_state) => page_state,
None => get_page_state::<S>(flash, flash_range.clone(), cache, page_index).await?,
Expand Down Expand Up @@ -521,7 +517,7 @@ impl ItemIter {
&mut self,
flash: &mut S,
data_buffer: &'m mut [u8],
) -> Result<Option<(Item<'m>, u32)>, Error< S::Error>> {
) -> Result<Option<(Item<'m>, u32)>, Error<S::Error>> {
let mut data_buffer = Some(data_buffer);
while let (Some(header), address) = self.header.next(flash).await? {
let buffer = data_buffer.take().unwrap();
Expand Down Expand Up @@ -558,7 +554,7 @@ impl ItemHeaderIter {
pub async fn next<S: NorFlash>(
&mut self,
flash: &mut S,
) -> Result<(Option<ItemHeader>, u32), Error< S::Error>> {
) -> Result<(Option<ItemHeader>, u32), Error<S::Error>> {
self.traverse(flash, |_, _| false).await
}

Expand All @@ -570,7 +566,7 @@ impl ItemHeaderIter {
&mut self,
flash: &mut S,
callback: impl Fn(&ItemHeader, u32) -> bool,
) -> Result<(Option<ItemHeader>, u32), Error< S::Error>> {
) -> Result<(Option<ItemHeader>, u32), Error<S::Error>> {
loop {
match ItemHeader::read_new(flash, self.current_address, self.end_address).await {
Ok(Some(header)) => {
Expand Down
12 changes: 6 additions & 6 deletions src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ async fn try_general_repair<S: NorFlash>(
flash: &mut S,
flash_range: Range<u32>,
cache: &mut impl PrivateCacheImpl,
) -> Result<(), Error< S::Error>> {
) -> Result<(), Error<S::Error>> {
// Loop through the pages and get their state. If one returns the corrupted error,
// the page is likely half-erased. Fix for that is to re-erase again to hopefully finish the job.
for page_index in get_pages::<S>(flash_range.clone(), 0) {
Expand All @@ -74,7 +74,7 @@ async fn find_first_page<S: NorFlash>(
cache: &mut impl PrivateCacheImpl,
starting_page_index: usize,
page_state: PageState,
) -> Result<Option<usize>, Error< S::Error>> {
) -> Result<Option<usize>, Error<S::Error>> {
for page_index in get_pages::<S>(flash_range.clone(), starting_page_index) {
if page_state == get_page_state::<S>(flash, flash_range.clone(), cache, page_index).await? {
return Ok(Some(page_index));
Expand Down Expand Up @@ -138,7 +138,7 @@ async fn get_page_state<S: NorFlash>(
flash_range: Range<u32>,
cache: &mut impl PrivateCacheImpl,
page_index: usize,
) -> Result<PageState, Error< S::Error>> {
) -> Result<PageState, Error<S::Error>> {
if let Some(cached_page_state) = cache.get_page_state(page_index) {
return Ok(cached_page_state);
}
Expand Down Expand Up @@ -206,7 +206,7 @@ async fn open_page<S: NorFlash>(
flash_range: Range<u32>,
cache: &mut impl PrivateCacheImpl,
page_index: usize,
) -> Result<(), Error< S::Error>> {
) -> Result<(), Error<S::Error>> {
cache.notice_page_state(page_index, PageState::Open, true);

flash
Expand All @@ -230,7 +230,7 @@ async fn close_page<S: NorFlash>(
flash_range: Range<u32>,
cache: &mut impl PrivateCacheImpl,
page_index: usize,
) -> Result<(), Error< S::Error>> {
) -> Result<(), Error<S::Error>> {
let current_state =
partial_close_page::<S>(flash, flash_range.clone(), cache, page_index).await?;

Expand Down Expand Up @@ -263,7 +263,7 @@ async fn partial_close_page<S: NorFlash>(
flash_range: Range<u32>,
cache: &mut impl PrivateCacheImpl,
page_index: usize,
) -> Result<PageState, Error< S::Error>> {
) -> Result<PageState, Error<S::Error>> {
let current_state = get_page_state::<S>(flash, flash_range.clone(), cache, page_index).await?;

if current_state != PageState::Open {
Expand Down
3 changes: 3 additions & 0 deletions src/map.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -736,6 +736,9 @@ async fn migrate_items<K: Key, S: NorFlash>(
let key = K::deserialize_from(&item.data()[..K::LEN]);
let (_, data_buffer) = item.destruct();

// We're in a decent state here
cache.unmark_dirty();

// Search for the newest item with the key we found
let Some((found_item, found_address)) = fetch_item_with_location::<K, S>(
flash,
Expand Down
Loading

0 comments on commit 342ca4f

Please sign in to comment.