ext4: merge uninitialized extents
diff mbox

Message ID 1426045819-24485-1-git-send-email-ming.lei@canonical.com
State New
Headers show

Commit Message

Ming Lei March 11, 2015, 3:50 a.m. UTC
From: "Darrick J. Wong" <darrick.wong@oracle.com>

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

Upstream commit:
        a9b8241 ext4: merge uninitialized extents

Allow for merging uninitialized extents.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 fs/ext4/extents.c |   21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

Comments

Stefan Bader March 11, 2015, 9:03 a.m. UTC | #1
On 11.03.2015 04:50, Ming Lei wrote:
> From: "Darrick J. Wong" <darrick.wong@oracle.com>
> 
> Buglink:
>         https://bugs.launchpad.net/bugs/1430184
> 
> Upstream commit:
>         a9b8241 ext4: merge uninitialized extents

Hi Ming,

it would help to add here whether the patch could be cherry-picked or needed
some backporting. From the look of it, it seems to be a clean pick.
Oh and also it helps to know what release(s) are targeted from the subject of an
SRU email. In this case it has to be Trusty according to the comments in the bug
report and the fact this patch came with 3.15.

Generally I am a bit undecided about this one. The commit message does not
really sound like this was considered a fix for something misbehaving. And the
fact that xfs tests fail may also be due to the test-case not properly checking
the features available. So the question for me would be whether there is a real
world use-case broken or not.

-Stefan

> 
> Allow for merging uninitialized extents.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
> ---
>  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;
>
Ming Lei March 11, 2015, 9:56 a.m. UTC | #2
Hi,

On Wed, Mar 11, 2015 at 5:03 PM, Stefan Bader
<stefan.bader@canonical.com> wrote:
> On 11.03.2015 04:50, Ming Lei wrote:
>> From: "Darrick J. Wong" <darrick.wong@oracle.com>
>>
>> Buglink:
>>         https://bugs.launchpad.net/bugs/1430184
>>
>> Upstream commit:
>>         a9b8241 ext4: merge uninitialized extents
>
> Hi Ming,
>
> it would help to add here whether the patch could be cherry-picked or needed
> some backporting. From the look of it, it seems to be a clean pick.

Yes, 'git am' can apply the patch cleanly.

> Oh and also it helps to know what release(s) are targeted from the subject of an
> SRU email. In this case it has to be Trusty according to the comments in the bug
> report and the fact this patch came with 3.15.
>
> Generally I am a bit undecided about this one. The commit message does not
> really sound like this was considered a fix for something misbehaving. And the
> fact that xfs tests fail may also be due to the test-case not properly checking
> the features available. So the question for me would be whether there is a real
> world use-case broken or not.

The xfstest tests(generic/324 and ext4/308) doesn't depend on
some 'features'.

This test just makes some holes in one file over ext4 via falloc & write,
then run 'e4defrag' to try to reduce fragmentation of the file.

Unfortunately the fragmentation of the test file aren't reduced by
e4defrag on trusty in this test case , instead of being increased
a lot(for 36 to 500+), so the bug can affect performance.

Please see the 1st sentence in DESCRIPTION section of man page
of e4defrag:

     'e4defrag  reduces fragmentation of extent based file.'

Thanks,
Ming Lei

>
> -Stefan
>
>>
>> Allow for merging uninitialized extents.
>>
>> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
>> Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
>> ---
>>  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;
>>
>
>
Stefan Bader March 18, 2015, 10:56 a.m. UTC | #3
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@oracle.com>
> 
> Allow for merging uninitialized extents.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>

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

(cherry-picked from commit a9b8241 upstream)
Signed-off-by: Ming Lei <ming.lei@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;
>
Luis Henriques March 18, 2015, 11:46 a.m. UTC | #4
On Wed, Mar 11, 2015 at 11:50:19AM +0800, Ming Lei wrote:
> From: "Darrick J. Wong" <darrick.wong@oracle.com>
> 
> Buglink:
>         https://bugs.launchpad.net/bugs/1430184
> 
> Upstream commit:
>         a9b8241 ext4: merge uninitialized extents
> 
> Allow for merging uninitialized extents.
>

This is an old commit and I couldn't find indication of regressions
caused by it.  It looks like a good candidate for an SRU, specially
because it's easily tested.

And I obviously also agree with Stefan's comments!

Cheers,
--
Luís

> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
> ---
>  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;
> -- 
> 1.7.9.5
> 
> 
> -- 
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
Ming Lei March 18, 2015, 12:15 p.m. UTC | #5
On Wed, Mar 18, 2015 at 7:46 PM, Luis Henriques
<luis.henriques@canonical.com> wrote:
> On Wed, Mar 11, 2015 at 11:50:19AM +0800, Ming Lei wrote:
>> From: "Darrick J. Wong" <darrick.wong@oracle.com>
>>
>> Buglink:
>>         https://bugs.launchpad.net/bugs/1430184
>>
>> Upstream commit:
>>         a9b8241 ext4: merge uninitialized extents
>>
>> Allow for merging uninitialized extents.
>>
>
> This is an old commit and I couldn't find indication of regressions
> caused by it.  It looks like a good candidate for an SRU, specially
> because it's easily tested.

Luis, thanks for the Ack.

Hope this commit can be merged because all xfstests(-g auto)
deployed in hyperscale lab on trusty can only be passed if this
patch is applied.

Thanks,
>
> And I obviously also agree with Stefan's comments!
>
> Cheers,
> --
> Luís
>
>> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
>> Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
>> ---
>>  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;
>> --
>> 1.7.9.5
>>
>>
>> --
>> kernel-team mailing list
>> kernel-team@lists.ubuntu.com
>> https://lists.ubuntu.com/mailman/listinfo/kernel-team
Luis Henriques March 23, 2015, 2:35 p.m. UTC | #6
Applied to Trusty, with all the changes referred by Stefan:

- include the "cherry picked from commit..." line
- include Ming Lei s-o-b line
- sanitized buglink line

Cheers,
--
Luís

Patch
diff mbox

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;