Skip to content

Commit 1a05f1e

Browse files
jplexerclaude
andcommitted
fw/services/settings: speed up file rewrite, pause watchdog
settings_file_rewrite_filtered runs under a held mutex while issuing flash erases and writes that, on a 1 MiB persist file, can take many seconds. Once the per-app cap was raised the rewrite started tripping the 5-second task watchdog and triggering App Throttling. FIRM-1649 flagged this; it didn't matter at the old 6 KiB cap. Three changes: - Pause the task watchdog for the duration of the rewrite (60 s budget, comfortably covers a worst-case 1 MiB rewrite). Same pattern already used during boot in main.c. - Add settings_raw_iter_read_key_val / write_key_val helpers that read or write the key and value as a single contiguous PFS call. Drops per-record I/O from 6 PFS round-trips (separate header, key, val) to 4, which on a file with thousands of records saves a few hundred ms of seek/read state-machine overhead. - Allocate one reusable buffer outside the rewrite loop instead of two kernel_malloc/kernel_free pairs per record. Flash erase and program time still dominate, so this doesn't make small rewrites feel different, but the watchdog pause keeps big rewrites from triggering throttle/recovery storms in the logs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Joshua Jun <lets@throw.rocks>
1 parent 48f15c0 commit 1a05f1e

4 files changed

Lines changed: 48 additions & 9 deletions

File tree

include/pbl/services/settings/settings_raw_iter.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,11 +112,20 @@ int settings_raw_iter_get_resumed_record_pos(SettingsRawIter *iter);
112112
void settings_raw_iter_read_key(SettingsRawIter *iter, uint8_t *key);
113113
void settings_raw_iter_read_val(SettingsRawIter *iter, uint8_t *val, int val_len);
114114

115+
//! Read key and value into a single contiguous buffer in one PFS call.
116+
//! key_val_out must be at least key_len + val_len bytes. Key occupies the first
117+
//! key_len bytes, val the next val_len bytes.
118+
void settings_raw_iter_read_key_val(SettingsRawIter *iter, uint8_t *key_val_out);
119+
115120
//! Write (over top of) the header/key/val for the current record.
116121
void settings_raw_iter_write_header(SettingsRawIter *iter, SettingsRecordHeader *hdr);
117122
void settings_raw_iter_write_key(SettingsRawIter *iter, const uint8_t *key);
118123
void settings_raw_iter_write_val(SettingsRawIter *iter, const uint8_t *val);
119124

125+
//! Write key and value from a single contiguous buffer in one PFS call.
126+
//! Layout matches settings_raw_iter_read_key_val.
127+
void settings_raw_iter_write_key_val(SettingsRawIter *iter, const uint8_t *key_val);
128+
120129
//! Write a byte in place for the current record
121130
void settings_raw_iter_write_byte(SettingsRawIter *iter, int offset, uint8_t byte);
122131

src/fw/services/settings/settings_file.c

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#include "pbl/services/settings/settings_raw_iter.h"
66

