Skip to content

Commit eae1680

Browse files
rghaddabcarlescufi
authored andcommitted
[nrf fromlist] settings: zms: add more robustness to the save function
When a power off happens after writing the settings name and before writing the linked list node we cannot write the settings name again in the future. Fix this by writing the linked list node before writing the name. When loading all settings, we already delete linked list node that do not have any name or value written. Adds as well a recover path if a power down happens in the middle of unlinking an LL node after a delete. Upstream PR #: 87792 Signed-off-by: Riadh Ghaddab <[email protected]> (cherry picked from commit bf4d15316b1a3bd9239ddbe179b11006eea1e3b1) (cherry picked from commit d9daa11)
1 parent 8ad3232 commit eae1680

File tree

2 files changed

+66
-36
lines changed

2 files changed

+66
-36
lines changed

subsys/settings/include/settings/settings_zms.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ extern "C" {
6363

6464
/* some useful macros */
6565
#define ZMS_NAME_ID_FROM_LL_NODE(x) (x & ~BIT(0))
66+
#define ZMS_LL_NODE_FROM_NAME_ID(x) (x | BIT(0))
6667
#define ZMS_UPDATE_COLLISION_NUM(x, y) \
6768
((x & ~ZMS_COLLISIONS_MASK) | ((y << 1) & ZMS_COLLISIONS_MASK))
6869
#define ZMS_COLLISION_NUM(x) ((x & ZMS_COLLISIONS_MASK) >> 1)

subsys/settings/src/settings_zms.c

Lines changed: 65 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -72,11 +72,39 @@ static int settings_zms_unlink_ll_node(struct settings_zms *cf, uint32_t name_ha
7272
struct settings_hash_linked_list settings_update_element;
7373

7474
/* let's update the linked list */
75-
rc = zms_read(&cf->cf_zms, name_hash | 1, &settings_element,
75+
rc = zms_read(&cf->cf_zms, ZMS_LL_NODE_FROM_NAME_ID(name_hash), &settings_element,
7676
sizeof(struct settings_hash_linked_list));
7777
if (rc < 0) {
7878
return rc;
7979
}
80+
81+
/* update the previous element */
82+
if (settings_element.previous_hash) {
83+
rc = zms_read(&cf->cf_zms, settings_element.previous_hash, &settings_update_element,
84+
sizeof(struct settings_hash_linked_list));
85+
if (rc < 0) {
86+
return rc;
87+
}
88+
if (!settings_element.next_hash) {
89+
/* we are deleting the last element of the linked list,
90+
* let's update the second_to_last_hash_id
91+
*/
92+
cf->second_to_last_hash_id = settings_update_element.previous_hash;
93+
}
94+
settings_update_element.next_hash = settings_element.next_hash;
95+
rc = zms_write(&cf->cf_zms, settings_element.previous_hash,
96+
&settings_update_element, sizeof(struct settings_hash_linked_list));
97+
if (rc < 0) {
98+
return rc;
99+
}
100+
}
101+
102+
/* Now delete the current linked list element */
103+
rc = zms_delete(&cf->cf_zms, ZMS_LL_NODE_FROM_NAME_ID(name_hash));
104+
if (rc < 0) {
105+
return rc;
106+
}
107+
80108
/* update the next element */
81109
if (settings_element.next_hash) {
82110
rc = zms_read(&cf->cf_zms, settings_element.next_hash, &settings_update_element,
@@ -100,28 +128,8 @@ static int settings_zms_unlink_ll_node(struct settings_zms *cf, uint32_t name_ha
100128
*/
101129
cf->last_hash_id = settings_element.previous_hash;
102130
}
103-
/* update the previous element */
104-
if (settings_element.previous_hash) {
105-
rc = zms_read(&cf->cf_zms, settings_element.previous_hash, &settings_update_element,
106-
sizeof(struct settings_hash_linked_list));
107-
if (rc < 0) {
108-
return rc;
109-
}
110-
if (!settings_element.next_hash) {
111-
/* we are deleting the last element of the linked list,
112-
* let's update the second_to_last_hash_id
113-
*/
114-
cf->second_to_last_hash_id = settings_update_element.previous_hash;
115-
}
116-
settings_update_element.next_hash = settings_element.next_hash;
117-
rc = zms_write(&cf->cf_zms, settings_element.previous_hash,
118-
&settings_update_element, sizeof(struct settings_hash_linked_list));
119-
if (rc < 0) {
120-
return rc;
121-
}
122-
}
123131

124-
return rc;
132+
return 0;
125133
}
126134
#endif /* CONFIG_SETTINGS_ZMS_NO_LL_DELETE */
127135

@@ -146,12 +154,6 @@ static int settings_zms_delete(struct settings_zms *cf, uint32_t name_hash)
146154
return rc;
147155
}
148156

149-
/* Now delete the current linked list element */
150-
rc = zms_delete(&cf->cf_zms, name_hash | 1);
151-
if (rc < 0) {
152-
return rc;
153-
}
154-
155157
#endif /* CONFIG_SETTINGS_ZMS_NO_LL_DELETE */
156158
return rc;
157159
}
@@ -468,13 +470,10 @@ static int settings_zms_save(struct settings_store *cs, const char *name, const
468470

469471
/* write the name if required */
470472
if (write_name) {
471-
rc = zms_write(&cf->cf_zms, name_hash, name, strlen(name));
472-
if (rc < 0) {
473-
return rc;
474-
}
473+
/* First let's update the linked list */
475474
#ifdef CONFIG_SETTINGS_ZMS_NO_LL_DELETE
476475
/* verify that the ll_node doesn't exist otherwise do not update it */
477-
rc = zms_read(&cf->cf_zms, name_hash | 1, &settings_element,
476+
rc = zms_read(&cf->cf_zms, ZMS_LL_NODE_FROM_NAME_ID(name_hash), &settings_element,
478477
sizeof(struct settings_hash_linked_list));
479478
if (rc >= 0) {
480479
goto no_ll_update;
@@ -496,28 +495,36 @@ static int settings_zms_save(struct settings_store *cs, const char *name, const
496495
}
497496
}
498497
settings_element.previous_hash = cf->last_hash_id;
499-
rc = zms_write(&cf->cf_zms, name_hash | 1, &settings_element,
498+
rc = zms_write(&cf->cf_zms, ZMS_LL_NODE_FROM_NAME_ID(name_hash), &settings_element,
500499
sizeof(struct settings_hash_linked_list));
501500
if (rc < 0) {
502501
return rc;
503502
}
504503

505504
/* Now update the previous linked list element */
506-
settings_element.next_hash = name_hash | 1;
505+
settings_element.next_hash = ZMS_LL_NODE_FROM_NAME_ID(name_hash);
507506
settings_element.previous_hash = cf->second_to_last_hash_id;
508507
rc = zms_write(&cf->cf_zms, cf->last_hash_id, &settings_element,
509508
sizeof(struct settings_hash_linked_list));
510509
if (rc < 0) {
511510
return rc;
512511
}
513512
cf->second_to_last_hash_id = cf->last_hash_id;
514-
cf->last_hash_id = name_hash | 1;
513+
cf->last_hash_id = ZMS_LL_NODE_FROM_NAME_ID(name_hash);
515514
#ifdef CONFIG_SETTINGS_ZMS_LL_CACHE
516515
if (cf->ll_cache_next < CONFIG_SETTINGS_ZMS_LL_CACHE_SIZE) {
517516
cf->ll_cache[cf->ll_cache_next] = settings_element;
518517
cf->ll_cache_next = cf->ll_cache_next + 1;
519518
}
520519
#endif
520+
#ifdef CONFIG_SETTINGS_ZMS_NO_LL_DELETE
521+
no_ll_update:
522+
#endif /* CONFIG_SETTINGS_ZMS_NO_LL_DELETE */
523+
/* Now let's write the name */
524+
rc = zms_write(&cf->cf_zms, name_hash, name, strlen(name));
525+
if (rc < 0) {
526+
return rc;
527+
}
521528
}
522529
#ifdef CONFIG_SETTINGS_ZMS_NO_LL_DELETE
523530
no_ll_update:
@@ -529,6 +536,7 @@ static int settings_zms_get_last_hash_ids(struct settings_zms *cf)
529536
{
530537
struct settings_hash_linked_list settings_element;
531538
uint32_t ll_last_hash_id = ZMS_LL_HEAD_HASH_ID;
539+
uint32_t previous_ll_hash_id = 0;
532540
int rc = 0;
533541

534542
#ifdef CONFIG_SETTINGS_ZMS_LL_CACHE
@@ -554,6 +562,27 @@ static int settings_zms_get_last_hash_ids(struct settings_zms *cf)
554562
return rc;
555563
}
556564

565+
if (settings_element.previous_hash != previous_ll_hash_id) {
566+
/* This is a special case that can happen when a power down occurred
567+
* when deleting a linked list node.
568+
* If the power down occurred after updating the previous linked list node,
569+
* then we would end up with a state where the previous_hash of the linked
570+
* list is broken. Let's recover from this
571+
*/
572+
rc = zms_delete(&cf->cf_zms, settings_element.previous_hash);
573+
if (rc < 0) {
574+
return rc;
575+
}
576+
/* Now recover the linked list */
577+
settings_element.previous_hash = previous_ll_hash_id;
578+
zms_write(&cf->cf_zms, ll_last_hash_id, &settings_element,
579+
sizeof(struct settings_hash_linked_list));
580+
if (rc < 0) {
581+
return rc;
582+
}
583+
}
584+
previous_ll_hash_id = ll_last_hash_id;
585+
557586
#ifdef CONFIG_SETTINGS_ZMS_LL_CACHE
558587
if ((cf->ll_cache_next < CONFIG_SETTINGS_ZMS_LL_CACHE_SIZE) &&
559588
(settings_element.next_hash)) {

0 commit comments

Comments
 (0)