[PATCH 0/7] ext4: Convert truncated extent data to inline data.

[PATCH 0/7] ext4: Convert truncated extent data to inline data.

[Julian Sun]

Ext4 provides the feature of storing data inline and automatically 
converts it to extent data when appropriate. However, files stored 
as extents cannot be converted back to inline data after truncation, 
even if the file size allows for inline data storage. 
This patch set implements the feature to store large truncated files 
as inline data when suitable, improving disk utilization. 
Patches 1-3 include some cleanups and fixes. 
Patches 4-6 refactor the functions responsible for writing inline data, 
consolidating their logic for better code organization.
Patch 7 implements the feature of storing truncated files as inline data 
on the next write operation. 

Below is a comparison of results before and after applying the patch set. 

Before:
root@q:linux# dd if=/dev/urandom bs=1M count=10 of=/mnt/ext4/test
10+0 records in
10+0 records out
10485760 bytes (10 MB, 10 MiB) copied, 0.0770325 s, 136 MB/s
root@q:linux# filefrag -v /mnt/ext4/test
Filesystem type is: ef53
File size of /mnt/ext4/test is 10485760 (2560 blocks of 4096 bytes)
 ext:     logical_offset:        physical_offset: length:   expected: flags:
   0:        0..    2559:          0..         0:      0:             last,unknown_loc,delalloc,eof
/mnt/ext4/test: 1 extent found
root@q:linux# echo a > /mnt/ext4/test
root@q:linux# filefrag -v /mnt/ext4/test
Filesystem type is: ef53
File size of /mnt/ext4/test is 2 (1 block of 4096 bytes)
 ext:     logical_offset:        physical_offset: length:   expected: flags:
   0:        0..       0:      34304..     34304:      1:             last,eof
/mnt/ext4/test: 1 extent found

After:
root@q:linux# dd if=/dev/urandom bs=1M count=10 of=/mnt/ext4/test
10+0 records in
10+0 records out
10485760 bytes (10 MB, 10 MiB) copied, 0.0883107 s, 119 MB/s
root@q:linux# filefrag -v /mnt/ext4/test
Filesystem type is: ef53
File size of /mnt/ext4/test is 10485760 (2560 blocks of 4096 bytes)
 ext:     logical_offset:        physical_offset: length:   expected: flags:
   0:        0..    2559:      38912..     41471:   2560:             last,unknown_loc,delalloc,eof
/mnt/ext4/test: 1 extent found
root@q:linux# echo a > /mnt/ext4/test
root@q:linux# filefrag -v /mnt/ext4/test
Filesystem type is: ef53
Filesystem cylinder groups approximately 78
File size of /mnt/ext4/test is 2 (1 block of 4096 bytes)
 ext:     logical_offset:        physical_offset: length:   expected: flags:
   0:        0..       1:    4340520..   4340521:      2:             last,not_aligned,inline,eof
/mnt/ext4/test: 1 extent found

Using filefrag, we can see that after applying this patch,
large truncated files also utilize the inline data feature.
This patch set has been tested with xfstests' check -g and has not 
introduced any additional failures.



Julian Sun (7):
  ext4: Modify ei->i_flags before calling ext4_mark_iloc_dirty()
  ext4: Remove a redundant return statement
  ext4: Don't set EXT4_STATE_MAY_INLINE_DATA for ea inodes
  ext4: Introduce a new helper function ext4_generic_write_inline_data()
  ext4: Refactor out ext4_da_write_inline_data_begin()
  ext4: Refactor out ext4_try_to_write_inline_data()
  ext4: Store truncated large files as inline data.

 fs/ext4/extents_status.c |   1 -
 fs/ext4/ialloc.c         |   2 +-
 fs/ext4/inline.c         | 205 ++++++++++++++++-----------------------
 fs/ext4/inode.c          |   5 +
 4 files changed, 91 insertions(+), 122 deletions(-)

-- 
2.39.5

[PATCH 1/7] ext4: Modify ei->i_flags before calling ext4_mark_iloc_dirty()

[Julian Sun]

Modify ei->i_flags before calling ext4_mark_iloc_dirty() so that
the modifications to ei->i_flags can be reflected in the raw_inode
during the call to ext4_mark_iloc_dirty()->ext4_do_update_inode()

Signed-off-by: Julian Sun <sunjunchao2870@gmail.com>
---
 fs/ext4/inline.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
index 3536ca7e4fcc..d479495d03aa 100644
--- a/fs/ext4/inline.c
+++ b/fs/ext4/inline.c
@@ -465,11 +465,10 @@ static int ext4_destroy_inline_data_nolock(handle_t *handle,
 	ext4_clear_inode_flag(inode, EXT4_INODE_INLINE_DATA);
 
 	get_bh(is.iloc.bh);
-	error = ext4_mark_iloc_dirty(handle, inode, &is.iloc);
-
 	EXT4_I(inode)->i_inline_off = 0;
 	EXT4_I(inode)->i_inline_size = 0;
 	ext4_clear_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA);
+	error = ext4_mark_iloc_dirty(handle, inode, &is.iloc);
 out:
 	brelse(is.iloc.bh);
 	if (error == -ENODATA)
-- 
2.39.5

Re: [PATCH 1/7] ext4: Modify ei->i_flags before calling ext4_mark_iloc_dirty()

[Jan Kara]

