[nrf fromlist] nvs: implement NVS cache

The NVS data lookup time grows linearly with the number of
allocation table entries to walk through, meaning that if
some data pair in the NVS changes frequently, access to
other data pairs that change rarely can take a lot of time.

It is particularly visible when the NVS is used as the
settings backend since the backend needs to perform multiple
NVS reads to find a requested key.

Implement a simple cache that stores an address of the most
recent ATE for all NVS IDs that fall into the given cache
position. CRC8/16 is used as a hash function used to
distribute NVS IDs across the cache entries.

The cache entries are only invalidated when an NVS sector
is erased as part of the garbage collector task.

PR: zephyrproject-rtos/zephyr#45769

Signed-off-by: Damian Krolik <[email protected]>

Author: Damian-Nordic

include/zephyr/fs/nvs.h

@@ -55,6 +55,9 @@ struct nvs_fs {
 	struct k_mutex nvs_lock;
 	const struct device *flash_device;
 	const struct flash_parameters *flash_parameters;
+#if CONFIG_NVS_LOOKUP_CACHE
+	uint32_t lookup_cache[CONFIG_NVS_LOOKUP_CACHE_SIZE];
+#endif
 };
 
 /**

subsys/fs/nvs/Kconfig

@@ -10,6 +10,22 @@ config NVS
 
 if NVS
 
+config NVS_LOOKUP_CACHE
+	bool "Non-volatile Storage lookup cache"
+	help
+	  Enable Non-volatile Storage cache, used to reduce the NVS data lookup
+	  time. Each cache entry holds an address of the most recent allocation
+	  table entry (ATE) for all NVS IDs that fall into that cache position.
+
+config NVS_LOOKUP_CACHE_SIZE
+	int "Non-volatile Storage lookup cache size"
+	default 128
+	range 1 65536
+	depends on NVS_LOOKUP_CACHE
+	help
+	  Number of entries in Non-volatile Storage lookup cache.
+	  It is recommended that it be a power of 2.
+
 module = NVS
 module-str = nvs
 source "subsys/logging/Kconfig.template.log_config"

subsys/fs/nvs/nvs.c

@@ -16,6 +16,78 @@
 #include <logging/log.h>
 LOG_MODULE_REGISTER(fs_nvs, CONFIG_NVS_LOG_LEVEL);
 
+static int nvs_prev_ate(struct nvs_fs *fs, uint32_t *addr, struct nvs_ate *ate);
+static int nvs_ate_valid(struct nvs_fs *fs, const struct nvs_ate *entry);
+
+#ifdef CONFIG_NVS_LOOKUP_CACHE
+
+static inline size_t nvs_lookup_cache_pos(uint16_t id)
+{
+	size_t pos;
+
+#if CONFIG_NVS_LOOKUP_CACHE_SIZE <= UINT8_MAX
+	/*
+	 * CRC8-CCITT is used for ATE checksums and it also acts well as a hash
+	 * function, so it can be a good choice from the code size perspective.
+	 * However, other hash functions can be used as well if proved better
+	 * performance.
+	 */
+	pos = crc8_ccitt(CRC8_CCITT_INITIAL_VALUE, &id, sizeof(id));
+#else
+	pos = crc16_ccitt(0xffff, (const uint8_t *)&id, sizeof(id));
+#endif
+
+	return pos % CONFIG_NVS_LOOKUP_CACHE_SIZE;
+}
+
+static int nvs_lookup_cache_rebuild(struct nvs_fs *fs)
+{
+	int rc;
+	uint32_t addr, ate_addr;
+	uint32_t *cache_entry;
+	struct nvs_ate ate;
+
+	memset(fs->lookup_cache, 0xff, sizeof(fs->lookup_cache));
+	addr = fs->ate_wra;
+
+	while (true) {
+		/* Make a copy of 'addr' as it will be advanced by nvs_pref_ate() */
+		ate_addr = addr;
+		rc = nvs_prev_ate(fs, &addr, &ate);
+
+		if (rc) {
+			return rc;
+		}
+
+		cache_entry = &fs->lookup_cache[nvs_lookup_cache_pos(ate.id)];
+
+		if (ate.id != 0xFFFF && *cache_entry == NVS_LOOKUP_CACHE_NO_ADDR &&
+		    nvs_ate_valid(fs, &ate)) {
+			*cache_entry = ate_addr;
+		}
+
+		if (addr == fs->ate_wra) {
+			break;
+		}
+	}
+
+	return 0;
+}
+
+static void nvs_lookup_cache_invalidate(struct nvs_fs *fs, uint32_t sector)
+{
+	uint32_t *cache_entry = fs->lookup_cache;
+	uint32_t *const cache_end = &fs->lookup_cache[CONFIG_NVS_LOOKUP_CACHE_SIZE];
+
+	for (; cache_entry < cache_end; ++cache_entry) {
+		if ((*cache_entry >> ADDR_SECT_SHIFT) == sector) {
+			*cache_entry = NVS_LOOKUP_CACHE_NO_ADDR;
+		}
+	}
+}
+
+#endif /* CONFIG_NVS_LOOKUP_CACHE */
+
 /* basic routines */
 /* nvs_al_size returns size aligned to fs->write_block_size */
 static inline size_t nvs_al_size(struct nvs_fs *fs, size_t len)
