Skip to content

Commit

Permalink
ext4: ext4_split_extent should take care of extent zeroout
Browse files Browse the repository at this point in the history
When ext4_split_extent_at() ends up doing zeroout & conversion to
initialized instead of split & conversion, ext4_split_extent() gets
confused and can wrongly mark the extent back as uninitialized
resulting in end IO code getting confused from large unwritten extents
and may result in data loss.

The example of problematic behavior is:
			    lblk len              lblk len
  ext4_split_extent() (ex=[1000,30,uninit], map=[1010,10])
    ext4_split_extent_at() (split [1000,30,uninit] at 1020)
      ext4_ext_insert_extent() -> ENOSPC
      ext4_ext_zeroout()
	 -> extent [1000,30] is now initialized
    ext4_split_extent_at() (split [1000,30,init] at 1010,
			     MARK_UNINIT1 | MARK_UNINIT2)
      -> extent is split and parts marked as uninitialized

Fix the problem by rechecking extent type after the first
ext4_split_extent_at() returns. None of split_flags can not be applied
to initialized extent so this patch also add BUG_ON to prevent similar
issues in future.

TESTCASE: https://github.com/dmonakhov/xfstests/commit/b8a55eb5ce28c6ff29e620ab090902fcd5833597

Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
Reviewed-by: Jan Kara <jack@suse.cz>
  • Loading branch information
Dmitry Monakhov authored and Theodore Ts'o committed Mar 4, 2013
1 parent 9b2ff35 commit 357b66f
Showing 1 changed file with 17 additions and 6 deletions.
23 changes: 17 additions & 6 deletions fs/ext4/extents.c
Original file line number Diff line number Diff line change
Expand Up @@ -2943,6 +2943,10 @@ static int ext4_split_extent_at(handle_t *handle,
newblock = split - ee_block + ext4_ext_pblock(ex);

BUG_ON(split < ee_block || split >= (ee_block + ee_len));
BUG_ON(!ext4_ext_is_uninitialized(ex) &&
split_flag & (EXT4_EXT_MAY_ZEROOUT |
EXT4_EXT_MARK_UNINIT1 |
EXT4_EXT_MARK_UNINIT2));

err = ext4_ext_get_access(handle, inode, path + depth);
if (err)
Expand Down Expand Up @@ -3061,19 +3065,26 @@ static int ext4_split_extent(handle_t *handle,
if (err)
goto out;
}

/*
* Update path is required because previous ext4_split_extent_at() may
* result in split of original leaf or extent zeroout.
*/
ext4_ext_drop_refs(path);
path = ext4_ext_find_extent(inode, map->m_lblk, path);
if (IS_ERR(path))
return PTR_ERR(path);
depth = ext_depth(inode);
ex = path[depth].p_ext;
uninitialized = ext4_ext_is_uninitialized(ex);
split_flag1 = 0;

if (map->m_lblk >= ee_block) {
split_flag1 = split_flag & (EXT4_EXT_MAY_ZEROOUT |
EXT4_EXT_DATA_VALID2);
if (uninitialized)
split_flag1 = split_flag & EXT4_EXT_DATA_VALID2;
if (uninitialized) {
split_flag1 |= EXT4_EXT_MARK_UNINIT1;
if (split_flag & EXT4_EXT_MARK_UNINIT2)
split_flag1 |= EXT4_EXT_MARK_UNINIT2;
split_flag1 |= split_flag & (EXT4_EXT_MAY_ZEROOUT |
EXT4_EXT_MARK_UNINIT2);
}
err = ext4_split_extent_at(handle, inode, path,
map->m_lblk, split_flag1, flags);
if (err)
Expand Down

0 comments on commit 357b66f

Please sign in to comment.