On Fri 20-12-24 23:16:19, Julian Sun wrote:
> Modify ei->i_flags before calling ext4_mark_iloc_dirty() so that
> the modifications to ei->i_flags can be reflected in the raw_inode
> during the call to ext4_mark_iloc_dirty()->ext4_do_update_inode()
> 
> Signed-off-by: Julian Sun <sunjunchao2870@gmail.com>
> ---
>  fs/ext4/inline.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
> index 3536ca7e4fcc..d479495d03aa 100644
> --- a/fs/ext4/inline.c
> +++ b/fs/ext4/inline.c
> @@ -465,11 +465,10 @@ static int ext4_destroy_inline_data_nolock(handle_t *handle,
>  	ext4_clear_inode_flag(inode, EXT4_INODE_INLINE_DATA);
>  
>  	get_bh(is.iloc.bh);
> -	error = ext4_mark_iloc_dirty(handle, inode, &is.iloc);
> -
>  	EXT4_I(inode)->i_inline_off = 0;
>  	EXT4_I(inode)->i_inline_size = 0;
>  	ext4_clear_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA);
> +	error = ext4_mark_iloc_dirty(handle, inode, &is.iloc);

I don't see what difference this makes since ext4_clear_inode_state() does
not modify ei->i_flags but ei->i_state_flags which is not stored on disk...

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 1/7] ext4: Modify ei->i_flags before calling ext4_mark_iloc_dirty()

[Julian Sun]

Jan Kara <jack@suse.cz> 于2025年1月6日周一 23:25写道：
>
> On Fri 20-12-24 23:16:19, Julian Sun wrote:
> > Modify ei->i_flags before calling ext4_mark_iloc_dirty() so that
> > the modifications to ei->i_flags can be reflected in the raw_inode
> > during the call to ext4_mark_iloc_dirty()->ext4_do_update_inode()
> >
> > Signed-off-by: Julian Sun <sunjunchao2870@gmail.com>
> > ---
> >  fs/ext4/inline.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
> > index 3536ca7e4fcc..d479495d03aa 100644
> > --- a/fs/ext4/inline.c
> > +++ b/fs/ext4/inline.c
> > @@ -465,11 +465,10 @@ static int ext4_destroy_inline_data_nolock(handle_t *handle,
> >       ext4_clear_inode_flag(inode, EXT4_INODE_INLINE_DATA);
> >
> >       get_bh(is.iloc.bh);
> > -     error = ext4_mark_iloc_dirty(handle, inode, &is.iloc);
> > -
> >       EXT4_I(inode)->i_inline_off = 0;
> >       EXT4_I(inode)->i_inline_size = 0;
> >       ext4_clear_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA);
> > +     error = ext4_mark_iloc_dirty(handle, inode, &is.iloc);
>
> I don't see what difference this makes since ext4_clear_inode_state() does
> not modify ei->i_flags but ei->i_state_flags which is not stored on disk...

Yes, you are correct. On my machine, BITS_PER_LONG is 64, so
ext4_clear_inode_state() modifies the high 32 bits of ei->i_flags
which are also not stored on disk.
>
>                                                                 Honza
> --
> Jan Kara <jack@suse.com>
> SUSE Labs, CR


Thanks,
-- 
Julian Sun <sunjunchao2870@gmail.com>

[PATCH 2/7] ext4: Remove a redundant return statement

[Julian Sun]

Remove a redundant return statements in the
ext4_es_remove_extent() function.

Signed-off-by: Julian Sun <sunjunchao2870@gmail.com>
---
 fs/ext4/extents_status.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/fs/ext4/extents_status.c b/fs/ext4/extents_status.c
index c786691dabd3..c56fb682a27e 100644
--- a/fs/ext4/extents_status.c
+++ b/fs/ext4/extents_status.c
@@ -1551,7 +1551,6 @@ void ext4_es_remove_extent(struct inode *inode, ext4_lblk_t lblk,
 
 	ext4_es_print_tree(inode);
 	ext4_da_release_space(inode, reserved);
-	return;
 }
 
 static int __es_shrink(struct ext4_sb_info *sbi, int nr_to_scan,
-- 
2.39.5

Re: [PATCH 2/7] ext4: Remove a redundant return statement

[Jan Kara]

On Fri 20-12-24 23:16:20, Julian Sun wrote:
> Remove a redundant return statements in the
> ext4_es_remove_extent() function.
> 
> Signed-off-by: Julian Sun <sunjunchao2870@gmail.com>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/ext4/extents_status.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/fs/ext4/extents_status.c b/fs/ext4/extents_status.c
> index c786691dabd3..c56fb682a27e 100644
> --- a/fs/ext4/extents_status.c
> +++ b/fs/ext4/extents_status.c
> @@ -1551,7 +1551,6 @@ void ext4_es_remove_extent(struct inode *inode, ext4_lblk_t lblk,
>  
>  	ext4_es_print_tree(inode);
>  	ext4_da_release_space(inode, reserved);
> -	return;
>  }
>  
>  static int __es_shrink(struct ext4_sb_info *sbi, int nr_to_scan,
> -- 
> 2.39.5
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

[PATCH 3/7] ext4: Don't set EXT4_STATE_MAY_INLINE_DATA for ea inodes

[Julian Sun]

Setting the EXT4_STATE_MAY_INLINE_DATA flag for ea inodes
is meaningless because ea inodes do not use functions
like ext4_write_begin().

Signed-off-by: Julian Sun <sunjunchao2870@gmail.com>
---
 fs/ext4/ialloc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
index 7f1a5f90dbbd..49b112bfbd93 100644
--- a/fs/ext4/ialloc.c
+++ b/fs/ext4/ialloc.c
@@ -1297,7 +1297,7 @@ struct inode *__ext4_new_inode(struct mnt_idmap *idmap,
 	ei->i_extra_isize = sbi->s_want_extra_isize;
 	ei->i_inline_off = 0;
 	if (ext4_has_feature_inline_data(sb) &&
-	    (!(ei->i_flags & EXT4_DAX_FL) || S_ISDIR(mode)))
+	    (!(ei->i_flags & (EXT4_DAX_FL|EXT4_EA_INODE_FL)) || S_ISDIR(mode)))
 		ext4_set_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA);
 	ret = inode;
 	err = dquot_alloc_inode(inode);
-- 
2.39.5

Re: [PATCH 3/7] ext4: Don't set EXT4_STATE_MAY_INLINE_DATA for ea inodes

[Jan Kara]

On Fri 20-12-24 23:16:21, Julian Sun wrote:
> Setting the EXT4_STATE_MAY_INLINE_DATA flag for ea inodes
> is meaningless because ea inodes do not use functions
> like ext4_write_begin().
> 
> Signed-off-by: Julian Sun <sunjunchao2870@gmail.com>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/ext4/ialloc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
> index 7f1a5f90dbbd..49b112bfbd93 100644
> --- a/fs/ext4/ialloc.c
> +++ b/fs/ext4/ialloc.c
> @@ -1297,7 +1297,7 @@ struct inode *__ext4_new_inode(struct mnt_idmap *idmap,
>  	ei->i_extra_isize = sbi->s_want_extra_isize;
>  	ei->i_inline_off = 0;
>  	if (ext4_has_feature_inline_data(sb) &&
> -	    (!(ei->i_flags & EXT4_DAX_FL) || S_ISDIR(mode)))
> +	    (!(ei->i_flags & (EXT4_DAX_FL|EXT4_EA_INODE_FL)) || S_ISDIR(mode)))
>  		ext4_set_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA);
>  	ret = inode;
>  	err = dquot_alloc_inode(inode);
> -- 
> 2.39.5
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