@@ -96,6 +168,12 @@ static int nvs_flash_ate_wrt(struct nvs_fs *fs, const struct nvs_ate *entry)
 
 	rc = nvs_flash_al_wrt(fs, fs->ate_wra, entry,
 			       sizeof(struct nvs_ate));
+#ifdef CONFIG_NVS_LOOKUP_CACHE
+	/* 0xFFFF is a special-purpose identifier. Exclude it from the cache */
+	if (entry->id != 0xFFFF) {
+		fs->lookup_cache[nvs_lookup_cache_pos(entry->id)] = fs->ate_wra;
+	}
+#endif
 	fs->ate_wra -= nvs_al_size(fs, sizeof(struct nvs_ate));
 
 	return rc;
@@ -225,6 +303,10 @@ static int nvs_flash_erase_sector(struct nvs_fs *fs, uint32_t addr)
 
 	LOG_DBG("Erasing flash at %lx, len %d", (long int) offset,
 		fs->sector_size);
+
+#ifdef CONFIG_NVS_LOOKUP_CACHE
+	nvs_lookup_cache_invalidate(fs, addr >> ADDR_SECT_SHIFT);
+#endif
 	rc = flash_erase(fs->flash_device, offset, fs->sector_size);
 
 	if (rc) {
@@ -810,6 +892,10 @@ static int nvs_startup(struct nvs_fs *fs)
 		fs->data_wra = fs->ate_wra & ADDR_SECT_MASK;
 	}
 
