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.