[PATCH 4/7] ext4: Introduce a new helper function ext4_generic_write_inline_data()

[Julian Sun]

A new function, ext4_generic_write_inline_data(), is introduced
to provide a generic implementation of the common logic found in
ext4_da_write_inline_data_begin() and ext4_try_to_write_inline_data().

This function will be utilized in the subsequent two patches.

Signed-off-by: Julian Sun <sunjunchao2870@gmail.com>
---
 fs/ext4/inline.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 85 insertions(+)

diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
index d479495d03aa..7eaa578e1021 100644
--- a/fs/ext4/inline.c
+++ b/fs/ext4/inline.c
@@ -20,6 +20,11 @@
 #define EXT4_INLINE_DOTDOT_OFFSET	2
 #define EXT4_INLINE_DOTDOT_SIZE		4
 
+
+static int ext4_da_convert_inline_data_to_extent(struct address_space *mapping,
+						 struct inode *inode,
+						 void **fsdata);
+
 static int ext4_get_inline_size(struct inode *inode)
 {
 	if (EXT4_I(inode)->i_inline_off)
@@ -651,6 +656,86 @@ static int ext4_convert_inline_data_to_extent(struct address_space *mapping,
 	return ret;
 }
 
+static int ext4_generic_write_inline_data(struct address_space *mapping,
+					  struct inode *inode,
+					  loff_t pos, unsigned len,
+					  struct folio **foliop,
+					  void **fsdata, bool da)
+{
+	int ret;
+	handle_t *handle;
+	struct folio *folio;
+	struct ext4_iloc iloc;
+	int retries = 0;
+
+	ret = ext4_get_inode_loc(inode, &iloc);
+	if (ret)
+		return ret;
+
+retry_journal:
+	handle = ext4_journal_start(inode, EXT4_HT_INODE, 1);
+	if (IS_ERR(handle)) {
+		ret = PTR_ERR(handle);
+		goto out_release_bh;
+	}
+
+	ret = ext4_prepare_inline_data(handle, inode, pos + len);
+	if (ret && ret != -ENOSPC)
+		goto out_stop_journal;
+
+	if (ret == -ENOSPC) {
+		ext4_journal_stop(handle);
+		if (!da) {
+			brelse(iloc.bh);
+			/* Retry inside */
+			return ext4_convert_inline_data_to_extent(mapping, inode);
+		}
+
+		ret = ext4_da_convert_inline_data_to_extent(mapping, inode, fsdata);
+		if (ret == -ENOSPC &&
+		    ext4_should_retry_alloc(inode->i_sb, &retries))
+			goto retry_journal;
+		goto out_release_bh;
+	}
+
+	folio = __filemap_get_folio(mapping, 0, FGP_WRITEBEGIN | FGP_NOFS,
+					mapping_gfp_mask(mapping));
+	if (IS_ERR(folio)) {
+		ret = PTR_ERR(folio);
+		goto out_stop_journal;
+	}
+
+	down_read(&EXT4_I(inode)->xattr_sem);
+	/* Someone else had converted it to extent */
+	if (!ext4_has_inline_data(inode)) {
+		ret = 0;
+		goto out_release_folio;
+	}
+
+	if (!folio_test_uptodate(folio)) {
+		ret = ext4_read_inline_folio(inode, folio);
+		if (ret < 0)
+			goto out_release_folio;
+	}
+
+	ret = ext4_journal_get_write_access(handle, inode->i_sb, iloc.bh, EXT4_JTR_NONE);
+	if (ret)
+		goto out_release_folio;
+	*foliop = folio;
+	up_read(&EXT4_I(inode)->xattr_sem);
+	brelse(iloc.bh);
+	return 1;
+out_release_folio:
+	up_read(&EXT4_I(inode)->xattr_sem);
+	folio_unlock(folio);
+	folio_put(folio);
+out_stop_journal:
+	ext4_journal_stop(handle);
+out_release_bh:
+	brelse(iloc.bh);
+	return ret;
+}
+
 /*
  * Try to write data in the inode.
  * If the inode has inline data, check whether the new write can be
-- 
2.39.5

Re: [PATCH 4/7] ext4: Introduce a new helper function ext4_generic_write_inline_data()

[Jan Kara]

On Fri 20-12-24 23:16:22, Julian Sun wrote:
> A new function, ext4_generic_write_inline_data(), is introduced
> to provide a generic implementation of the common logic found in
> ext4_da_write_inline_data_begin() and ext4_try_to_write_inline_data().
> 
> This function will be utilized in the subsequent two patches.
> 
> Signed-off-by: Julian Sun <sunjunchao2870@gmail.com>

Looks good, just one style nit below. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

...
> +	*foliop = folio;
> +	up_read(&EXT4_I(inode)->xattr_sem);
> +	brelse(iloc.bh);
> +	return 1;

Here I'd suggest empty line for better readability.

> +out_release_folio:
> +	up_read(&EXT4_I(inode)->xattr_sem);
> +	folio_unlock(folio);
> +	folio_put(folio);
> +out_stop_journal:
> +	ext4_journal_stop(handle);
> +out_release_bh:
> +	brelse(iloc.bh);
> +	return ret;
> +}
> +

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 4/7] ext4: Introduce a new helper function ext4_generic_write_inline_data()

[Julian Sun]

Jan Kara <jack@suse.cz> 于2025年1月6日周一 23:35写道：
>
> On Fri 20-12-24 23:16:22, Julian Sun wrote:
> > A new function, ext4_generic_write_inline_data(), is introduced
> > to provide a generic implementation of the common logic found in
> > ext4_da_write_inline_data_begin() and ext4_try_to_write_inline_data().
> >
> > This function will be utilized in the subsequent two patches.
> >
> > Signed-off-by: Julian Sun <sunjunchao2870@gmail.com>
>
> Looks good, just one style nit below. Feel free to add:
>
> Reviewed-by: Jan Kara <jack@suse.cz>
>
> ...
> > +     *foliop = folio;
> > +     up_read(&EXT4_I(inode)->xattr_sem);
> > +     brelse(iloc.bh);
> > +     return 1;
>
> Here I'd suggest empty line for better readability.

I will fix it in the next version. thanks!
>
> > +out_release_folio:
> > +     up_read(&EXT4_I(inode)->xattr_sem);
> > +     folio_unlock(folio);
> > +     folio_put(folio);
> > +out_stop_journal:
> > +     ext4_journal_stop(handle);
> > +out_release_bh:
> > +     brelse(iloc.bh);
> > +     return ret;
> > +}
> > +
>
>                                                                 Honza
> --
> Jan Kara <jack@suse.com>
> SUSE Labs, CR



-- 
Julian Sun <sunjunchao2870@gmail.com>

[PATCH 5/7] ext4: Refactor out ext4_da_write_inline_data_begin()

[Julian Sun]

Refactor ext4_da_write_inline_data_begin() to simplify its
implementation by directly invoking ext4_generic_write_inline_data().

Signed-off-by: Julian Sun <sunjunchao2870@gmail.com>
---
 fs/ext4/inline.c | 74 ++----------------------------------------------
 1 file changed, 2 insertions(+), 72 deletions(-)

diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
index 7eaa578e1021..5dd91524b2ca 100644
--- a/fs/ext4/inline.c
+++ b/fs/ext4/inline.c
@@ -979,78 +979,8 @@ int ext4_da_write_inline_data_begin(struct address_space *mapping,
 				    struct folio **foliop,
 				    void **fsdata)
 {
-	int ret;
-	handle_t *handle;
-	struct folio *folio;
-	struct ext4_iloc iloc;
-	int retries = 0;
-
-	ret = ext4_get_inode_loc(inode, &iloc);
-	if (ret)
-		return ret;
-
-retry_journal:
-	handle = ext4_journal_start(inode, EXT4_HT_INODE, 1);
-	if (IS_ERR(handle)) {
-		ret = PTR_ERR(handle);
-		goto out;
-	}
-
-	ret = ext4_prepare_inline_data(handle, inode, pos + len);
-	if (ret && ret != -ENOSPC)
-		goto out_journal;
-
-	if (ret == -ENOSPC) {
-		ext4_journal_stop(handle);
-		ret = ext4_da_convert_inline_data_to_extent(mapping,
-							    inode,
-							    fsdata);
-		if (ret == -ENOSPC &&
-		    ext4_should_retry_alloc(inode->i_sb, &retries))
-			goto retry_journal;
-		goto out;
-	}
-
-	/*
-	 * We cannot recurse into the filesystem as the transaction
-	 * is already started.
-	 */
-	folio = __filemap_get_folio(mapping, 0, FGP_WRITEBEGIN | FGP_NOFS,
-					mapping_gfp_mask(mapping));
-	if (IS_ERR(folio)) {
-		ret = PTR_ERR(folio);
-		goto out_journal;
-	}
-
-	down_read(&EXT4_I(inode)->xattr_sem);
-	if (!ext4_has_inline_data(inode)) {
-		ret = 0;
-		goto out_release_page;
-	}
-
-	if (!folio_test_uptodate(folio)) {
-		ret = ext4_read_inline_folio(inode, folio);
-		if (ret < 0)
-			goto out_release_page;
-	}
-	ret = ext4_journal_get_write_access(handle, inode->i_sb, iloc.bh,
-					    EXT4_JTR_NONE);
-	if (ret)
-		goto out_release_page;
-
-	up_read(&EXT4_I(inode)->xattr_sem);
-	*foliop = folio;
-	brelse(iloc.bh);
-	return 1;
-out_release_page:
-	up_read(&EXT4_I(inode)->xattr_sem);
-	folio_unlock(folio);
-	folio_put(folio);
-out_journal:
-	ext4_journal_stop(handle);
-out:
-	brelse(iloc.bh);
-	return ret;
+	return ext4_generic_write_inline_data(mapping, inode, pos, len,
+					      foliop, fsdata, true);
 }
 
 #ifdef INLINE_DIR_DEBUG
