On Thu, Jul 01, 2021 at 05:55:51PM +0100, Filipe Manana wrote:
if (pre) {/* Insert the middle extent_map */split_mid->start = em->start + pre;split_mid->len = em->len - pre - post;split_mid->orig_start = split_mid->start;split_mid->block_start = em->block_start + pre;split_mid->block_len = split_mid->len;split_mid->orig_block_len = split_mid->block_len;split_mid->ram_bytes = split_mid->len;split_mid->flags = flags;split_mid->compress_type = em->compress_type;split_mid->generation = em->generation;add_extent_mapping(em_tree, split_mid, modified);}if (post) {split_post->start = em->start + em->len - post;split_post->len = post;split_post->orig_start = split_post->start;split_post->block_start = em->block_start + em->len - post;split_post->block_len = split_post->len;split_post->orig_block_len = split_post->block_len;split_post->ram_bytes = split_post->len;split_post->flags = flags;split_post->compress_type = em->compress_type;split_post->generation = em->generation;add_extent_mapping(em_tree, split_post, modified);}So this happens when running delalloc, after creating the original extent map and ordered extent, the original "em" must have had the PINNED flag set.
The "pre" and "post" extent maps should have the PINNED flag set. It's important to have the flag set to prevent extent map merging, which could result in a log corruption if the file is being fsync'ed multiple times in the current transaction and running delalloc was triggered precisely by an fsync. The corruption result would be logging extent items with overlapping ranges, since we construct them based on extent maps, and that's why we have the PINNED flag to prevent merging.
Well, it actually happens that merging should not happen because the original extent map was in the list of modified extents, and so should be the new extent maps. But we are really supposed to have the PINNED flag from the moment we run delalloc and create a new extent map until the respective ordered extent completes and unpins it.
Also EXTENT_FLAG_LOGGING should not be set at this point - if it were we would screw up with a task logging the extent map.
Maybe assert that it is not set in the original extent map? And also assert that the original em is in the list of modified extents and has the PINNED flag set?
Agreed, the asserts should be here, Naohiro, please send a followup, thanks.