Skip to content

Commit 4dd30c1

Browse files
gekygeky
authored
Merge pull request #948 from littlefs-project/fix-sync-ordering
Fix sync issue where data writes could appear before metadata writes
2 parents 5c0d332 + 7873d81 commit 4dd30c1

File tree

10 files changed

+163
-13
lines changed

10 files changed

+163
-13
lines changed

‎bd/lfs_emubd.c‎

Lines changed: 101 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,8 @@ int lfs_emubd_create(const struct lfs_config *cfg,
129129
bd->proged = 0;
130130
bd->erased = 0;
131131
bd->power_cycles = bd->cfg->power_cycles;
132+
bd->ooo_block = -1;
133+
bd->ooo_data = NULL;
132134
bd->disk = NULL;
133135

134136
if (bd->cfg->disk_path) {
@@ -195,6 +197,7 @@ int lfs_emubd_destroy(const struct lfs_config *cfg) {
195197
free(bd->blocks);
196198

197199
// clean up other resources
200+
lfs_emubd_decblock(bd->ooo_data);
198201
if (bd->disk) {
199202
bd->disk->rc -= 1;
200203
if (bd->disk->rc == 0) {
@@ -209,6 +212,75 @@ int lfs_emubd_destroy(const struct lfs_config *cfg) {
209212
}
210213

211214

215+
// powerloss hook
216+
static int lfs_emubd_powerloss(const struct lfs_config *cfg) {
217+
lfs_emubd_t *bd = cfg->context;
218+
219+
// emulate out-of-order writes?
220+
lfs_emubd_block_t *ooo_data = NULL;
221+
if (bd->cfg->powerloss_behavior == LFS_EMUBD_POWERLOSS_OOO
222+
&& bd->ooo_block != -1) {
223+
// since writes between syncs are allowed to be out-of-order, it
224+
// shouldn't hurt to restore the first write on powerloss, right?
225+
ooo_data = bd->blocks[bd->ooo_block];
226+
bd->blocks[bd->ooo_block] = lfs_emubd_incblock(bd->ooo_data);
227+
228+
// mirror to disk file?
229+
if (bd->disk
230+
&& (bd->blocks[bd->ooo_block]
231+
|| bd->cfg->erase_value != -1)) {
232+
off_t res1 = lseek(bd->disk->fd,
233+
(off_t)bd->ooo_block*bd->cfg->erase_size,
234+
SEEK_SET);
235+
if (res1 < 0) {
236+
return -errno;
237+
}
238+
239+
ssize_t res2 = write(bd->disk->fd,
240+
(bd->blocks[bd->ooo_block])
241+
? bd->blocks[bd->ooo_block]->data
242+
: bd->disk->scratch,
243+
bd->cfg->erase_size);
244+
if (res2 < 0) {
245+
return -errno;
246+
}
247+
}
248+
}
249+
250+
// simulate power loss
251+
bd->cfg->powerloss_cb(bd->cfg->powerloss_data);
252+
253+
// if we continue, undo out-of-order write emulation
254+
if (bd->cfg->powerloss_behavior == LFS_EMUBD_POWERLOSS_OOO
255+
&& bd->ooo_block != -1) {
256+
lfs_emubd_decblock(bd->blocks[bd->ooo_block]);
257+
bd->blocks[bd->ooo_block] = ooo_data;
258+
259+
// mirror to disk file?
260+
if (bd->disk
261+
&& (bd->blocks[bd->ooo_block]
262+
|| bd->cfg->erase_value != -1)) {
263+
off_t res1 = lseek(bd->disk->fd,
264+
(off_t)bd->ooo_block*bd->cfg->erase_size,
265+
SEEK_SET);
266+
if (res1 < 0) {
267+
return -errno;
268+
}
269+
270+
ssize_t res2 = write(bd->disk->fd,
271+
(bd->blocks[bd->ooo_block])
272+
? bd->blocks[bd->ooo_block]->data
273+
: bd->disk->scratch,
274+
bd->cfg->erase_size);
275+
if (res2 < 0) {
276+
return -errno;
277+
}
278+
}
279+
}
280+
281+
return 0;
282+
}
283+
212284

213285
// block device API
214286

@@ -344,8 +416,11 @@ int lfs_emubd_prog(const struct lfs_config *cfg, lfs_block_t block,
344416
if (bd->power_cycles > 0) {
345417
bd->power_cycles -= 1;
346418
if (bd->power_cycles == 0) {
347-
// simulate power loss
348-
bd->cfg->powerloss_cb(bd->cfg->powerloss_data);
419+
int err = lfs_emubd_powerloss(cfg);
420+
if (err) {
421+
LFS_EMUBD_TRACE("lfs_emubd_prog -> %d", err);
422+
return err;
423+
}
349424
}
350425
}
351426

@@ -361,10 +436,17 @@ int lfs_emubd_erase(const struct lfs_config *cfg, lfs_block_t block) {
361436
// check if erase is valid
362437
LFS_ASSERT(block < bd->cfg->erase_count);
363438

439+
// emulate out-of-order writes? save first write
440+
if (bd->cfg->powerloss_behavior == LFS_EMUBD_POWERLOSS_OOO
441+
&& bd->ooo_block == -1) {
442+
bd->ooo_block = block;
443+
bd->ooo_data = lfs_emubd_incblock(bd->blocks[block]);
444+
}
445+
364446
// get the block
365447
lfs_emubd_block_t *b = lfs_emubd_mutblock(cfg, &bd->blocks[block]);
366448
if (!b) {
367-
LFS_EMUBD_TRACE("lfs_emubd_prog -> %d", LFS_ERR_NOMEM);
449+
LFS_EMUBD_TRACE("lfs_emubd_erase -> %d", LFS_ERR_NOMEM);
368450
return LFS_ERR_NOMEM;
369451
}
370452

@@ -430,8 +512,11 @@ int lfs_emubd_erase(const struct lfs_config *cfg, lfs_block_t block) {
430512
if (bd->power_cycles > 0) {
431513
bd->power_cycles -= 1;
432514
if (bd->power_cycles == 0) {
433-
// simulate power loss
434-
bd->cfg->powerloss_cb(bd->cfg->powerloss_data);
515+
int err = lfs_emubd_powerloss(cfg);
516+
if (err) {
517+
LFS_EMUBD_TRACE("lfs_emubd_erase -> %d", err);
518+
return err;
519+
}
435520
}
436521
}
437522

@@ -441,14 +526,21 @@ int lfs_emubd_erase(const struct lfs_config *cfg, lfs_block_t block) {
441526

442527
int lfs_emubd_sync(const struct lfs_config *cfg) {
443528
LFS_EMUBD_TRACE("lfs_emubd_sync(%p)", (void*)cfg);
529+
lfs_emubd_t *bd = cfg->context;
444530

445-
// do nothing
446-
(void)cfg;
531+
// emulate out-of-order writes? reset first write, writes
532+
// cannot be out-of-order across sync
533+
if (bd->cfg->powerloss_behavior == LFS_EMUBD_POWERLOSS_OOO) {
534+
lfs_emubd_decblock(bd->ooo_data);
535+
bd->ooo_block = -1;
536+
bd->ooo_data = NULL;
537+
}
447538

448539
LFS_EMUBD_TRACE("lfs_emubd_sync -> %d", 0);
449540
return 0;
450541
}
451542

543+
452544
/// Additional extended API for driving test features ///
453545

454546
static int lfs_emubd_crc_(const struct lfs_config *cfg,
@@ -633,6 +725,8 @@ int lfs_emubd_copy(const struct lfs_config *cfg, lfs_emubd_t *copy) {
633725
copy->proged = bd->proged;
634726
copy->erased = bd->erased;
635727
copy->power_cycles = bd->power_cycles;
728+
copy->ooo_block = bd->ooo_block;
729+
copy->ooo_data = lfs_emubd_incblock(bd->ooo_data);
636730
copy->disk = bd->disk;
637731
if (copy->disk) {
638732
copy->disk->rc += 1;

‎bd/lfs_emubd.h‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,17 +36,18 @@ extern "C"
3636
// Not that read-noop is not allowed. Read _must_ return a consistent (but
3737
// may be arbitrary) value on every read.
3838
typedef enum lfs_emubd_badblock_behavior {
39-
LFS_EMUBD_BADBLOCK_PROGERROR,
40-
LFS_EMUBD_BADBLOCK_ERASEERROR,
41-
LFS_EMUBD_BADBLOCK_READERROR,
42-
LFS_EMUBD_BADBLOCK_PROGNOOP,
43-
LFS_EMUBD_BADBLOCK_ERASENOOP,
39+
LFS_EMUBD_BADBLOCK_PROGERROR = 0, // Error on prog
40+
LFS_EMUBD_BADBLOCK_ERASEERROR = 1, // Error on erase
41+
LFS_EMUBD_BADBLOCK_READERROR = 2, // Error on read
42+
LFS_EMUBD_BADBLOCK_PROGNOOP = 3, // Prog does nothing silently
43+
LFS_EMUBD_BADBLOCK_ERASENOOP = 4, // Erase does nothing silently
4444
} lfs_emubd_badblock_behavior_t;
4545

4646
// Mode determining how power-loss behaves during testing. For now this
4747
// only supports a noop behavior, leaving the data on-disk untouched.
4848
typedef enum lfs_emubd_powerloss_behavior {
49-
LFS_EMUBD_POWERLOSS_NOOP,
49+
LFS_EMUBD_POWERLOSS_NOOP = 0, // Progs are atomic
50+
LFS_EMUBD_POWERLOSS_OOO = 1, // Blocks are written out-of-order
5051
} lfs_emubd_powerloss_behavior_t;
5152

5253
// Type for measuring read/program/erase operations
@@ -152,6 +153,8 @@ typedef struct lfs_emubd {
152153
lfs_emubd_io_t proged;
153154
lfs_emubd_io_t erased;
154155
lfs_emubd_powercycles_t power_cycles;
156+
lfs_ssize_t ooo_block;
157+
lfs_emubd_block_t *ooo_data;
155158
lfs_emubd_disk_t *disk;
156159

157160
const struct lfs_emubd_config *cfg;

‎lfs.c‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3404,6 +3404,15 @@ static int lfs_file_sync_(lfs_t *lfs, lfs_file_t *file) {
34043404

34053405
if ((file->flags & LFS_F_DIRTY) &&
34063406
!lfs_pair_isnull(file->m.pair)) {
3407+
// before we commit metadata, we need sync the disk to make sure
3408+
// data writes don't complete after metadata writes
3409+
if (!(file->flags & LFS_F_INLINE)) {
3410+
err = lfs_bd_sync(lfs, &lfs->pcache, &lfs->rcache, false);
3411+
if (err) {
3412+
return err;
3413+
}
3414+
}
3415+
34073416
// update dir entry
34083417
uint16_t type;
34093418
const void *buffer;

‎tests/test_dirs.toml‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,10 @@ code = '''
181181
defines.N = [5, 11]
182182
if = 'BLOCK_COUNT >= 4*N'
183183
reentrant = true
184+
defines.POWERLOSS_BEHAVIOR = [
185+
'LFS_EMUBD_POWERLOSS_NOOP',
186+
'LFS_EMUBD_POWERLOSS_OOO',
187+
]
184188
code = '''
185189
lfs_t lfs;
186190
int err = lfs_mount(&lfs, cfg);
@@ -439,6 +443,10 @@ code = '''
439443
defines.N = [5, 25]
440444
if = 'N < BLOCK_COUNT/2'
441445
reentrant = true
446+
defines.POWERLOSS_BEHAVIOR = [
447+
'LFS_EMUBD_POWERLOSS_NOOP',
448+
'LFS_EMUBD_POWERLOSS_OOO',
449+
]
442450
code = '''
443451
lfs_t lfs;
444452
int err = lfs_mount(&lfs, cfg);

‎tests/test_files.toml‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,10 @@ defines.SIZE = [32, 0, 7, 2049]
310310
defines.CHUNKSIZE = [31, 16, 65]
311311
defines.INLINE_MAX = [0, -1, 8]
312312
reentrant = true
313+
defines.POWERLOSS_BEHAVIOR = [
314+
'LFS_EMUBD_POWERLOSS_NOOP',
315+
'LFS_EMUBD_POWERLOSS_OOO',
316+
]
313317
code = '''
314318
lfs_t lfs;
315319
int err = lfs_mount(&lfs, cfg);
@@ -500,6 +504,10 @@ code = '''
500504
[cases.test_files_many_power_loss]
501505
defines.N = 300
502506
reentrant = true
507+
defines.POWERLOSS_BEHAVIOR = [
508+
'LFS_EMUBD_POWERLOSS_NOOP',
509+
'LFS_EMUBD_POWERLOSS_OOO',
510+
]
503511
code = '''
504512
lfs_t lfs;
505513
int err = lfs_mount(&lfs, cfg);

‎tests/test_interspersed.toml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,10 @@ code = '''
195195
defines.SIZE = [10, 100]
196196
defines.FILES = [4, 10, 26]
197197
reentrant = true
198+
defines.POWERLOSS_BEHAVIOR = [
199+
'LFS_EMUBD_POWERLOSS_NOOP',
200+
'LFS_EMUBD_POWERLOSS_OOO',
201+
]
198202
code = '''
199203
lfs_t lfs;
200204
lfs_file_t files[FILES];

‎tests/test_move.toml‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,10 @@ code = '''
357357

358358
[cases.test_move_reentrant_file]
359359
reentrant = true
360+
defines.POWERLOSS_BEHAVIOR = [
361+
'LFS_EMUBD_POWERLOSS_NOOP',
362+
'LFS_EMUBD_POWERLOSS_OOO',
363+
]
360364
code = '''
361365
lfs_t lfs;
362366
int err = lfs_mount(&lfs, cfg);
@@ -839,6 +843,10 @@ code = '''
839843

840844
[cases.test_reentrant_dir]
841845
reentrant = true
846+
defines.POWERLOSS_BEHAVIOR = [
847+
'LFS_EMUBD_POWERLOSS_NOOP',
848+
'LFS_EMUBD_POWERLOSS_OOO',
849+
]
842850
code = '''
843851
lfs_t lfs;
844852
int err = lfs_mount(&lfs, cfg);

‎tests/test_seek.toml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -329,6 +329,10 @@ code = '''
329329
# must be power-of-2 for quadratic probing to be exhaustive
330330
defines.COUNT = [4, 64, 128]
331331
reentrant = true
332+
defines.POWERLOSS_BEHAVIOR = [
333+
'LFS_EMUBD_POWERLOSS_NOOP',
334+
'LFS_EMUBD_POWERLOSS_OOO',
335+
]
332336
code = '''
333337
lfs_t lfs;
334338
int err = lfs_mount(&lfs, cfg);

‎tests/test_superblocks.toml‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,10 @@ code = '''
3232
# reentrant format
3333
[cases.test_superblocks_reentrant_format]
3434
reentrant = true
35+
defines.POWERLOSS_BEHAVIOR = [
36+
'LFS_EMUBD_POWERLOSS_NOOP',
37+
'LFS_EMUBD_POWERLOSS_OOO',
38+
]
3539
code = '''
3640
lfs_t lfs;
3741
int err = lfs_mount(&lfs, cfg);
@@ -174,6 +178,10 @@ code = '''
174178
defines.BLOCK_CYCLES = [2, 1]
175179
defines.N = 24
176180
reentrant = true
181+
defines.POWERLOSS_BEHAVIOR = [
182+
'LFS_EMUBD_POWERLOSS_NOOP',
183+
'LFS_EMUBD_POWERLOSS_OOO',
184+
]
177185
code = '''
178186
lfs_t lfs;
179187
int err = lfs_mount(&lfs, cfg);

‎tests/test_truncate.toml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,10 @@ defines.SMALLSIZE = [4, 512]
231231
defines.MEDIUMSIZE = [0, 3, 4, 5, 31, 32, 33, 511, 512, 513, 1023, 1024, 1025]
232232
defines.LARGESIZE = 2048
233233
reentrant = true
234+
defines.POWERLOSS_BEHAVIOR = [
235+
'LFS_EMUBD_POWERLOSS_NOOP',
236+
'LFS_EMUBD_POWERLOSS_OOO',
237+
]
234238
code = '''
235239
lfs_t lfs;
236240
int err = lfs_mount(&lfs, cfg);

0 commit comments

Comments
 (0)