-- 
2.39.5

Re: [PATCH 5/7] ext4: Refactor out ext4_da_write_inline_data_begin()

[Jan Kara]

On Fri 20-12-24 23:16:23, Julian Sun wrote:
> Refactor ext4_da_write_inline_data_begin() to simplify its
> implementation by directly invoking ext4_generic_write_inline_data().
> 
> Signed-off-by: Julian Sun <sunjunchao2870@gmail.com>
> ---
>  fs/ext4/inline.c | 74 ++----------------------------------------------
>  1 file changed, 2 insertions(+), 72 deletions(-)
> 
> diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
> index 7eaa578e1021..5dd91524b2ca 100644
> --- a/fs/ext4/inline.c
> +++ b/fs/ext4/inline.c
> @@ -979,78 +979,8 @@ int ext4_da_write_inline_data_begin(struct address_space *mapping,
>  				    struct folio **foliop,
>  				    void **fsdata)
>  {

Since there's a single caller of this function, I guess there's no point
for this wrapper now. Just call ext4_generic_write_inline_data() directly
from the call site. Otherwise the patch looks good.

								Honza

> -	int ret;
> -	handle_t *handle;
> -	struct folio *folio;
> -	struct ext4_iloc iloc;
> -	int retries = 0;
> -
> -	ret = ext4_get_inode_loc(inode, &iloc);
> -	if (ret)
> -		return ret;
> -
> -retry_journal:
> -	handle = ext4_journal_start(inode, EXT4_HT_INODE, 1);
> -	if (IS_ERR(handle)) {
> -		ret = PTR_ERR(handle);
> -		goto out;
> -	}
> -
> -	ret = ext4_prepare_inline_data(handle, inode, pos + len);
> -	if (ret && ret != -ENOSPC)
> -		goto out_journal;
> -
> -	if (ret == -ENOSPC) {
> -		ext4_journal_stop(handle);
> -		ret = ext4_da_convert_inline_data_to_extent(mapping,
> -							    inode,
> -							    fsdata);
> -		if (ret == -ENOSPC &&
> -		    ext4_should_retry_alloc(inode->i_sb, &retries))
> -			goto retry_journal;
> -		goto out;
> -	}
> -
> -	/*
> -	 * We cannot recurse into the filesystem as the transaction
> -	 * is already started.
> -	 */
> -	folio = __filemap_get_folio(mapping, 0, FGP_WRITEBEGIN | FGP_NOFS,
> -					mapping_gfp_mask(mapping));
> -	if (IS_ERR(folio)) {
> -		ret = PTR_ERR(folio);
> -		goto out_journal;
> -	}
> -
> -	down_read(&EXT4_I(inode)->xattr_sem);
> -	if (!ext4_has_inline_data(inode)) {
> -		ret = 0;
> -		goto out_release_page;
> -	}
> -
> -	if (!folio_test_uptodate(folio)) {
> -		ret = ext4_read_inline_folio(inode, folio);
> -		if (ret < 0)
> -			goto out_release_page;
> -	}
> -	ret = ext4_journal_get_write_access(handle, inode->i_sb, iloc.bh,
> -					    EXT4_JTR_NONE);
> -	if (ret)
> -		goto out_release_page;
> -
> -	up_read(&EXT4_I(inode)->xattr_sem);
> -	*foliop = folio;
> -	brelse(iloc.bh);
> -	return 1;
> -out_release_page:
> -	up_read(&EXT4_I(inode)->xattr_sem);
> -	folio_unlock(folio);
> -	folio_put(folio);
> -out_journal:
> -	ext4_journal_stop(handle);
> -out:
> -	brelse(iloc.bh);
> -	return ret;
> +	return ext4_generic_write_inline_data(mapping, inode, pos, len,
> +					      foliop, fsdata, true);
>  }
>  
>  #ifdef INLINE_DIR_DEBUG
> -- 
> 2.39.5
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 5/7] ext4: Refactor out ext4_da_write_inline_data_begin()