+#ifdef CONFIG_NVS_LOOKUP_CACHE
+	rc = nvs_lookup_cache_rebuild(fs);
+#endif
+
 end:
 	/* If the sector is empty add a gc done ate to avoid having insufficient
 	 * space when doing gc.
@@ -1051,7 +1137,16 @@ ssize_t nvs_read_hist(struct nvs_fs *fs, uint16_t id, void *data, size_t len,
 
 	cnt_his = 0U;
 
+#ifdef CONFIG_NVS_LOOKUP_CACHE
+	wlk_addr = fs->lookup_cache[nvs_lookup_cache_pos(id)];
+
+	if (wlk_addr == NVS_LOOKUP_CACHE_NO_ADDR) {
+		rc = -ENOENT;
+		goto err;
+	}
+#else
 	wlk_addr = fs->ate_wra;
+#endif
 	rd_addr = wlk_addr;
 
 	while (cnt_his <= cnt) {

subsys/fs/nvs/nvs_priv.h

@@ -28,6 +28,8 @@ extern "C" {
 
 #define NVS_BLOCK_SIZE 32
 
+#define NVS_LOOKUP_CACHE_NO_ADDR 0xFFFFFFFF
+
 /* Allocation Table Entry */
 struct nvs_ate {
 	uint16_t id;	/* data id */

tests/subsys/fs/nvs/boards/native_posix.overlay

@@ -0,0 +1,9 @@
+/*
+ * Copyright (c) 2022 Nordic Semiconductor ASA
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+&flash0 {
+	erase-block-size = <0x400>;
+};

tests/subsys/fs/nvs/src/main.c

@@ -8,11 +8,11 @@
  * This test is designed to be run using flash-simulator which provide
  * functionality for flash property customization and emulating errors in
  * flash operation in parallel to regular flash API.
- * Test should be run on qemu_x86 target.
+ * Test should be run on qemu_x86 or native_posix target.
  */
 
-#ifndef CONFIG_BOARD_QEMU_X86
-#error "Run on qemu_x86 only"
+#if !defined(CONFIG_BOARD_QEMU_X86) && !defined(CONFIG_BOARD_NATIVE_POSIX)
+#error "Run on qemu_x86 or native_posix only"
 #endif
 
 #include <stdio.h>
@@ -41,7 +41,7 @@ void setup(void)
 	sim_thresholds = stats_group_find("flash_sim_thresholds");
 
 	/* Verify if NVS is initialized. */
-	if (fs.sector_count != 0) {
+	if (fs.ready) {
 		int err;
 
 		err = nvs_clear(&fs);
@@ -700,6 +700,146 @@ void test_nvs_gc_corrupt_ate(void)
 	zassert_true(err == 0,  "nvs_mount call failure: %d", err);
 }
 
+#ifdef CONFIG_NVS_LOOKUP_CACHE
+static size_t num_matching_cache_entries(uint32_t addr, bool compare_sector_only)
+{
+	size_t i, num = 0;
+	uint32_t mask = compare_sector_only ? ADDR_SECT_MASK : UINT32_MAX;
+
+	for (i = 0; i < CONFIG_NVS_LOOKUP_CACHE_SIZE; i++) {
+		if ((fs.lookup_cache[i] & mask) == addr) {
+			num++;
+		}
+	}
+
+	return num;
+}
+#endif
+
+/*
+ * Test that NVS lookup cache is properly rebuilt on nvs_mount(), or initialized
+ * to NVS_LOOKUP_CACHE_NO_ADDR if the store is empty.
+ */
+void test_nvs_cache_init(void)
+{
+#ifdef CONFIG_NVS_LOOKUP_CACHE
+	int err;
+	size_t num;
+	uint32_t ate_addr;
+	uint8_t data = 0;
+
+	/* Test cache initialization when the store is empty */
+
+	fs.sector_count = 3;
+	err = nvs_mount(&fs);
+	zassert_true(err == 0, "nvs_init call failure: %d", err);
+
+	num = num_matching_cache_entries(NVS_LOOKUP_CACHE_NO_ADDR, false);
+	zassert_equal(num, CONFIG_NVS_LOOKUP_CACHE_SIZE, "uninitialized cache");
+
+	/* Test cache update after nvs_write() */
+
+	ate_addr = fs.ate_wra;
+	err = nvs_write(&fs, 1, &data, sizeof(data));
+	zassert_equal(err, sizeof(data), "nvs_write call failure: %d", err);
+
+	num = num_matching_cache_entries(NVS_LOOKUP_CACHE_NO_ADDR, false);
+	zassert_equal(num, CONFIG_NVS_LOOKUP_CACHE_SIZE - 1, "cache not updated after write");
+
+	num = num_matching_cache_entries(ate_addr, false);
+	zassert_equal(num, 1, "invalid cache entry after write");
+
+	/* Test cache initialization when the store is non-empty */
+
+	memset(fs.lookup_cache, 0xAA, sizeof(fs.lookup_cache));
+	err = nvs_mount(&fs);
+	zassert_true(err == 0, "nvs_init call failure: %d", err);
+
+	num = num_matching_cache_entries(NVS_LOOKUP_CACHE_NO_ADDR, false);
+	zassert_equal(num, CONFIG_NVS_LOOKUP_CACHE_SIZE - 1, "uninitialized cache after restart");
+
+	num = num_matching_cache_entries(ate_addr, false);
+	zassert_equal(num, 1, "invalid cache entry after restart");
+#endif
+}
+
+/*
+ * Test that even after writing more NVS IDs than the number of NVS lookup cache
+ * entries they all can be read correctly.
+ */
+void test_nvs_cache_collission(void)
+{
+#ifdef CONFIG_NVS_LOOKUP_CACHE
+	int err;
+	uint16_t id;
+	uint16_t data;
+
+	fs.sector_count = 3;
+	err = nvs_mount(&fs);
+	zassert_true(err == 0, "nvs_init call failure: %d", err);
+
+	for (id = 0; id < CONFIG_NVS_LOOKUP_CACHE_SIZE + 1; id++) {
+		data = id;
+		err = nvs_write(&fs, id, &data, sizeof(data));
+		zassert_equal(err, sizeof(data), "nvs_write call failure: %d", err);
+	}
+
+	for (id = 0; id < CONFIG_NVS_LOOKUP_CACHE_SIZE + 1; id++) {
+		err = nvs_read(&fs, id, &data, sizeof(data));
+		zassert_equal(err, sizeof(data), "nvs_read call failure: %d", err);
+		zassert_equal(data, id, "incorrect data read");
+	}
+#endif
+}
+
+/*
+ * Test that NVS lookup cache does not contain any address from gc-ed sector
+ */
+void test_nvs_cache_gc(void)
+{
+#ifdef CONFIG_NVS_LOOKUP_CACHE
+	int err;
+	size_t num;
+	uint16_t data = 0;
+
+	fs.sector_count = 3;
+	err = nvs_mount(&fs);
+	zassert_true(err == 0, "nvs_init call failure: %d", err);
+
+	/* Fill the first sector with writes of ID 1 */
+
+	while (fs.data_wra + sizeof(data) <= fs.ate_wra) {
+		++data;
+		err = nvs_write(&fs, 1, &data, sizeof(data));
+		zassert_equal(err, sizeof(data), "nvs_write call failure: %d", err);
+	}
+
+	/* Verify that cache contains a single entry for sector 0 */
+
+	num = num_matching_cache_entries(0 << ADDR_SECT_SHIFT, true);
+	zassert_equal(num, 1, "invalid cache content after filling sector 0");
+
+	/* Fill the second sector with writes of ID 2 */
+
+	while ((fs.ate_wra >> ADDR_SECT_SHIFT) != 2) {
+		++data;
+		err = nvs_write(&fs, 2, &data, sizeof(data));
+		zassert_equal(err, sizeof(data), "nvs_write call failure: %d", err);
+	}
+
+	/*
+	 * At this point sector 0 should have been gc-ed. Verify that action is
+	 * reflected by the cache content.
+	 */
+
+	num = num_matching_cache_entries(0 << ADDR_SECT_SHIFT, true);
+	zassert_equal(num, 0, "not invalidated cache entries aftetr gc");
+
+	num = num_matching_cache_entries(2 << ADDR_SECT_SHIFT, true);
+	zassert_equal(num, 2, "invalid cache content after gc");
+#endif
+}
+
 void test_main(void)
 {
 	__ASSERT_NO_MSG(device_is_ready(flash_dev));
@@ -725,7 +865,13 @@ void test_main(void)
 			 ztest_unit_test_setup_teardown(
 				 test_nvs_gc_corrupt_close_ate, setup, teardown),
 			 ztest_unit_test_setup_teardown(
-				 test_nvs_gc_corrupt_ate, setup, teardown)
+				 test_nvs_gc_corrupt_ate, setup, teardown),
+			 ztest_unit_test_setup_teardown(
+				 test_nvs_cache_init, setup, teardown),
+			 ztest_unit_test_setup_teardown(
+				 test_nvs_cache_collission, setup, teardown),
+			 ztest_unit_test_setup_teardown(
+				 test_nvs_cache_gc, setup, teardown)
 			);
 
 	ztest_run_test_suite(test_nvs);

tests/subsys/fs/nvs/testcase.yaml

@@ -6,3 +6,6 @@ tests:
   filesystem.nvs_0x00:
     extra_args: DTC_OVERLAY_FILE=boards/qemu_x86_ev_0x00.overlay
     platform_allow: qemu_x86
+  filesystem.nvs_cache:
+    extra_args: CONFIG_NVS_LOOKUP_CACHE=y CONFIG_NVS_LOOKUP_CACHE_SIZE=64
+    platform_allow: native_posix