| Message ID | 1426045819-24485-1-git-send-email-ming.lei@canonical.com |
|---|---|
| State | New |
| Headers | show |
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; >
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; >> > >
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; >
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
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
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
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;