[Julian Sun]

Jan Kara <jack@suse.cz> 于2025年1月6日周一 23:39写道：
>
> On Fri 20-12-24 23:16:23, Julian Sun wrote:
> > Refactor ext4_da_write_inline_data_begin() to simplify its
> > implementation by directly invoking ext4_generic_write_inline_data().
> >
> > Signed-off-by: Julian Sun <sunjunchao2870@gmail.com>
> > ---
> >  fs/ext4/inline.c | 74 ++----------------------------------------------
> >  1 file changed, 2 insertions(+), 72 deletions(-)
> >
> > diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
> > index 7eaa578e1021..5dd91524b2ca 100644
> > --- a/fs/ext4/inline.c
> > +++ b/fs/ext4/inline.c
> > @@ -979,78 +979,8 @@ int ext4_da_write_inline_data_begin(struct address_space *mapping,
> >                                   struct folio **foliop,
> >                                   void **fsdata)
> >  {
>
> Since there's a single caller of this function, I guess there's no point
> for this wrapper now. Just call ext4_generic_write_inline_data() directly
> from the call site. Otherwise the patch looks good.

Reasonable. I will fix it in the next version. Thanks!
>
>                                                                 Honza
>
> > -     int ret;
> > -     handle_t *handle;
> > -     struct folio *folio;
> > -     struct ext4_iloc iloc;
> > -     int retries = 0;
> > -
> > -     ret = ext4_get_inode_loc(inode, &iloc);
> > -     if (ret)
> > -             return ret;
> > -
> > -retry_journal:
> > -     handle = ext4_journal_start(inode, EXT4_HT_INODE, 1);
> > -     if (IS_ERR(handle)) {
> > -             ret = PTR_ERR(handle);
> > -             goto out;
> > -     }
> > -
> > -     ret = ext4_prepare_inline_data(handle, inode, pos + len);
> > -     if (ret && ret != -ENOSPC)
> > -             goto out_journal;
> > -
> > -     if (ret == -ENOSPC) {
> > -             ext4_journal_stop(handle);
> > -             ret = ext4_da_convert_inline_data_to_extent(mapping,
> > -                                                         inode,
> > -                                                         fsdata);
> > -             if (ret == -ENOSPC &&
> > -                 ext4_should_retry_alloc(inode->i_sb, &retries))
> > -                     goto retry_journal;
> > -             goto out;
> > -     }
> > -
> > -     /*
> > -      * We cannot recurse into the filesystem as the transaction
> > -      * is already started.
> > -      */
> > -     folio = __filemap_get_folio(mapping, 0, FGP_WRITEBEGIN | FGP_NOFS,
> > -                                     mapping_gfp_mask(mapping));
> > -     if (IS_ERR(folio)) {
> > -             ret = PTR_ERR(folio);
> > -             goto out_journal;
> > -     }
> > -
> > -     down_read(&EXT4_I(inode)->xattr_sem);
> > -     if (!ext4_has_inline_data(inode)) {
> > -             ret = 0;
> > -             goto out_release_page;
> > -     }
> > -
> > -     if (!folio_test_uptodate(folio)) {
> > -             ret = ext4_read_inline_folio(inode, folio);
> > -             if (ret < 0)
> > -                     goto out_release_page;
> > -     }
> > -     ret = ext4_journal_get_write_access(handle, inode->i_sb, iloc.bh,
> > -                                         EXT4_JTR_NONE);
> > -     if (ret)
> > -             goto out_release_page;
> > -
> > -     up_read(&EXT4_I(inode)->xattr_sem);
> > -     *foliop = folio;
> > -     brelse(iloc.bh);
> > -     return 1;
> > -out_release_page:
> > -     up_read(&EXT4_I(inode)->xattr_sem);
> > -     folio_unlock(folio);
> > -     folio_put(folio);
> > -out_journal:
> > -     ext4_journal_stop(handle);
> > -out:
> > -     brelse(iloc.bh);
> > -     return ret;
> > +     return ext4_generic_write_inline_data(mapping, inode, pos, len,
> > +                                           foliop, fsdata, true);
> >  }
> >
> >  #ifdef INLINE_DIR_DEBUG
> > --
> > 2.39.5
> >
> --
> Jan Kara <jack@suse.com>
> SUSE Labs, CR



