diff --git a/src/fw/applib/persist.h b/src/fw/applib/persist.h index 78c9b5fe2..1ac7f083c 100644 --- a/src/fw/applib/persist.h +++ b/src/fw/applib/persist.h @@ -35,7 +35,7 @@ //! retrieve values from the phone, it provides you with a much faster way to restore state. //! In addition, it draws less power from the battery. //! -//! Note that the size of all persisted values cannot exceed 4K per app. +//! Note that the size of all persisted values cannot exceed 1MB per app. //! @{ //! The maximum size of a persist value in bytes diff --git a/src/fw/services/normal/persist.c b/src/fw/services/normal/persist.c index 31e0abf67..d3d35d359 100644 --- a/src/fw/services/normal/persist.c +++ b/src/fw/services/normal/persist.c @@ -19,7 +19,8 @@ #include "util/list.h" #include "util/units.h" -#define PERSIST_STORAGE_MAX_SPACE KiBYTES(6) +#define PERSIST_STORAGE_MAX_SPACE MiBYTES(1) +#define PERSIST_STORAGE_INITIAL_ALLOC KiBYTES(4) typedef struct PersistStore { ListNode list_node; @@ -124,7 +125,9 @@ SettingsFile * persist_service_lock_and_get_store(const Uuid *uuid) { if (!store->file_open) { char filename[PERSIST_FILE_NAME_MAX_LENGTH]; PBL_ASSERTN(PASSED(prv_get_file_name(filename, sizeof(filename), uuid))); - PBL_ASSERTN(PASSED(settings_file_open(&store->file, filename, PERSIST_STORAGE_MAX_SPACE))); + PBL_ASSERTN(PASSED(settings_file_open_growable(&store->file, filename, + PERSIST_STORAGE_MAX_SPACE, + PERSIST_STORAGE_INITIAL_ALLOC))); store->file_open = true; } return &store->file; diff --git a/src/fw/services/normal/settings/settings_file.c b/src/fw/services/normal/settings/settings_file.c index 2d397a963..19df0b773 100644 --- a/src/fw/services/normal/settings/settings_file.c +++ b/src/fw/services/normal/settings/settings_file.c @@ -25,16 +25,16 @@ static bool file_hdr_is_uninitialized(SettingsFileHeader *file_hdr) { && (file_hdr->flags == 0xffff); } -static status_t prv_open(SettingsFile *file, const char *name, uint8_t flags, int max_used_space) { +static status_t prv_open(SettingsFile *file, const char *name, uint8_t flags, + int max_used_space, int alloc_used_space) { // Making the max_space_total at least a little bit larger than the - // max_used_space allows us to avoid thrashing. Without it, if - // max_space_total == max_used_space, then if the file is full, changing a + // alloc_used_space allows us to avoid thrashing. Without it, if + // max_space_total == alloc_used_space, then if the file is full, changing a // single value would force the whole file to be rewritten- every single // time! It's probably worth it to "waste" a bit of flash space to avoid // this pathalogical case. - int max_space_total = pfs_sector_optimal_size(max_used_space * 12 / 10, strlen(name)); + int max_space_total = pfs_sector_optimal_size(alloc_used_space * 12 / 10, strlen(name)); - // TODO: Dynamically sized files? int fd = pfs_open(name, flags, FILE_TYPE_STATIC, max_space_total); if (fd < 0) { PBL_LOG_ERR("Could not open settings file '%s', %d", name, fd); @@ -50,6 +50,7 @@ static status_t prv_open(SettingsFile *file, const char *name, uint8_t flags, in *file = (SettingsFile) { .name = kernel_strdup_check(name), .max_used_space = max_used_space, + .alloc_used_space = alloc_used_space, .max_space_total = max_space_total, }; @@ -73,7 +74,15 @@ static status_t prv_open(SettingsFile *file, const char *name, uint8_t flags, in PBL_LOG_WRN("Unrecognized version %d for file %s, removing...", file_hdr.version, name); pfs_close_and_remove(fd); - return prv_open(file, name, flags, max_used_space); + return prv_open(file, name, flags, max_used_space, alloc_used_space); + } + + // For growable files, adopt the actual file size before bootup_check so that + // any compaction during recovery uses the correct (grown) allocation size. + int actual_size = pfs_get_file_size(file->iter.fd); + if (alloc_used_space < max_used_space && actual_size > max_space_total) { + file->alloc_used_space = actual_size * 10 / 12; + file->max_space_total = actual_size; } status_t status = bootup_check(file); @@ -81,24 +90,21 @@ static status_t prv_open(SettingsFile *file, const char *name, uint8_t flags, in PBL_LOG_ERR("Bootup check failed (%"PRId32"), not good. " "Attempting to recover by deleting %s...", status, name); pfs_close_and_remove(fd); - return prv_open(file, name, flags, max_used_space); + return prv_open(file, name, flags, max_used_space, alloc_used_space); } // There's a chance that the caller increased the desired size of the settings file since // the file was originally created (i.e. the file was created in an earlier version of the // firmware). If we detect that situation, let's re-write the file to the new larger requested // size. - int actual_size = pfs_get_file_size(file->iter.fd); - if (actual_size < max_space_total) { + if (alloc_used_space >= max_used_space && actual_size < max_space_total) { PBL_LOG_INFO("Re-writing settings file %s to increase its size from %d to %d.", name, actual_size, max_space_total); - // The settings_file_rewrite_filtered call creates a new file based on file->max_used_space - // and copies the contents of the existing file into it. status = settings_file_rewrite_filtered(file, NULL, NULL); if (status < 0) { PBL_LOG_ERR("Could not resize file %s (error %"PRId32"). Creating new one", name, status); - return prv_open(file, name, flags, max_used_space); + return prv_open(file, name, flags, max_used_space, alloc_used_space); } } @@ -109,7 +115,12 @@ static status_t prv_open(SettingsFile *file, const char *name, uint8_t flags, in status_t settings_file_open(SettingsFile *file, const char *name, int max_used_space) { - return prv_open(file, name, OP_FLAG_READ | OP_FLAG_WRITE, max_used_space); + return prv_open(file, name, OP_FLAG_READ | OP_FLAG_WRITE, max_used_space, max_used_space); +} + +status_t settings_file_open_growable(SettingsFile *file, const char *name, + int max_used_space, int initial_alloc_size) { + return prv_open(file, name, OP_FLAG_READ | OP_FLAG_WRITE, max_used_space, initial_alloc_size); } void settings_file_close(SettingsFile *file) { @@ -192,7 +203,7 @@ status_t settings_file_rewrite_filtered( SettingsFile *file, SettingsFileRewriteFilterCallback filter_cb, void *context) { SettingsFile new_file; status_t status = prv_open(&new_file, file->name, OP_FLAG_OVERWRITE | OP_FLAG_READ, - file->max_used_space); + file->max_used_space, file->alloc_used_space); if (status < 0) { PBL_LOG_ERR("Could not open temporary file to compact settings file. Error %"PRIi32".", status); @@ -246,8 +257,10 @@ status_t settings_file_rewrite_filtered( // old file. After the close suceeds, we will end up reading the new // (compacted) file. char *name = kernel_strdup(new_file.name); + int alloc_used_space = new_file.alloc_used_space; settings_file_close(&new_file); - status = prv_open(file, name, OP_FLAG_READ | OP_FLAG_WRITE, file->max_used_space); + status = prv_open(file, name, OP_FLAG_READ | OP_FLAG_WRITE, + file->max_used_space, alloc_used_space); kernel_free(name); return status; } @@ -405,6 +418,27 @@ status_t settings_file_set_byte(SettingsFile *file, const void *key, return S_SUCCESS; } +static status_t prv_grow(SettingsFile *file, int needed_used_space) { + int new_alloc = file->alloc_used_space; + while (new_alloc < needed_used_space && new_alloc < file->max_used_space) { + new_alloc *= 2; + } + if (new_alloc > file->max_used_space) { + new_alloc = file->max_used_space; + } + if (new_alloc < needed_used_space) { + return E_OUT_OF_STORAGE; + } + + int old_alloc = file->alloc_used_space; + file->alloc_used_space = new_alloc; + status_t status = settings_file_rewrite_filtered(file, NULL, NULL); + if (status < 0) { + file->alloc_used_space = old_alloc; + } + return status; +} + // Internal implementation that takes a timestamp parameter // Note that this operation is designed to be atomic from the perspective of // an outside observer. That is, either the new value will be completely @@ -428,7 +462,14 @@ static status_t prv_settings_file_set_internal(SettingsFile *file, const void *k return E_OUT_OF_STORAGE; } if (file->used_space + file->dead_space + rec_size > file->max_space_total) { - status_t status = settings_file_compact(file); + bool needs_growth = (file->used_space + rec_size > file->max_space_total) && + (file->alloc_used_space < file->max_used_space); + status_t status; + if (needs_growth) { + status = prv_grow(file, file->used_space + rec_size); + } else { + status = settings_file_compact(file); + } if (status < 0) { return status; } @@ -597,7 +638,7 @@ status_t settings_file_rewrite(SettingsFile *file, SettingsFile new_file; status_t status = prv_open(&new_file, file->name, OP_FLAG_OVERWRITE | OP_FLAG_READ, - file->max_used_space); + file->max_used_space, file->alloc_used_space); if (status < 0) { return status; } @@ -613,8 +654,10 @@ status_t settings_file_rewrite(SettingsFile *file, // old file. After the close suceeds, we will end up reading the new // (compacted) file. char *name = kernel_strdup(new_file.name); + int alloc_used_space = new_file.alloc_used_space; settings_file_close(&new_file); - status = prv_open(file, name, OP_FLAG_READ | OP_FLAG_WRITE, file->max_used_space); + status = prv_open(file, name, OP_FLAG_READ | OP_FLAG_WRITE, + file->max_used_space, alloc_used_space); kernel_free(name); return status; diff --git a/src/fw/services/normal/settings/settings_file.h b/src/fw/services/normal/settings/settings_file.h index 34c8d3d37..6a29c4bf8 100644 --- a/src/fw/services/normal/settings/settings_file.h +++ b/src/fw/services/normal/settings/settings_file.h @@ -41,6 +41,11 @@ typedef struct SettingsFile { //! fail. int max_used_space; + //! The current allocation budget for physical file size. For growable files, + //! this starts at a small initial value and grows toward max_used_space on + //! demand. For non-growable files, this equals max_used_space. + int alloc_used_space; + //! Amount of space in the settings_file that is currently dead, i.e. //! has been written to with some data, but that data is no longer valid. //! (overwritten records get added to this) @@ -69,6 +74,8 @@ typedef struct SettingsFile { //! ignored. We could change this if the need arises. status_t settings_file_open(SettingsFile *file, const char *name, int max_used_space); +status_t settings_file_open_growable(SettingsFile *file, const char *name, + int max_used_space, int initial_alloc_size); void settings_file_close(SettingsFile *file); bool settings_file_exists(SettingsFile *file, const void *key, size_t key_len); diff --git a/tests/fakes/fake_settings_file.c b/tests/fakes/fake_settings_file.c index 017f5b1d5..46b8e476c 100644 --- a/tests/fakes/fake_settings_file.c +++ b/tests/fakes/fake_settings_file.c @@ -103,6 +103,11 @@ status_t settings_file_open(SettingsFile *file, const char *name, } } +status_t settings_file_open_growable(SettingsFile *file, const char *name, + int max_used_space, int initial_alloc_size) { + return settings_file_open(file, name, max_used_space); +} + void settings_file_close(SettingsFile *file) { cl_assert(s_settings_file.open); s_settings_file.open = false; diff --git a/tests/fw/services/settings/test_settings_file.c b/tests/fw/services/settings/test_settings_file.c index 38d14de2b..457e1df1e 100644 --- a/tests/fw/services/settings/test_settings_file.c +++ b/tests/fw/services/settings/test_settings_file.c @@ -611,6 +611,139 @@ void test_settings_file__reallocate_larger(void) { verify(&file, key, key_len, val, val_len); } +// Test that a growable file automatically grows when writes exceed the initial allocation +// but stay within the enforcement cap. +void test_settings_file__growable_auto_growth(void) { + printf("\nTesting growable file auto-growth...\n"); + + // PFS allocates in 4096-byte pages, so the initial_alloc must span at least + // one page. Use an initial_alloc that results in one PFS page and a max_cap + // that allows multiple pages. Write enough large records to exceed one page. + const int initial_alloc = 2048; + const int max_cap = 32768; + SettingsFile file; + cl_must_pass(settings_file_open_growable(&file, "tg", max_cap, initial_alloc)); + + int initial_file_size = pfs_get_file_size(file.iter.fd); + printf("Initial file size: %d, max_space_total: %d\n", + initial_file_size, file.max_space_total); + + // Write large records to fill and exceed the initial allocation. + // Each record: 8 (header) + 4 (key) + 128 (val) = 140 bytes. + // One PFS page holds ~4000 usable bytes, so ~28 records fills it. + uint8_t key[5]; + int key_len = 4; + uint8_t val[128]; + int val_len = sizeof(val); + memset(val, 0xAB, val_len); + // 8114 / 140 ≈ 57 records to fill the initial allocation. Write 65 to force growth. + int num_records = 65; + for (int i = 0; i < num_records; i++) { + snprintf((char *)key, sizeof(key), "k%03d", i); + val[0] = (uint8_t)i; // Make each value unique + cl_must_pass(settings_file_set(&file, key, key_len, val, val_len)); + } + + int grown_file_size = pfs_get_file_size(file.iter.fd); + printf("Grown file size: %d\n", grown_file_size); + cl_assert(grown_file_size > initial_file_size); + + // Verify all data survived the growth + for (int i = 0; i < num_records; i++) { + snprintf((char *)key, sizeof(key), "k%03d", i); + val[0] = (uint8_t)i; + verify(&file, key, key_len, val, val_len); + } + + settings_file_close(&file); +} + +// Test that a growable file returns E_OUT_OF_STORAGE when the enforcement cap is hit. +void test_settings_file__growable_enforces_cap(void) { + printf("\nTesting growable file cap enforcement...\n"); + + // Use a cap of 4096 (one PFS page worth) and initial_alloc of 2048. + // Write large records to hit the cap. + const int initial_alloc = 2048; + const int max_cap = 4096; + SettingsFile file; + cl_must_pass(settings_file_open_growable(&file, "tc", max_cap, initial_alloc)); + + uint8_t key[5]; + int key_len = 4; + uint8_t val[128]; + int val_len = sizeof(val); + memset(val, 0xCD, val_len); + // Each record = 8 + 4 + 128 = 140 bytes. max_cap of 4096 holds ~28 records. + int i; + for (i = 0; i < 100; i++) { + snprintf((char *)key, sizeof(key), "k%03d", i); + val[0] = (uint8_t)i; + status_t status = settings_file_set(&file, key, key_len, val, val_len); + if (status == E_OUT_OF_STORAGE) { + printf("Hit storage cap at record %d\n", i); + break; + } + cl_must_pass(status); + } + + // We should have hit the cap before writing 100 records + cl_assert(i < 100); + + // Verify the records that were written are still readable + for (int j = 0; j < i; j++) { + snprintf((char *)key, sizeof(key), "k%03d", j); + val[0] = (uint8_t)j; + verify(&file, key, key_len, val, val_len); + } + + settings_file_close(&file); +} + +// Test that closing and reopening a grown file preserves its size and data. +void test_settings_file__growable_reopen(void) { + printf("\nTesting growable file reopen after growth...\n"); + + const int initial_alloc = 2048; + const int max_cap = 32768; + SettingsFile file; + cl_must_pass(settings_file_open_growable(&file, "tr", max_cap, initial_alloc)); + + // Write enough large records to force growth past the initial page + uint8_t key[5]; + int key_len = 4; + uint8_t val[128]; + int val_len = sizeof(val); + memset(val, 0xEF, val_len); + int num_records = 65; + for (int i = 0; i < num_records; i++) { + snprintf((char *)key, sizeof(key), "k%03d", i); + val[0] = (uint8_t)i; + cl_must_pass(settings_file_set(&file, key, key_len, val, val_len)); + } + + int grown_file_size = pfs_get_file_size(file.iter.fd); + printf("Grown file size: %d\n", grown_file_size); + settings_file_close(&file); + + // Reopen with the same growable parameters + cl_must_pass(settings_file_open_growable(&file, "tr", max_cap, initial_alloc)); + + int reopened_file_size = pfs_get_file_size(file.iter.fd); + printf("Reopened file size: %d\n", reopened_file_size); + // The file should not have shrunk + cl_assert(reopened_file_size >= grown_file_size); + + // Verify all data survived the close/reopen + for (int i = 0; i < num_records; i++) { + snprintf((char *)key, sizeof(key), "k%03d", i); + val[0] = (uint8_t)i; + verify(&file, key, key_len, val, val_len); + } + + settings_file_close(&file); +} + // Test that we can start searching beginning at the record we previously found in a recent // call into settings file. This makes sure we don't start searching at the beginning of a file // each API call. diff --git a/tests/stubs/stubs_settings_file.h b/tests/stubs/stubs_settings_file.h index c194018af..1e7f97dbe 100644 --- a/tests/stubs/stubs_settings_file.h +++ b/tests/stubs/stubs_settings_file.h @@ -9,6 +9,11 @@ status_t settings_file_open(SettingsFile *file, const char *name, int max_used_s return S_SUCCESS; } +status_t settings_file_open_growable(SettingsFile *file, const char *name, + int max_used_space, int initial_alloc_size) { + return settings_file_open(file, name, max_used_space); +} + status_t settings_file_get(SettingsFile *file, const void *key, size_t key_len, void *val_out, size_t val_out_len) { return S_SUCCESS;