Resuming a cleared map

[htherrien]

Resuming a cleared and synced map does not seem to produce the expected behavior.
I use the pattern (clear the map, then sync it) to perform a quick format of the flash device.
After playing with the simulator, it looks like there is no reset of the journal size cookie after a call to dhara_map_clear followed by dhara_map_sync. This implies that if there is a power failure after the call to dhara_map_sync, a dhara_map_resume will produce a map having the wrong number of allocated sectors. I found 3 solutions to this problem:

1. Perform a full format of the device by erasing all blocks. This option is costly.
2. Do a dummy write to any sector between map clear and sync. The resumed map will have 1 sector allocated.
3. Set the journal size cookie when calling dhara_map_clear

The third option is my preferred solution:

 void dhara_map_clear(struct dhara_map *m)
 {
 	if (m->count) {
 		m->count = 0;
+		ck_set_count(dhara_journal_cookie(&m->journal), m->count);
 		dhara_journal_clear(&m->journal);
 	}
 }

[dlbeer]

On Mon, Sep 16, 2019 at 02:44:46PM -0700, htherrien wrote: Resuming a cleared and synced map does not seem to produce the expected behavior. I use the pattern (clear the map then sync it) to perform a quick format of the flash device. After playing with the simulator, it looks like there is no reset of the journal size cookie after a call of `dhara_map_clear` followed by `dhara_map_sync`. This implies that if there is a power failure after the call to `dhara_map_sync`, a `dhara_map_resume` will produce a map having the wrong number of allocated sectors. I found 3 solutions to this problem: 1. Perform a full format of the device by erasing all blocks. This option is costly. 2. Do a dummy write to any sector between map clear and sync. The resumed map will have 1 sector allocated. 3. Set the journal size cookie when calling `dhara_map_clear` The third option is my preferred solution: ``` void dhara_map_clear(struct dhara_map *m) { if (m->count) { m->count = 0; + ck_set_count(dhara_journal_cookie(&m->journal), m->count); dhara_journal_clear(&m->journal); } } ```

Yep, that looks like a bug. Is the problem also fixed if you move the ck_set_count() call in pad_queue() up so it's the first thing that runs? Cheers, Daniel

…

-- Daniel Beer <dlbeer@gmail.com> http://dlbeer.co.nz/ PGP: BA6E 0B26 1F89 246C E3F3 C910 1E58 C43A 160A 553B

[htherrien]

Is the problem also fixed if you move the
ck_set_count() call in pad_queue() up so it's the first thing that runs?

This also fixes the bug, but I am unaware if it will break other map operations.
If you move the ck_set_count() up in pad_queue(), should you also move it up in raw_gc()?

[dlbeer]

On Tue, Sep 17, 2019 at 06:04:15AM -0700, htherrien wrote: > Is the problem also fixed if you move the > ck_set_count() call in pad_queue() up so it's the first thing that runs? This also fixes the bug, but I am unaware if it will break other map operations. If you move the ck_set_count() up in pad_queue(), should you also move it up in raw_gc()?

No, that should be ok -- as long as the count field is updated before anything is written to the journal. The problem is that pad_queue() can add to the journal via enqueue() without updating the count. If you're satisfied that that fixes the problem, I'll go ahead and commit a change. Cheers, Daniel

…

-- Daniel Beer <dlbeer@gmail.com> http://dlbeer.co.nz/ PGP: BA6E 0B26 1F89 246C E3F3 C910 1E58 C43A 160A 553B