-- 
Julian Sun <sunjunchao2870@gmail.com>

[PATCH 6/7] ext4: Refactor out ext4_try_to_write_inline_data()

[Julian Sun]

Refactor ext4_try_to_write_inline_data() to simplify its
implementation by directly invoking ext4_generic_write_inline_data().

Signed-off-by: Julian Sun <sunjunchao2870@gmail.com>
---
 fs/ext4/inline.c | 77 ++----------------------------------------------
 1 file changed, 3 insertions(+), 74 deletions(-)

diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
index 5dd91524b2ca..2abb35f1555d 100644
--- a/fs/ext4/inline.c
+++ b/fs/ext4/inline.c
@@ -747,81 +747,10 @@ int ext4_try_to_write_inline_data(struct address_space *mapping,
 				  loff_t pos, unsigned len,
 				  struct folio **foliop)
 {
-	int ret;
-	handle_t *handle;
-	struct folio *folio;
-	struct ext4_iloc iloc;
-
 	if (pos + len > ext4_get_max_inline_size(inode))
-		goto convert;
-
-	ret = ext4_get_inode_loc(inode, &iloc);
-	if (ret)
-		return ret;
-
-	/*
-	 * The possible write could happen in the inode,
-	 * so try to reserve the space in inode first.
-	 */
-	handle = ext4_journal_start(inode, EXT4_HT_INODE, 1);
-	if (IS_ERR(handle)) {
-		ret = PTR_ERR(handle);
-		handle = NULL;
-		goto out;
-	}
-
-	ret = ext4_prepare_inline_data(handle, inode, pos + len);
-	if (ret && ret != -ENOSPC)
-		goto out;
-
-	/* We don't have space in inline inode, so convert it to extent. */
-	if (ret == -ENOSPC) {
-		ext4_journal_stop(handle);
-		brelse(iloc.bh);
-		goto convert;
-	}
-
-	ret = ext4_journal_get_write_access(handle, inode->i_sb, iloc.bh,
-					    EXT4_JTR_NONE);
-	if (ret)
-		goto out;
-
-	folio = __filemap_get_folio(mapping, 0, FGP_WRITEBEGIN | FGP_NOFS,
-					mapping_gfp_mask(mapping));
-	if (IS_ERR(folio)) {
-		ret = PTR_ERR(folio);
-		goto out;
-	}
-
-	*foliop = folio;
-	down_read(&EXT4_I(inode)->xattr_sem);
-	if (!ext4_has_inline_data(inode)) {
-		ret = 0;
-		folio_unlock(folio);
-		folio_put(folio);
-		goto out_up_read;
-	}
-
-	if (!folio_test_uptodate(folio)) {
-		ret = ext4_read_inline_folio(inode, folio);
-		if (ret < 0) {
-			folio_unlock(folio);
-			folio_put(folio);
-			goto out_up_read;
-		}
-	}
-
-	ret = 1;
-	handle = NULL;
-out_up_read:
-	up_read(&EXT4_I(inode)->xattr_sem);
-out:
-	if (handle && (ret != 1))
-		ext4_journal_stop(handle);
-	brelse(iloc.bh);
-	return ret;
-convert:
-	return ext4_convert_inline_data_to_extent(mapping, inode);
+		return ext4_convert_inline_data_to_extent(mapping, inode);
+	return ext4_generic_write_inline_data(mapping, inode, pos, len,
+					      foliop, NULL, false);
 }
 
 int ext4_write_inline_data_end(struct inode *inode, loff_t pos, unsigned len,
-- 
2.39.5

Re: [PATCH 6/7] ext4: Refactor out ext4_try_to_write_inline_data()

[Jan Kara]

On Fri 20-12-24 23:16:24, Julian Sun wrote:
> Refactor ext4_try_to_write_inline_data() to simplify its
> implementation by directly invoking ext4_generic_write_inline_data().
> 
> Signed-off-by: Julian Sun <sunjunchao2870@gmail.com>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/ext4/inline.c | 77 ++----------------------------------------------
>  1 file changed, 3 insertions(+), 74 deletions(-)
> 
> diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
> index 5dd91524b2ca..2abb35f1555d 100644
> --- a/fs/ext4/inline.c
> +++ b/fs/ext4/inline.c
> @@ -747,81 +747,10 @@ int ext4_try_to_write_inline_data(struct address_space *mapping,
>  				  loff_t pos, unsigned len,
>  				  struct folio **foliop)
>  {
> -	int ret;
> -	handle_t *handle;
> -	struct folio *folio;
> -	struct ext4_iloc iloc;
> -
>  	if (pos + len > ext4_get_max_inline_size(inode))
> -		goto convert;
> -
> -	ret = ext4_get_inode_loc(inode, &iloc);
> -	if (ret)
> -		return ret;
> -
> -	/*
> -	 * The possible write could happen in the inode,
> -	 * so try to reserve the space in inode first.
> -	 */
> -	handle = ext4_journal_start(inode, EXT4_HT_INODE, 1);
> -	if (IS_ERR(handle)) {
> -		ret = PTR_ERR(handle);
> -		handle = NULL;
> -		goto out;
> -	}
> -
> -	ret = ext4_prepare_inline_data(handle, inode, pos + len);
> -	if (ret && ret != -ENOSPC)
> -		goto out;
> -
> -	/* We don't have space in inline inode, so convert it to extent. */
> -	if (ret == -ENOSPC) {
> -		ext4_journal_stop(handle);
> -		brelse(iloc.bh);
> -		goto convert;
> -	}
> -
> -	ret = ext4_journal_get_write_access(handle, inode->i_sb, iloc.bh,
> -					    EXT4_JTR_NONE);
> -	if (ret)
> -		goto out;
> -
> -	folio = __filemap_get_folio(mapping, 0, FGP_WRITEBEGIN | FGP_NOFS,
> -					mapping_gfp_mask(mapping));
> -	if (IS_ERR(folio)) {
> -		ret = PTR_ERR(folio);
> -		goto out;
> -	}
> -
> -	*foliop = folio;
> -	down_read(&EXT4_I(inode)->xattr_sem);
> -	if (!ext4_has_inline_data(inode)) {
> -		ret = 0;
> -		folio_unlock(folio);
> -		folio_put(folio);
> -		goto out_up_read;
> -	}
> -
> -	if (!folio_test_uptodate(folio)) {
> -		ret = ext4_read_inline_folio(inode, folio);
> -		if (ret < 0) {
> -			folio_unlock(folio);
> -			folio_put(folio);
> -			goto out_up_read;
> -		}
> -	}
> -
> -	ret = 1;
> -	handle = NULL;
> -out_up_read:
> -	up_read(&EXT4_I(inode)->xattr_sem);
> -out:
> -	if (handle && (ret != 1))
> -		ext4_journal_stop(handle);
> -	brelse(iloc.bh);
> -	return ret;
> -convert:
> -	return ext4_convert_inline_data_to_extent(mapping, inode);
> +		return ext4_convert_inline_data_to_extent(mapping, inode);
> +	return ext4_generic_write_inline_data(mapping, inode, pos, len,
> +					      foliop, NULL, false);
>  }
>  
>  int ext4_write_inline_data_end(struct inode *inode, loff_t pos, unsigned len,
> -- 
> 2.39.5
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

[PATCH 7/7] ext4: Store truncated large files as inline data.

[Julian Sun]

Ext4 provides the flexibility to store file data inline, directly
within the inode, and also supports automatic conversion from inline
to extent data when necessary. However, it lacks a mechanism to convert
extent-based data to inline data, even if the file size permits.

This patch fills the gap by automatically converting truncated files
to inline data when possible, resulting in improved disk space efficiency.
Below is a comparison of results before and after applying the patch set.

Before:
root@q:linux# dd if=/dev/urandom bs=1M count=10 of=/mnt/ext4/test
10+0 records in
10+0 records out
10485760 bytes (10 MB, 10 MiB) copied, 0.0770325 s, 136 MB/s
root@q:linux# filefrag -v /mnt/ext4/test
Filesystem type is: ef53
File size of /mnt/ext4/test is 10485760 (2560 blocks of 4096 bytes)
 ext:     logical_offset:        physical_offset: length:   expected: flags:
   0:        0..    2559:          0..         0:      0:             last,unknown_loc,delalloc,eof
/mnt/ext4/test: 1 extent found
root@q:linux# echo a > /mnt/ext4/test
root@q:linux# filefrag -v /mnt/ext4/test
Filesystem type is: ef53
File size of /mnt/ext4/test is 2 (1 block of 4096 bytes)
 ext:     logical_offset:        physical_offset: length:   expected: flags:
   0:        0..       0:      34304..     34304:      1:             last,eof
/mnt/ext4/test: 1 extent found

After:
root@q:linux# dd if=/dev/urandom bs=1M count=10 of=/mnt/ext4/test
10+0 records in
10+0 records out
10485760 bytes (10 MB, 10 MiB) copied, 0.0883107 s, 119 MB/s
root@q:linux# filefrag -v /mnt/ext4/test
Filesystem type is: ef53
File size of /mnt/ext4/test is 10485760 (2560 blocks of 4096 bytes)
 ext:     logical_offset:        physical_offset: length:   expected: flags:
   0:        0..    2559:      38912..     41471:   2560:             last,unknown_loc,delalloc,eof
/mnt/ext4/test: 1 extent found
root@q:linux# echo a > /mnt/ext4/test
root@q:linux# filefrag -v /mnt/ext4/test
Filesystem type is: ef53
Filesystem cylinder groups approximately 78
File size of /mnt/ext4/test is 2 (1 block of 4096 bytes)
 ext:     logical_offset:        physical_offset: length:   expected: flags:
   0:        0..       1:    4340520..   4340521:      2:             last,not_aligned,inline,eof
/mnt/ext4/test: 1 extent found

Using filefrag, we can see that after applying this patch,
large truncated files also utilize the inline data feature.

This patch has been tested with xfstests' check -g and has not
introduced any additional failures.

Signed-off-by: Julian Sun <sunjunchao2870@gmail.com>
---
 fs/ext4/inline.c | 24 +++++++++++++++++++++++-
 fs/ext4/inode.c  |  5 +++++
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
index 2abb35f1555d..ff107f7ab936 100644
--- a/fs/ext4/inline.c
+++ b/fs/ext4/inline.c
@@ -667,13 +667,22 @@ static int ext4_generic_write_inline_data(struct address_space *mapping,
 	struct folio *folio;
 	struct ext4_iloc iloc;
 	int retries = 0;
+	bool none_inline_data = inode->i_blocks != 0;
+	int credits;
 
 	ret = ext4_get_inode_loc(inode, &iloc);
 	if (ret)
 		return ret;
 
 retry_journal:
-	handle = ext4_journal_start(inode, EXT4_HT_INODE, 1);
+	if (none_inline_data)
+		if (ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS))
+			credits = ext4_writepage_trans_blocks(inode);
+		else
+			credits = ext4_blocks_for_truncate(inode);
+	else
+		credits = 1;
+	handle = ext4_journal_start(inode, EXT4_HT_INODE, credits);
 	if (IS_ERR(handle)) {
 		ret = PTR_ERR(handle);
 		goto out_release_bh;
@@ -698,6 +707,19 @@ static int ext4_generic_write_inline_data(struct address_space *mapping,
 		goto out_release_bh;
 	}
 
+	if (none_inline_data) {
+		down_write(&EXT4_I(inode)->i_data_sem);
+		ext4_es_remove_extent(inode, 0, EXT_MAX_BLOCKS);
+
+		if (ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS))
+			ret = ext4_ext_remove_space(inode, 0,
+						    EXT_MAX_BLOCKS - 1);
+		else
+			ret = ext4_ind_remove_space(handle, inode, 0,
+						    EXT_MAX_BLOCKS);
+		up_write(&EXT4_I(inode)->i_data_sem);
+	}
+
 	folio = __filemap_get_folio(mapping, 0, FGP_WRITEBEGIN | FGP_NOFS,
 					mapping_gfp_mask(mapping));
 	if (IS_ERR(folio)) {
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 54bdd4884fe6..fb1e4caa37b0 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -4164,6 +4164,11 @@ int ext4_truncate(struct inode *inode)
 	if (inode->i_size & (inode->i_sb->s_blocksize - 1))
 		ext4_block_truncate_page(handle, mapping, inode->i_size);
 
+	if (ext4_has_feature_inline_data(inode->i_sb) &&
+	    !(ei->i_flags & (EXT4_EA_INODE_FL|EXT4_DAX_FL)) &&
+	    inode->i_size < ext4_get_max_inline_size(inode))
+		ext4_set_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA);
+
 	/*
 	 * We add the inode to the orphan list, so that if this
 	 * truncate spans multiple transactions, and we crash, we will
-- 
2.39.5

Re: [PATCH 0/7] ext4: Convert truncated extent data to inline data.

[Jan Kara]

On Fri 20-12-24 23:16:18, Julian Sun wrote:
> Ext4 provides the feature of storing data inline and automatically 
> converts it to extent data when appropriate. However, files stored 
> as extents cannot be converted back to inline data after truncation, 
> even if the file size allows for inline data storage. 
> This patch set implements the feature to store large truncated files 
> as inline data when suitable, improving disk utilization. 
> Patches 1-3 include some cleanups and fixes. 
> Patches 4-6 refactor the functions responsible for writing inline data, 
> consolidating their logic for better code organization.
> Patch 7 implements the feature of storing truncated files as inline data 
> on the next write operation. 

Thanks for the patches! So ext4 inline data feature is a bit problematic
and has some locking issues in the implementation which we didn't manage to
fix [1]. We are even considering just disabling this feature due to the
complications with it. Hence I don't quite like further complicating this
code by adding possibility to create inline data in an inode after it had
data blocks allocated. That being said I like the preparatory cleanups in
this patch series. These are useful regardless of the feature itself.

								Honza

[1] See our attempts in thread:
https://lore.kernel.org/all/d704ce55-321a-9c1d-1f8b-3360a0fdf978@huawei.com

-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 0/7] ext4: Convert truncated extent data to inline data.

[Julian Sun]

Jan Kara <jack@suse.cz> 于2025年1月6日周一 23:51写道：
>
> On Fri 20-12-24 23:16:18, Julian Sun wrote:
> > Ext4 provides the feature of storing data inline and automatically
> > converts it to extent data when appropriate. However, files stored
> > as extents cannot be converted back to inline data after truncation,
> > even if the file size allows for inline data storage.
> > This patch set implements the feature to store large truncated files
> > as inline data when suitable, improving disk utilization.
> > Patches 1-3 include some cleanups and fixes.
> > Patches 4-6 refactor the functions responsible for writing inline data,
> > consolidating their logic for better code organization.
> > Patch 7 implements the feature of storing truncated files as inline data
> > on the next write operation.
>
> Thanks for the patches! So ext4 inline data feature is a bit problematic
> and has some locking issues in the implementation which we didn't manage to
> fix [1]. We are even considering just disabling this feature due to the
> complications with it. Hence I don't quite like further complicating this
> code by adding possibility to create inline data in an inode after it had
> data blocks allocated. That being said I like the preparatory cleanups in
> this patch series. These are useful regardless of the feature itself.

Ok. It looks fine to me. Thank you for your clarification.
>
>                                                                 Honza
>
> [1] See our attempts in thread:
> https://lore.kernel.org/all/d704ce55-321a-9c1d-1f8b-3360a0fdf978@huawei.com
>
> --
> Jan Kara <jack@suse.com>
> SUSE Labs, CR


Thanks,
-- 
Julian Sun <sunjunchao2870@gmail.com>