Patchwork [RFC,2/3] ext4:Add two functions splitting an extent.

login
register
mail settings
Submitter Allison Henderson
Date April 20, 2011, 5:21 p.m.
Message ID <4DAF1635.7070406@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/92250/
State Not Applicable
Headers show

Comments

Allison Henderson - April 20, 2011, 5:21 p.m.
On 4/13/2011 10:40 PM, Yongqiang Yang wrote:
> 1] Add a function named ext4_split_extent_at() which splits an extent
>     into two extents at given logical block.
> 
> 2] Add a function called ext4_split_extent() which splits an extent
>     into three extents.
> 
> Signed-off-by: Yongqiang Yang<xiaoqiangnk@gmail.com>
> ---
>   fs/ext4/extents.c |  200 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 files changed, 200 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index 11f30d2..1756e0f 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -2554,6 +2554,206 @@ static int ext4_ext_zeroout(struct inode *inode, struct ext4_extent *ex)
>   	return ret;
>   }
> 
> +/*
> + * used by extent splitting.
> + */
> +#define EXT4_EXT_MAY_ZEROOUT	0x1  /* safe to zeroout if split fails \
> +					due to ENOSPC */
> +#define EXT4_EXT_MARK_UNINIT1	0x2  /* mark first half uninitialized */
> +#define EXT4_EXT_MARK_UNINIT2	0x4  /* mark second half uninitialized */
> +
> +/*
> + * ext4_split_extent_at() splits an extent at given block.
> + *
> + * @handle: the journal handle
> + * @inode: the file inode
> + * @path: the path to the extent
> + * @split: the logical block where the extent is splitted.
> + * @split_flags: indicates if the extent could be zeroout if split fails, and
> + *		 the states(init or uninit) of new extents.
> + * @flags: flags used to insert new extent to extent tree.
> + *
> + *
> + * Splits extent [a, b] into two extents [a, @split) and [@split, b], states
> + * of which are deterimined by split_flag.
> + *
> + * There are two cases:
> + *  a>  the extent are splitted into two extent.
> + *  b>  split is not needed, and just mark the extent.
> + *
> + * return 0 on success.
> + */
> +static int ext4_split_extent_at(handle_t *handle,
> +			     struct inode *inode,
> +			     struct ext4_ext_path *path,
> +			     ext4_lblk_t split,
> +			     int split_flag,
> +			     int flags)
> +{
> +	ext4_fsblk_t newblock;
> +	ext4_lblk_t ee_block;
> +	struct ext4_extent_header *eh;
> +	struct ext4_extent *ex, newex, orig_ex;
> +	struct ext4_extent *ex2 = NULL;
> +	unsigned int ee_len, depth;
> +	int err = 0;
> +
> +	ext_debug("ext4_split_extents_at: inode %lu, logical"
> +		"block %llu\n", inode->i_ino, (unsigned long long)split);
> +
> +	ext4_ext_show_leaf(inode, path);
> +
> +	depth = ext_depth(inode);
> +	eh = path[depth].p_hdr;
> +	ex = path[depth].p_ext;
> +	ee_block = le32_to_cpu(ex->ee_block);
> +	ee_len = ext4_ext_get_actual_len(ex);
> +	newblock = split - ee_block + ext4_ext_pblock(ex);
> +
> +	BUG_ON(split<  ee_block || split>= (ee_block + ee_len));
> +
> +	err = ext4_ext_get_access(handle, inode, path + depth);
> +	if (err)
> +		goto out;
> +
> +	if (split == ee_block) {
> +		/*
> +		 * case b: block @split is the block that the extent begins with
> +		 * then we just change the state of the extent, and splitting
> +		 * is not needed.
> +		 */
> +		if (split_flag&  EXT4_EXT_MARK_UNINIT2)
> +			ext4_ext_mark_uninitialized(ex);
> +		else
> +			ext4_ext_mark_initialized(ex);
> +
> +		if (!(flags&  EXT4_GET_BLOCKS_PRE_IO))
> +			ext4_ext_try_to_merge(inode, path, ex);
> +
> +		err = ext4_ext_dirty(handle, inode, path + depth);
> +		goto out;
> +	}
> +
> +	/* case a */
> +	orig_ex.ee_block = ex->ee_block;
> +	orig_ex.ee_len   = ex->ee_len;
> +	ext4_ext_store_pblock(&orig_ex, ext4_ext_pblock(ex));
> +
> +	ex->ee_len = cpu_to_le16(split - ee_block);
> +	if (split_flag&  EXT4_EXT_MARK_UNINIT1)
> +		ext4_ext_mark_uninitialized(ex);
> +
> +	/*
> +	 * path may lead to new leaf, not to original leaf any more
> +	 * after ext4_ext_insert_extent() returns,
> +	 */
> +	err = ext4_ext_dirty(handle, inode, path + depth);
> +	if (err)
> +		goto fix_extent_len;
> +
> +	ex2 =&newex;
> +	ex2->ee_block = cpu_to_le32(split);
> +	ex2->ee_len   = cpu_to_le16(ee_len - (split - ee_block));
> +	ext4_ext_store_pblock(ex2, newblock);
> +	if (split_flag&  EXT4_EXT_MARK_UNINIT2)
> +		ext4_ext_mark_uninitialized(ex2);
> +
> +	err = ext4_ext_insert_extent(handle, inode, path,&newex, flags);
> +	if (err == -ENOSPC&&  (EXT4_EXT_MAY_ZEROOUT&  split_flag)) {
> +
> +		err = ext4_ext_zeroout(inode,&orig_ex);
> +		if (err)
> +			goto fix_extent_len;
> +		/* update the extent length and mark as initialized */
> +		ex->ee_block = orig_ex.ee_block;
> +		ex->ee_len   = orig_ex.ee_len;
> +		ext4_ext_mark_initialized(ex);
> +		ext4_ext_store_pblock(ex, ext4_ext_pblock(&orig_ex));
> +		err = ext4_ext_dirty(handle, inode, path + depth);
> +		goto out;
> +	} else if (err)
> +		goto fix_extent_len;
> +
> +out:
> +	ext4_ext_show_leaf(inode, path);
> +	return err;
> +
> +fix_extent_len:
> +	ex->ee_block = orig_ex.ee_block;
> +	ex->ee_len   = orig_ex.ee_len;
> +	ext4_ext_store_pblock(ex, ext4_ext_pblock(&orig_ex));
> +	ext4_ext_dirty(handle, inode, path + depth);
> +	return err;
> +}
> +
> +/*
> + * ext4_split_extents() splits an extent and mark extent which is covered
> + * by @map as split_flags indicates
> + *
> + * It may result in splitting the extent into multiple extents (upto three)
> + * There are three possibilities:
> + *   a>  There is no split required
> + *   b>  Splits in two extents: Split is happening at either end of the extent
> + *   c>  Splits in three extents: Somone is splitting in middle of the extent
> + *
> + */
> +static int ext4_split_extent(handle_t *handle,
> +			      struct inode *inode,
> +			      struct ext4_ext_path *path,
> +			      struct ext4_map_blocks *map,
> +			      int split_flag,
> +			      int flags)
> +{
> +	ext4_lblk_t ee_block;
> +	struct ext4_extent *ex;
> +	unsigned int ee_len, depth;
> +	int err = 0;
> +	int uninitialized;
> +	int split_flag1, flags1;
> +
> +	depth = ext_depth(inode);
> +	ex = path[depth].p_ext;
> +	ee_block = le32_to_cpu(ex->ee_block);
> +	ee_len = ext4_ext_get_actual_len(ex);
> +	uninitialized = ext4_ext_is_uninitialized(ex);
> +
> +	if (map->m_lblk + map->m_len<  ee_block + ee_len) {
> +		split_flag1 = 0;
> +		split_flag1 |= split_flag&  EXT4_EXT_MAY_ZEROOUT ?
> +			       EXT4_EXT_MAY_ZEROOUT : 0;
> +		flags1 = flags;
> +		flags1 |= EXT4_GET_BLOCKS_PRE_IO;
> +		if (uninitialized)
> +			split_flag1 |= EXT4_EXT_MARK_UNINIT1 |
> +				       EXT4_EXT_MARK_UNINIT2;
> +		err = ext4_split_extent_at(handle, inode, path,
> +				map->m_lblk + map->m_len, split_flag1, flags1);
> +	}
> +
> +	ext4_ext_drop_refs(path);
> +	path = ext4_ext_find_extent(inode, map->m_lblk, path);
> +	if (IS_ERR(path))
> +		return PTR_ERR(path);
> +
> +	if (map->m_lblk>= ee_block) {
> +		split_flag1 = 0;
> +		split_flag1 |= split_flag&  EXT4_EXT_MAY_ZEROOUT ?
> +			       EXT4_EXT_MAY_ZEROOUT : 0;
> +		if (uninitialized)
> +			split_flag1 |= EXT4_EXT_MARK_UNINIT1;
> +		if (split_flag&  EXT4_EXT_MARK_UNINIT2)
> +			split_flag1 |= EXT4_EXT_MARK_UNINIT2;
> +		err = ext4_split_extent_at(handle, inode, path,
> +				map->m_lblk, split_flag1, flags);
> +		if (err)
> +			goto out;
> +	}
> +
> +	ext4_ext_show_leaf(inode, path);
> +out:
> +	return err ? err : map->m_len;
> +}
> +
>   #define EXT4_EXT_ZERO_LEN 7
>   /*
>    * This function is called by ext4_ext_map_blocks() if someone tries to write


Hi there,

I've been working on trying to get the punch hole patch to work with with these new changes, but it looks like some test cases are not passing at the moment, so I'm trying track down where the issues are.  I had to make some adjustments to this patch to fix one of the test cases.  Here is what I did:

---
:100644 100644 ee2dda3... c7d763d... M  fs/ext4/extents.c
 fs/ext4/extents.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

--
1.7.1


Let me know if this change works for you.  It looks like the problem was that the extents were getting merged back together because the second ext4_split_extent_at didnt have the EXT4_GET_BLOCKS_PRE_IO flag, so this should fix it.  There are still some other test cases that are showing some odd behavior, so I will keep you posted if I have to make any other changes.

Allison Henderson
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andreas Dilger - April 20, 2011, 6:13 p.m.
On 2011-04-20, at 11:21 AM, Allison Henderson wrote:
> I've been working on trying to get the punch hole patch to work with with these new changes, but it looks like some test cases are not passing at the moment, so I'm trying track down where the issues are.  I had to make some adjustments to this patch to fix one of the test cases.  Here is what I did:
> 
> ---
> :100644 100644 ee2dda3... c7d763d... M  fs/ext4/extents.c
> fs/ext4/extents.c |    6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index ee2dda3..c7d763d 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -2717,12 +2717,12 @@ static int ext4_split_extent(handle_t *handle,
>        ee_len = ext4_ext_get_actual_len(ex);
>        uninitialized = ext4_ext_is_uninitialized(ex);
> 
> +       flags1 = flags;
> +       flags1 |= EXT4_GET_BLOCKS_PRE_IO;

Can you please use normal C style: "flags1 = flags | EXT4_GET_BLOCKS_PRE_IO;"


Cheers, Andreas





--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Allison Henderson - April 20, 2011, 8:52 p.m.
On 4/20/2011 11:13 AM, Andreas Dilger wrote:
> On 2011-04-20, at 11:21 AM, Allison Henderson wrote:
>> I've been working on trying to get the punch hole patch to work with with these new changes, but it looks like some test cases are not passing at the moment, so I'm trying track down where the issues are.  I had to make some adjustments to this patch to fix one of the test cases.  Here is what I did:
>>
>> ---
>> :100644 100644 ee2dda3... c7d763d... M  fs/ext4/extents.c
>> fs/ext4/extents.c |    6 +++---
>> 1 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
>> index ee2dda3..c7d763d 100644
>> --- a/fs/ext4/extents.c
>> +++ b/fs/ext4/extents.c
>> @@ -2717,12 +2717,12 @@ static int ext4_split_extent(handle_t *handle,
>>         ee_len = ext4_ext_get_actual_len(ex);
>>         uninitialized = ext4_ext_is_uninitialized(ex);
>>
>> +       flags1 = flags;
>> +       flags1 |= EXT4_GET_BLOCKS_PRE_IO;
> 
> Can you please use normal C style: "flags1 = flags | EXT4_GET_BLOCKS_PRE_IO;"
> 
> 
> Cheers, Andreas
> 
> 
> 
> 
> 
Hi again,

I realize after I sent the note that maybe the caller is still expected to pass EXT4_GET_BLOCKS_PRE_IO in the flags parameter if it is supposed to be used in both ext4_split_extent_at cases.  That also fixes that particular failing test case. So maybe we wont need the extra fix if that is how the code is intended to work.  In any case though, yes "flags1 = flags | EXT4_GET_BLOCKS_PRE_IO;" does seem a bit cleaner. 

Allison Henderson
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Yongqiang Yang - April 21, 2011, 1:18 a.m.
On Thu, Apr 21, 2011 at 1:21 AM, Allison Henderson
<achender@linux.vnet.ibm.com> wrote:
> On 4/13/2011 10:40 PM, Yongqiang Yang wrote:
>> 1] Add a function named ext4_split_extent_at() which splits an extent
>>     into two extents at given logical block.
>>
>> 2] Add a function called ext4_split_extent() which splits an extent
>>     into three extents.
>>
>> Signed-off-by: Yongqiang Yang<xiaoqiangnk@gmail.com>
>> ---
>>   fs/ext4/extents.c |  200 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 files changed, 200 insertions(+), 0 deletions(-)
>>
>> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
>> index 11f30d2..1756e0f 100644
>> --- a/fs/ext4/extents.c
>> +++ b/fs/ext4/extents.c
>> @@ -2554,6 +2554,206 @@ static int ext4_ext_zeroout(struct inode *inode, struct ext4_extent *ex)
>>       return ret;
>>   }
>>
>> +/*
>> + * used by extent splitting.
>> + */
>> +#define EXT4_EXT_MAY_ZEROOUT 0x1  /* safe to zeroout if split fails \
>> +                                     due to ENOSPC */
>> +#define EXT4_EXT_MARK_UNINIT1        0x2  /* mark first half uninitialized */
>> +#define EXT4_EXT_MARK_UNINIT2        0x4  /* mark second half uninitialized */
>> +
>> +/*
>> + * ext4_split_extent_at() splits an extent at given block.
>> + *
>> + * @handle: the journal handle
>> + * @inode: the file inode
>> + * @path: the path to the extent
>> + * @split: the logical block where the extent is splitted.
>> + * @split_flags: indicates if the extent could be zeroout if split fails, and
>> + *            the states(init or uninit) of new extents.
>> + * @flags: flags used to insert new extent to extent tree.
>> + *
>> + *
>> + * Splits extent [a, b] into two extents [a, @split) and [@split, b], states
>> + * of which are deterimined by split_flag.
>> + *
>> + * There are two cases:
>> + *  a>  the extent are splitted into two extent.
>> + *  b>  split is not needed, and just mark the extent.
>> + *
>> + * return 0 on success.
>> + */
>> +static int ext4_split_extent_at(handle_t *handle,
>> +                          struct inode *inode,
>> +                          struct ext4_ext_path *path,
>> +                          ext4_lblk_t split,
>> +                          int split_flag,
>> +                          int flags)
>> +{
>> +     ext4_fsblk_t newblock;
>> +     ext4_lblk_t ee_block;
>> +     struct ext4_extent_header *eh;
>> +     struct ext4_extent *ex, newex, orig_ex;
>> +     struct ext4_extent *ex2 = NULL;
>> +     unsigned int ee_len, depth;
>> +     int err = 0;
>> +
>> +     ext_debug("ext4_split_extents_at: inode %lu, logical"
>> +             "block %llu\n", inode->i_ino, (unsigned long long)split);
>> +
>> +     ext4_ext_show_leaf(inode, path);
>> +
>> +     depth = ext_depth(inode);
>> +     eh = path[depth].p_hdr;
>> +     ex = path[depth].p_ext;
>> +     ee_block = le32_to_cpu(ex->ee_block);
>> +     ee_len = ext4_ext_get_actual_len(ex);
>> +     newblock = split - ee_block + ext4_ext_pblock(ex);
>> +
>> +     BUG_ON(split<  ee_block || split>= (ee_block + ee_len));
>> +
>> +     err = ext4_ext_get_access(handle, inode, path + depth);
>> +     if (err)
>> +             goto out;
>> +
>> +     if (split == ee_block) {
>> +             /*
>> +              * case b: block @split is the block that the extent begins with
>> +              * then we just change the state of the extent, and splitting
>> +              * is not needed.
>> +              */
>> +             if (split_flag&  EXT4_EXT_MARK_UNINIT2)
>> +                     ext4_ext_mark_uninitialized(ex);
>> +             else
>> +                     ext4_ext_mark_initialized(ex);
>> +
>> +             if (!(flags&  EXT4_GET_BLOCKS_PRE_IO))
>> +                     ext4_ext_try_to_merge(inode, path, ex);
>> +
>> +             err = ext4_ext_dirty(handle, inode, path + depth);
>> +             goto out;
>> +     }
>> +
>> +     /* case a */
>> +     orig_ex.ee_block = ex->ee_block;
>> +     orig_ex.ee_len   = ex->ee_len;
>> +     ext4_ext_store_pblock(&orig_ex, ext4_ext_pblock(ex));
>> +
>> +     ex->ee_len = cpu_to_le16(split - ee_block);
>> +     if (split_flag&  EXT4_EXT_MARK_UNINIT1)
>> +             ext4_ext_mark_uninitialized(ex);
>> +
>> +     /*
>> +      * path may lead to new leaf, not to original leaf any more
>> +      * after ext4_ext_insert_extent() returns,
>> +      */
>> +     err = ext4_ext_dirty(handle, inode, path + depth);
>> +     if (err)
>> +             goto fix_extent_len;
>> +
>> +     ex2 =&newex;
>> +     ex2->ee_block = cpu_to_le32(split);
>> +     ex2->ee_len   = cpu_to_le16(ee_len - (split - ee_block));
>> +     ext4_ext_store_pblock(ex2, newblock);
>> +     if (split_flag&  EXT4_EXT_MARK_UNINIT2)
>> +             ext4_ext_mark_uninitialized(ex2);
>> +
>> +     err = ext4_ext_insert_extent(handle, inode, path,&newex, flags);
>> +     if (err == -ENOSPC&&  (EXT4_EXT_MAY_ZEROOUT&  split_flag)) {
>> +
>> +             err = ext4_ext_zeroout(inode,&orig_ex);
>> +             if (err)
>> +                     goto fix_extent_len;
>> +             /* update the extent length and mark as initialized */
>> +             ex->ee_block = orig_ex.ee_block;
>> +             ex->ee_len   = orig_ex.ee_len;
>> +             ext4_ext_mark_initialized(ex);
>> +             ext4_ext_store_pblock(ex, ext4_ext_pblock(&orig_ex));
>> +             err = ext4_ext_dirty(handle, inode, path + depth);
>> +             goto out;
>> +     } else if (err)
>> +             goto fix_extent_len;
>> +
>> +out:
>> +     ext4_ext_show_leaf(inode, path);
>> +     return err;
>> +
>> +fix_extent_len:
>> +     ex->ee_block = orig_ex.ee_block;
>> +     ex->ee_len   = orig_ex.ee_len;
>> +     ext4_ext_store_pblock(ex, ext4_ext_pblock(&orig_ex));
>> +     ext4_ext_dirty(handle, inode, path + depth);
>> +     return err;
>> +}
>> +
>> +/*
>> + * ext4_split_extents() splits an extent and mark extent which is covered
>> + * by @map as split_flags indicates
>> + *
>> + * It may result in splitting the extent into multiple extents (upto three)
>> + * There are three possibilities:
>> + *   a>  There is no split required
>> + *   b>  Splits in two extents: Split is happening at either end of the extent
>> + *   c>  Splits in three extents: Somone is splitting in middle of the extent
>> + *
>> + */
>> +static int ext4_split_extent(handle_t *handle,
>> +                           struct inode *inode,
>> +                           struct ext4_ext_path *path,
>> +                           struct ext4_map_blocks *map,
>> +                           int split_flag,
>> +                           int flags)
>> +{
>> +     ext4_lblk_t ee_block;
>> +     struct ext4_extent *ex;
>> +     unsigned int ee_len, depth;
>> +     int err = 0;
>> +     int uninitialized;
>> +     int split_flag1, flags1;
>> +
>> +     depth = ext_depth(inode);
>> +     ex = path[depth].p_ext;
>> +     ee_block = le32_to_cpu(ex->ee_block);
>> +     ee_len = ext4_ext_get_actual_len(ex);
>> +     uninitialized = ext4_ext_is_uninitialized(ex);
>> +
>> +     if (map->m_lblk + map->m_len<  ee_block + ee_len) {
>> +             split_flag1 = 0;
>> +             split_flag1 |= split_flag&  EXT4_EXT_MAY_ZEROOUT ?
>> +                            EXT4_EXT_MAY_ZEROOUT : 0;
>> +             flags1 = flags;
>> +             flags1 |= EXT4_GET_BLOCKS_PRE_IO;
>> +             if (uninitialized)
>> +                     split_flag1 |= EXT4_EXT_MARK_UNINIT1 |
>> +                                    EXT4_EXT_MARK_UNINIT2;
>> +             err = ext4_split_extent_at(handle, inode, path,
>> +                             map->m_lblk + map->m_len, split_flag1, flags1);
>> +     }
>> +
>> +     ext4_ext_drop_refs(path);
>> +     path = ext4_ext_find_extent(inode, map->m_lblk, path);
>> +     if (IS_ERR(path))
>> +             return PTR_ERR(path);
>> +
>> +     if (map->m_lblk>= ee_block) {
>> +             split_flag1 = 0;
>> +             split_flag1 |= split_flag&  EXT4_EXT_MAY_ZEROOUT ?
>> +                            EXT4_EXT_MAY_ZEROOUT : 0;
>> +             if (uninitialized)
>> +                     split_flag1 |= EXT4_EXT_MARK_UNINIT1;
>> +             if (split_flag&  EXT4_EXT_MARK_UNINIT2)
>> +                     split_flag1 |= EXT4_EXT_MARK_UNINIT2;
>> +             err = ext4_split_extent_at(handle, inode, path,
>> +                             map->m_lblk, split_flag1, flags);
>> +             if (err)
>> +                     goto out;
>> +     }
>> +
>> +     ext4_ext_show_leaf(inode, path);
>> +out:
>> +     return err ? err : map->m_len;
>> +}
>> +
>>   #define EXT4_EXT_ZERO_LEN 7
>>   /*
>>    * This function is called by ext4_ext_map_blocks() if someone tries to write
>
>
> Hi there,
>
> I've been working on trying to get the punch hole patch to work with with these new changes, but it looks like some test cases are not passing at the moment, so I'm trying track down where the issues are.  I had to make some adjustments to this patch to fix one of the test cases.  Here is what I did:
>
> ---
> :100644 100644 ee2dda3... c7d763d... M  fs/ext4/extents.c
>  fs/ext4/extents.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index ee2dda3..c7d763d 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -2717,12 +2717,12 @@ static int ext4_split_extent(handle_t *handle,
>        ee_len = ext4_ext_get_actual_len(ex);
>        uninitialized = ext4_ext_is_uninitialized(ex);
>
> +       flags1 = flags;
> +       flags1 |= EXT4_GET_BLOCKS_PRE_IO;
Hi, the flag should be passed via parameters,
ext4_convert_to_initialized() uses this function also.  so merge
should be done in this case.

If the patches has other problems needing me to fix, please let me know.

Thank you,
Yongqiang.

>        if (map->m_lblk + map->m_len < ee_block + ee_len) {
>                split_flag1 = 0;
>                split_flag1 |= split_flag & EXT4_EXT_MAY_ZEROOUT ?
>                               EXT4_EXT_MAY_ZEROOUT : 0;
> -               flags1 = flags;
> -               flags1 |= EXT4_GET_BLOCKS_PRE_IO;
>                if (uninitialized)
>                        split_flag1 |= EXT4_EXT_MARK_UNINIT1 |
>                                       EXT4_EXT_MARK_UNINIT2;
> @@ -2744,7 +2744,7 @@ static int ext4_split_extent(handle_t *handle,
>                if (split_flag & EXT4_EXT_MARK_UNINIT2)
>                        split_flag1 |= EXT4_EXT_MARK_UNINIT2;
>                err = ext4_split_extent_at(handle, inode, path,
> -                               map->m_lblk, split_flag1, flags);
> +                               map->m_lblk, split_flag1, flags1);
>                if (err)
>                        goto out;
>        }
> --
> 1.7.1
>
>
> Let me know if this change works for you.  It looks like the problem was that the extents were getting merged back together because the second ext4_split_extent_at didnt have the EXT4_GET_BLOCKS_PRE_IO flag, so this should fix it.  There are still some other test cases that are showing some odd behavior, so I will keep you posted if I have to make any other changes.
>
> Allison Henderson
>

Patch

diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index ee2dda3..c7d763d 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -2717,12 +2717,12 @@  static int ext4_split_extent(handle_t *handle,
        ee_len = ext4_ext_get_actual_len(ex);
        uninitialized = ext4_ext_is_uninitialized(ex);

+       flags1 = flags;
+       flags1 |= EXT4_GET_BLOCKS_PRE_IO;
        if (map->m_lblk + map->m_len < ee_block + ee_len) {
                split_flag1 = 0;
                split_flag1 |= split_flag & EXT4_EXT_MAY_ZEROOUT ?
                               EXT4_EXT_MAY_ZEROOUT : 0;
-               flags1 = flags;
-               flags1 |= EXT4_GET_BLOCKS_PRE_IO;
                if (uninitialized)
                        split_flag1 |= EXT4_EXT_MARK_UNINIT1 |
                                       EXT4_EXT_MARK_UNINIT2;
@@ -2744,7 +2744,7 @@  static int ext4_split_extent(handle_t *handle,
                if (split_flag & EXT4_EXT_MARK_UNINIT2)
                        split_flag1 |= EXT4_EXT_MARK_UNINIT2;
                err = ext4_split_extent_at(handle, inode, path,
-                               map->m_lblk, split_flag1, flags);
+                               map->m_lblk, split_flag1, flags1);
                if (err)
                        goto out;
        }