77
#include "drivers/rtc.h"
8+
#include "drivers/task_watchdog.h"
89
#include "kernel/pbl_malloc.h"
910
#include "pbl/services/filesystem/pfs.h"
1011
#include "system/logging.h"
@@ -214,18 +215,28 @@ status_t settings_file_rewrite_filtered(
214215
PBL_LOG_INFO("FIRM-1649: settings_file_rewrite_filtered start file=%s",
215216
file->name);
216217

218+
// A 1 MiB grow can take many seconds of pure flash erase + write time. Pause
219+
// the task watchdog rather than letting it trip and kick App Throttling.
220+
task_watchdog_pause(60);
221+
217222
SettingsFile new_file;
218223
status_t status = prv_open(&new_file, file->name, OP_FLAG_OVERWRITE | OP_FLAG_READ,
219224
file->max_used_space, file->alloc_used_space,
220225
file->min_alloc_used_space);
221226
if (status < 0) {
222227
PBL_LOG_ERR("Could not open temporary file to compact settings file. Error %"PRIi32".",
223228
status);
229+
task_watchdog_resume();
224230
return status;
225231
}
226232

227233
settings_raw_iter_begin(&new_file.iter);
228234

235+
// One reusable buffer for key+val per record; sized for the worst case.
236+
// Avoids two malloc/free pairs per record over what can be thousands of
237+
// records on a large persist file.
238+
uint8_t *kv_buf = kernel_malloc(SETTINGS_KEY_MAX_LEN + SETTINGS_VAL_MAX_LEN);
239+
229240
for (settings_raw_iter_begin(&file->iter); !settings_raw_iter_end(&file->iter);
230241
settings_raw_iter_next(&file->iter)) {
231242
SettingsRecordHeader *hdr = &file->iter.hdr;
@@ -249,22 +260,19 @@ status_t settings_file_rewrite_filtered(
249260
clear_flag(hdr, SETTINGS_FLAG_OVERWRITE_STARTED);
250261
}
251262

252-
// Get the old key and value
253-
uint8_t *key = kernel_malloc(hdr->key_len);
254-
settings_raw_iter_read_key(&file->iter, key);
255-
uint8_t *val = kernel_malloc(hdr->val_len);
256-
settings_raw_iter_read_val(&file->iter, val, hdr->val_len);
263+
// Read key+val in a single PFS call; key occupies the first key_len bytes.
264+
settings_raw_iter_read_key_val(&file->iter, kv_buf);
265+
uint8_t *key = kv_buf;
266+
uint8_t *val = kv_buf + hdr->key_len;
257267

258268
// Include in re-written file if it passes the filter
259269
if (!filter_cb || filter_cb(key, hdr->key_len, val, hdr->val_len, context)) {
260270
settings_raw_iter_write_header(&new_file.iter, hdr);
261-
settings_raw_iter_write_key(&new_file.iter, key);
262-
settings_raw_iter_write_val(&new_file.iter, val);
271+
settings_raw_iter_write_key_val(&new_file.iter, kv_buf);
263272
settings_raw_iter_next(&new_file.iter);
264273
}
265-
kernel_free(key);
266-
kernel_free(val);
267274
}
275+
kernel_free(kv_buf);
268276
settings_file_close(file);
269277
// We have to close and reopen the new_file so that it's temp flag is cleared.
270278
// Before the close succeeds, if we reboot, we will just end up reading the
@@ -278,6 +286,8 @@ status_t settings_file_rewrite_filtered(
278286
file->max_used_space, alloc_used_space, min_alloc_used_space);
279287
kernel_free(name);
280288

289+
task_watchdog_resume();
290+
281291
// FIRM-1649: instrumentation. See note at the top of this function.
282292
const uint32_t rewrite_elapsed_ms =
283293
(uint32_t)(((rtc_get_ticks() - rewrite_start_ticks) * 1000) / RTC_TICKS_HZ);

src/fw/services/settings/settings_raw_iter.c

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,13 @@ void settings_raw_iter_read_val(SettingsRawIter *iter, uint8_t *val_out, int val
186186
sfs_read(iter, val_out, val_len);
187187
}
188188

189+
void settings_raw_iter_read_key_val(SettingsRawIter *iter, uint8_t *key_val_out) {
190+
const int kv_len = iter->hdr.key_len + iter->hdr.val_len;
191+
if (kv_len == 0) return;
192+
sfs_seek(iter, iter->hdr_pos + sizeof(SettingsRecordHeader), FSeekSet);
193+
sfs_read(iter, key_val_out, kv_len);
194+
}
195+
189196
void settings_raw_iter_write_header(SettingsRawIter *iter, SettingsRecordHeader *hdr) {
190197
PBL_ASSERTN(hdr->key_len <= SETTINGS_KEY_MAX_LEN);
191198
PBL_ASSERTN(hdr->val_len <= SETTINGS_VAL_MAX_LEN);
@@ -203,6 +210,13 @@ void settings_raw_iter_write_val(SettingsRawIter *iter, const uint8_t *val) {
203210
sfs_seek(iter, iter->hdr_pos + sizeof(SettingsRecordHeader) + iter->hdr.key_len, FSeekSet);
204211
sfs_write(iter, val, iter->hdr.val_len);
205212
}
213+
214+
void settings_raw_iter_write_key_val(SettingsRawIter *iter, const uint8_t *key_val) {
215+
const int kv_len = iter->hdr.key_len + iter->hdr.val_len;
216+
if (kv_len == 0) return;
217+
sfs_seek(iter, iter->hdr_pos + sizeof(SettingsRecordHeader), FSeekSet);
218+
sfs_write(iter, key_val, kv_len);
219+
}
206220
void settings_raw_iter_write_byte(SettingsRawIter *iter, int offset, uint8_t byte) {
207221
sfs_seek(iter, iter->hdr_pos +
208222
sizeof(SettingsRecordHeader) + iter->hdr.key_len + offset, FSeekSet);

tests/stubs/stubs_task_watchdog.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,3 +18,9 @@ void task_watchdog_feed(void) {
1818

1919
void task_watchdog_bit_set(PebbleTask task) {
2020
}
21+
22+
void task_watchdog_pause(unsigned int seconds) {
23+
}
24+
25+
void task_watchdog_resume(void) {
26+
}

0 commit comments

Comments
 (0)