Skip to content

Fix max_keys reset limitation #9054

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

Merged
merged 1 commit into from
Dec 14, 2018
Merged

Fix max_keys reset limitation #9054

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
15 changes: 13 additions & 2 deletions features/storage/nvstore/TESTS/nvstore/functionality/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -449,12 +449,13 @@ static void thread_test_worker()
static void nvstore_multi_thread_test()
{
#ifdef MBED_CONF_RTOS_PRESENT
int i;
int i, result;
uint16_t size;
uint16_t key;
int ret;
char *dummy;
uint16_t max_possible_keys;
uint16_t max_possible_keys, actual_len_bytes = 0;
char test[] = "Test", read_buf[10] = {};

NVStore &nvstore = NVStore::get_instance();

Expand Down Expand Up @@ -483,6 +484,10 @@ static void nvstore_multi_thread_test()
TEST_SKIP_UNLESS_MESSAGE(max_possible_keys >= max_possible_keys_threshold,
"Max possible keys below threshold. Test skipped.");

nvstore.set_max_keys(max_test_keys + 1);
result = nvstore.set(max_test_keys, strlen(test), test);
TEST_ASSERT_EQUAL(NVSTORE_SUCCESS, result);

thr_test_data->stop_threads = false;
for (key = 0; key < thr_test_data->max_keys; key++) {
for (i = 0; i < thr_test_num_buffs; i++) {
Expand Down Expand Up @@ -527,6 +532,12 @@ static void nvstore_multi_thread_test()
for (key = 0; key < thr_test_data->max_keys; key++) {
thread_test_check_key(key);
}

result = nvstore.get(max_test_keys, sizeof(read_buf), read_buf, actual_len_bytes);
TEST_ASSERT_EQUAL(NVSTORE_SUCCESS, result);
TEST_ASSERT_EQUAL(strlen(test), actual_len_bytes);
TEST_ASSERT_EQUAL_UINT8_ARRAY(test, read_buf, strlen(test));

goto clean;

mem_fail:
Expand Down
90 changes: 70 additions & 20 deletions features/storage/nvstore/source/nvstore.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ static const unsigned int owner_bit_pos = 12;

typedef struct {
uint16_t version;
uint16_t reserved1;
uint32_t reserved2;
uint16_t max_keys;
uint32_t reserved;
} master_record_data_t;

static const uint32_t min_area_size = 4096;
Expand Down Expand Up @@ -171,11 +171,54 @@ uint16_t NVStore::get_max_possible_keys()

void NVStore::set_max_keys(uint16_t num_keys)
{
uint16_t key = 0, old_max_keys = 0;

MBED_ASSERT(num_keys < get_max_possible_keys());

if (num_keys < NVSTORE_NUM_PREDEFINED_KEYS) {
return;
}

if (!_init_done) {
int ret = init();
if (ret != NVSTORE_SUCCESS) {
return;
}
}

_mutex->lock();

//check if there are values that might be discarded
if (num_keys < _max_keys) {
for (key = num_keys; key < _max_keys; key++) {
if (_offset_by_key[key] != 0) {
return;
}
}
}

old_max_keys = _max_keys;
_max_keys = num_keys;
// User is allowed to change number of keys. As this affects init, need to deinitialize now.
// Don't call init right away - it is lazily called by get/set functions if needed.
deinit();

// Invoke GC to write new max_keys to master record
garbage_collection(no_key, 0, 0, 0, NULL, std::min(_max_keys, old_max_keys));

// Reallocate _offset_by_key with new size
if (_max_keys != old_max_keys) {
// Reallocate _offset_by_key with new size
uint32_t *old_offset_by_key = (uint32_t *) _offset_by_key;
uint32_t *new_offset_by_key = new uint32_t[_max_keys];
MBED_ASSERT(new_offset_by_key);

// Copy old content to new table
memset(new_offset_by_key, 0, sizeof(uint32_t) * _max_keys);
memcpy(new_offset_by_key, old_offset_by_key, sizeof(uint32_t) * std::min(_max_keys, old_max_keys));

_offset_by_key = new_offset_by_key;
delete[] old_offset_by_key;
}

_mutex->unlock();
}

int NVStore::flash_read_area(uint8_t area, uint32_t offset, uint32_t size, void *buf)
Expand Down Expand Up @@ -444,8 +487,8 @@ int NVStore::write_master_record(uint8_t area, uint16_t version, uint32_t &next_
master_record_data_t master_rec;

master_rec.version = version;
master_rec.reserved1 = 0;
master_rec.reserved2 = 0;
master_rec.max_keys = _max_keys;
master_rec.reserved = 0;
return write_record(area, 0, master_record_key, 0, 0, sizeof(master_rec),
&master_rec, next_offset);
}
Expand Down Expand Up @@ -518,7 +561,7 @@ int NVStore::copy_record(uint8_t from_area, uint32_t from_offset, uint32_t to_of
return NVSTORE_SUCCESS;
}

int NVStore::garbage_collection(uint16_t key, uint16_t flags, uint8_t owner, uint16_t buf_size, const void *buf)
int NVStore::garbage_collection(uint16_t key, uint16_t flags, uint8_t owner, uint16_t buf_size, const void *buf, uint16_t num_keys)
{
uint32_t curr_offset, new_area_offset, next_offset, curr_owner;
int ret;
Expand All @@ -542,7 +585,7 @@ int NVStore::garbage_collection(uint16_t key, uint16_t flags, uint8_t owner, uin

// Now iterate on all types, and copy the ones who have valid offsets (meaning that they exist)
// to the other area.
for (key = 0; key < _max_keys; key++) {
for (key = 0; key < num_keys; key++) {
curr_offset = _offset_by_key[key];
uint16_t save_flags = curr_offset & offs_by_key_flag_mask & ~offs_by_key_area_mask;
curr_area = (uint8_t)(curr_offset >> offs_by_key_area_bit_pos) & 1;
Expand Down Expand Up @@ -579,7 +622,6 @@ int NVStore::garbage_collection(uint16_t key, uint16_t flags, uint8_t owner, uin
return ret;
}


int NVStore::do_get(uint16_t key, uint16_t buf_size, void *buf, uint16_t &actual_size,
int validate_only)
{
Expand Down Expand Up @@ -684,7 +726,7 @@ int NVStore::do_set(uint16_t key, uint16_t buf_size, const void *buf, uint16_t f

// If we cross the area limit, we need to invoke GC.
if (new_free_space >= _size) {
ret = garbage_collection(key, flags, owner, buf_size, buf);
ret = garbage_collection(key, flags, owner, buf_size, buf, _max_keys);
_mutex->unlock();
return ret;
}
Expand Down Expand Up @@ -800,6 +842,7 @@ int NVStore::init()
uint16_t key;
uint16_t flags;
uint16_t versions[NVSTORE_NUM_AREAS];
uint16_t keys[NVSTORE_NUM_AREAS];
uint16_t actual_size;
uint8_t owner;

Expand All @@ -818,13 +861,6 @@ int NVStore::init()
return NVSTORE_SUCCESS;
}

_offset_by_key = new uint32_t[_max_keys];
MBED_ASSERT(_offset_by_key);

for (key = 0; key < _max_keys; key++) {
_offset_by_key[key] = 0;
}

_mutex = new PlatformMutex;
MBED_ASSERT(_mutex);

Expand All @@ -841,10 +877,12 @@ int NVStore::init()

calc_validate_area_params();

//retrieve max keys from master record
for (uint8_t area = 0; area < NVSTORE_NUM_AREAS; area++) {
area_state[area] = NVSTORE_AREA_STATE_NONE;
free_space_offset_of_area[area] = 0;
versions[area] = 0;
keys[area] = 0;

_size = std::min(_size, _flash_area_params[area].size);

Expand Down Expand Up @@ -878,6 +916,7 @@ int NVStore::init()
continue;
}
versions[area] = master_rec.version;
keys[area] = master_rec.max_keys;

// Place _free_space_offset after the master record (for the traversal,
// which takes place after this loop).
Expand All @@ -888,6 +927,17 @@ int NVStore::init()
// that we found our active area.
_active_area = area;
_active_area_version = versions[area];
if (!keys[area]) {
keys[area] = NVSTORE_NUM_PREDEFINED_KEYS;
}
_max_keys = keys[area];
}

_offset_by_key = new uint32_t[_max_keys];
MBED_ASSERT(_offset_by_key);

for (key = 0; key < _max_keys; key++) {
_offset_by_key[key] = 0;
}

// In case we have two empty areas, arbitrarily assign 0 to the active one.
Expand Down Expand Up @@ -920,9 +970,9 @@ int NVStore::init()
MBED_ASSERT(ret == NVSTORE_SUCCESS);

// In case we have a faulty record, this probably means that the system crashed when written.
// Perform a garbage collection, to make the the other area valid.
// Perform a garbage collection, to make the other area valid.
if (!valid) {
ret = garbage_collection(no_key, 0, 0, 0, NULL);
ret = garbage_collection(no_key, 0, 0, 0, NULL, _max_keys);
break;
}
if (flags & delete_item_flag) {
Expand Down
4 changes: 2 additions & 2 deletions features/storage/nvstore/source/nvstore.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,6 @@ class NVStore : private mbed::NonCopyable<NVStore> {
*/
int get_area_params(uint8_t area, uint32_t &address, size_t &size);


private:
typedef struct {
uint32_t address;
Expand Down Expand Up @@ -417,10 +416,11 @@ class NVStore : private mbed::NonCopyable<NVStore> {
* @param[in] owner Owner.
* @param[in] buf_size Data size (bytes).
* @param[in] buf Data buffer.
* @param[in] num_keys number of keys.
*
* @returns 0 for success, nonzero for failure.
*/
int garbage_collection(uint16_t key, uint16_t flags, uint8_t owner, uint16_t buf_size, const void *buf);
int garbage_collection(uint16_t key, uint16_t flags, uint8_t owner, uint16_t buf_size, const void *buf, uint16_t num_keys);

/**
* @brief Actual logics of get API (covers also get size API).
Expand Down