Skip to content

Commit 72f6f6c

Browse files
author
David Saada
committed
TDBStore: Perform garbage collection on failed writes
Partial writes may turn storage unusable. GC clears this scenario.
1 parent 70956ee commit 72f6f6c

File tree

2 files changed

+38
-5
lines changed

2 files changed

+38
-5
lines changed

‎features/storage/TESTS/kvstore/general_tests_phase_1/main.cpp

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -258,9 +258,7 @@ static void set_several_keys_multithreaded()
258258
for (i = 0; i < num_of_threads; i++) {
259259
res = kvstore->get(keys[i], buffer, buffer_size, &actual_size, 0);
260260
TEST_ASSERT_EQUAL_ERROR_CODE(MBED_SUCCESS, res);
261-
262261
TEST_ASSERT_EQUAL_STRING_LEN(buffer, data, data_size);
263-
264262
}
265263

266264
for (i = 0; i < num_of_threads; i++) {
@@ -329,6 +327,7 @@ static void set_key_value_two_byte_size()
329327
TEST_ASSERT_EQUAL_ERROR_CODE(MBED_SUCCESS, res);
330328

331329
res = kvstore->get(key, buffer, buffer_size, &actual_size, 0);
330+
TEST_ASSERT_EQUAL_ERROR_CODE(MBED_SUCCESS, res);
332331
TEST_ASSERT_EQUAL_STRING_LEN(buffer, data_two, 1);
333332
memset(buffer, 0, buffer_size);
334333

@@ -346,6 +345,7 @@ static void set_key_value_five_byte_size()
346345
TEST_ASSERT_EQUAL_ERROR_CODE(MBED_SUCCESS, res);
347346

348347
res = kvstore->get(key, buffer, buffer_size, &actual_size, 0);
348+
TEST_ASSERT_EQUAL_ERROR_CODE(MBED_SUCCESS, res);
349349
TEST_ASSERT_EQUAL_STRING_LEN(buffer, data_five, 4);
350350
memset(buffer, 0, buffer_size);
351351

@@ -363,6 +363,7 @@ static void set_key_value_fifteen_byte_size()
363363
TEST_ASSERT_EQUAL_ERROR_CODE(MBED_SUCCESS, res);
364364

365365
res = kvstore->get(key, buffer, buffer_size, &actual_size, 0);
366+
TEST_ASSERT_EQUAL_ERROR_CODE(MBED_SUCCESS, res);
366367
TEST_ASSERT_EQUAL_STRING_LEN(buffer, data_fif, 14);
367368
memset(buffer, 0, buffer_size);
368369

@@ -380,6 +381,7 @@ static void set_key_value_seventeen_byte_size()
380381
TEST_ASSERT_EQUAL_ERROR_CODE(MBED_SUCCESS, res);
381382

382383
res = kvstore->get(key, buffer, buffer_size, &actual_size, 0);
384+
TEST_ASSERT_EQUAL_ERROR_CODE(MBED_SUCCESS, res);
383385
TEST_ASSERT_EQUAL_STRING_LEN(buffer, data_fif, 16);
384386
memset(buffer, 0, buffer_size);
385387

@@ -451,7 +453,8 @@ static void set_key_init_deinit()
451453
TEST_ASSERT_EQUAL_ERROR_CODE(MBED_SUCCESS, res);
452454

453455
res = kvstore->get(key, buffer, buffer_size, &actual_size, 0);
454-
TEST_ASSERT_EQUAL_STRING(buffer, data);
456+
TEST_ASSERT_EQUAL_ERROR_CODE(MBED_SUCCESS, res);
457+
TEST_ASSERT_EQUAL_STRING(data, buffer);
455458
memset(buffer, 0, buffer_size);
456459

457460
res = kvstore->remove(key);

‎features/storage/kvstore/tdbstore/TDBStore.cpp

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -390,6 +390,7 @@ int TDBStore::set_start(set_handle_t *handle, const char *key, size_t final_data
390390
uint32_t offset;
391391
uint32_t hash, ram_table_ind;
392392
inc_set_handle_t *ih;
393+
bool need_gc = false;
393394

394395
if (!is_valid_key(key)) {
395396
return MBED_ERROR_INVALID_ARGUMENT;
@@ -485,14 +486,19 @@ int TDBStore::set_start(set_handle_t *handle, const char *key, size_t final_data
485486
// Write key now
486487
ret = write_area(_active_area, ih->bd_curr_offset, ih->header.key_size, key);
487488
if (ret) {
489+
need_gc = true;
488490
goto fail;
489491
}
490492
ih->bd_curr_offset += ih->header.key_size;
491493
goto end;
492494

493495
fail:
496+
if ((need_gc) && (ih->bd_base_offset != _master_record_offset)) {
497+
garbage_collection();
498+
}
494499
// mark handle as invalid by clearing magic field in header
495500
ih->header.magic = 0;
501+
496502
_mutex.unlock();
497503

498504
end:
@@ -503,6 +509,7 @@ int TDBStore::set_add_data(set_handle_t handle, const void *value_data, size_t d
503509
{
504510
int ret = MBED_SUCCESS;
505511
inc_set_handle_t *ih;
512+
bool need_gc = false;
506513

507514
if (handle != _inc_set_handle) {
508515
return MBED_ERROR_INVALID_ARGUMENT;
@@ -532,12 +539,17 @@ int TDBStore::set_add_data(set_handle_t handle, const void *value_data, size_t d
532539
// Write the data chunk
533540
ret = write_area(_active_area, ih->bd_curr_offset, data_size, value_data);
534541
if (ret) {
542+
need_gc = true;
535543
goto end;
536544
}
537545
ih->bd_curr_offset += data_size;
538546
ih->offset_in_data += data_size;
539547

540548
end:
549+
if ((need_gc) && (ih->bd_base_offset != _master_record_offset)) {
550+
garbage_collection();
551+
}
552+
541553
_inc_set_mutex.unlock();
542554
return ret;
543555
}
@@ -548,6 +560,8 @@ int TDBStore::set_finalize(set_handle_t handle)
548560
inc_set_handle_t *ih;
549561
ram_table_entry_t *ram_table = (ram_table_entry_t *) _ram_table;
550562
ram_table_entry_t *entry;
563+
bool need_gc = false;
564+
uint32_t actual_data_size, hash, flags, next_offset;
551565

552566
if (handle != _inc_set_handle) {
553567
return MBED_ERROR_INVALID_ARGUMENT;
@@ -563,21 +577,22 @@ int TDBStore::set_finalize(set_handle_t handle)
563577

564578
if (ih->offset_in_data != ih->header.data_size) {
565579
ret = MBED_ERROR_INVALID_SIZE;
566-
// Need GC as otherwise our storage is left in a non-usable state
567-
garbage_collection();
580+
need_gc = true;
568581
goto end;
569582
}
570583

571584
// Write header
572585
ret = write_area(_active_area, ih->bd_base_offset, sizeof(record_header_t), &ih->header);
573586
if (ret) {
587+
need_gc = true;
574588
goto end;
575589
}
576590

577591
// Need to flush buffered BD as our record is totally written now
578592
os_ret = _buff_bd->sync();
579593
if (os_ret) {
580594
ret = MBED_ERROR_WRITE_FAILED;
595+
need_gc = true;
581596
goto end;
582597
}
583598

@@ -586,6 +601,16 @@ int TDBStore::set_finalize(set_handle_t handle)
586601
goto end;
587602
}
588603

604+
// Writes may fail without returning a failure (specially in flash components). Reread the record
605+
// to ensure write success (this won't read the data anywhere - just use the CRC calculation).
606+
ret = read_record(_active_area, ih->bd_base_offset, 0, 0, (uint32_t) -1,
607+
actual_data_size, 0, false, false, false, false,
608+
hash, flags, next_offset);
609+
if (ret) {
610+
need_gc = true;
611+
goto end;
612+
}
613+
589614
// Update RAM table
590615
if (ih->header.flags & delete_flag) {
591616
_num_keys--;
@@ -611,10 +636,15 @@ int TDBStore::set_finalize(set_handle_t handle)
611636
_free_space_offset = align_up(ih->bd_curr_offset, _prog_size);
612637

613638
end:
639+
if ((need_gc) && (ih->bd_base_offset != _master_record_offset)) {
640+
garbage_collection();
641+
}
642+
614643
// mark handle as invalid by clearing magic field in header
615644
ih->header.magic = 0;
616645

617646
_inc_set_mutex.unlock();
647+
618648
if (ih->bd_base_offset != _master_record_offset) {
619649
_mutex.unlock();
620650
}

0 commit comments

Comments
 (0)