ACK/cmnt: [PATCH] ext4: merge uninitialized extents

[Stefan Bader]

Sorry for the delay. So given the explanation (which really should go into the
bug report as a SRU justification to give a reasoning for it) I think the change
looks simple enough, does fix some kind of bug and should be testable.
For future submissions you should use a subject like "[SRU Trusty]..." so it is
clear which release(s) you are targeting.
Also the BugLink has to be in a single line. I would suggest the following style
as that also shows the path the patch too (so after all the upstream signoff, it
was cherry-picked or backported and then sent (signed-off) by you).

-Stefan

On 11.03.2015 04:50, Ming Lei wrote:
> From: "Darrick J. Wong" <darrick.wong at oracle.com>
> 
> Allow for merging uninitialized extents.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong at oracle.com>
> Signed-off-by: "Theodore Ts'o" <tytso at mit.edu>

BugLink: https://bugs.launchpad.net/bugs/1430184

(cherry-picked from commit a9b8241 upstream)
Signed-off-by: Ming Lei <ming.lei at canonical.com>

> ---
>  fs/ext4/extents.c |   21 +++++++++++++++++----
>  1 file changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index 9875fd0..ef4b535 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -1691,7 +1691,7 @@ ext4_can_extents_be_merged(struct inode *inode, struct ext4_extent *ex1,
>  	 * the extent that was written properly split out and conversion to
>  	 * initialized is trivial.
>  	 */
> -	if (ext4_ext_is_uninitialized(ex1) || ext4_ext_is_uninitialized(ex2))
> +	if (ext4_ext_is_uninitialized(ex1) != ext4_ext_is_uninitialized(ex2))
>  		return 0;
>  
>  	ext1_ee_len = ext4_ext_get_actual_len(ex1);
> @@ -1708,6 +1708,11 @@ ext4_can_extents_be_merged(struct inode *inode, struct ext4_extent *ex1,
>  	 */
>  	if (ext1_ee_len + ext2_ee_len > EXT_INIT_MAX_LEN)
>  		return 0;
> +	if (ext4_ext_is_uninitialized(ex1) &&
> +	    (ext4_test_inode_state(inode, EXT4_STATE_DIO_UNWRITTEN) ||
> +	     atomic_read(&EXT4_I(inode)->i_unwritten) ||
> +	     (ext1_ee_len + ext2_ee_len > EXT_UNINIT_MAX_LEN)))
> +		return 0;
>  #ifdef AGGRESSIVE_TEST
>  	if (ext1_ee_len >= 4)
>  		return 0;
> @@ -1731,7 +1736,7 @@ static int ext4_ext_try_to_merge_right(struct inode *inode,
>  {
>  	struct ext4_extent_header *eh;
>  	unsigned int depth, len;
> -	int merge_done = 0;
> +	int merge_done = 0, uninit;
>  
>  	depth = ext_depth(inode);
>  	BUG_ON(path[depth].p_hdr == NULL);
> @@ -1741,8 +1746,11 @@ static int ext4_ext_try_to_merge_right(struct inode *inode,
>  		if (!ext4_can_extents_be_merged(inode, ex, ex + 1))
>  			break;
>  		/* merge with next extent! */
> +		uninit = ext4_ext_is_uninitialized(ex);
>  		ex->ee_len = cpu_to_le16(ext4_ext_get_actual_len(ex)
>  				+ ext4_ext_get_actual_len(ex + 1));
> +		if (uninit)
> +			ext4_ext_mark_uninitialized(ex);
>  
>  		if (ex + 1 < EXT_LAST_EXTENT(eh)) {
>  			len = (EXT_LAST_EXTENT(eh) - ex - 1)
> @@ -1896,7 +1904,7 @@ int ext4_ext_insert_extent(handle_t *handle, struct inode *inode,
>  	struct ext4_ext_path *npath = NULL;
>  	int depth, len, err;
>  	ext4_lblk_t next;
> -	int mb_flags = 0;
> +	int mb_flags = 0, uninit;
>  
>  	if (unlikely(ext4_ext_get_actual_len(newext) == 0)) {
>  		EXT4_ERROR_INODE(inode, "ext4_ext_get_actual_len(newext) == 0");
> @@ -1946,9 +1954,11 @@ int ext4_ext_insert_extent(handle_t *handle, struct inode *inode,
>  						  path + depth);
>  			if (err)
>  				return err;
> -
> +			uninit = ext4_ext_is_uninitialized(ex);
>  			ex->ee_len = cpu_to_le16(ext4_ext_get_actual_len(ex)
>  					+ ext4_ext_get_actual_len(newext));
> +			if (uninit)
> +				ext4_ext_mark_uninitialized(ex);
>  			eh = path[depth].p_hdr;
>  			nearex = ex;
>  			goto merge;
> @@ -1971,10 +1981,13 @@ prepend:
>  			if (err)
>  				return err;
>  
> +			uninit = ext4_ext_is_uninitialized(ex);
>  			ex->ee_block = newext->ee_block;
>  			ext4_ext_store_pblock(ex, ext4_ext_pblock(newext));
>  			ex->ee_len = cpu_to_le16(ext4_ext_get_actual_len(ex)
>  					+ ext4_ext_get_actual_len(newext));
> +			if (uninit)
> +				ext4_ext_mark_uninitialized(ex);
>  			eh = path[depth].p_hdr;
>  			nearex = ex;
>  			goto merge;
> 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20150318/0ddc63a7/attachment.sig>