This patch sets fix 2 crash-consistency issues of fast commit.
First patch change to use ext4_ext_remove_space instead of
ext4_punch_hole during replay delete range procedure. This
avoid replay procedure being affeced by incorrect inode->i_size.
Second patch correct the trank range logic for ftruncte.
After testing this patch sets with xfstests-bld, in the "log" and
"quick" group with config "fast_commit" is selected. No regressions
was found.
Signed-off-by: Xin Yin <[email protected]>
Xin Yin (2):
ext4: use ext4_ext_remove_space() for fast commit replay delete range
ext4: fast commit may miss tracking unwritten range during ftruncate
fs/ext4/fast_commit.c | 13 ++++++++-----
fs/ext4/inode.c | 3 +--
2 files changed, 9 insertions(+), 7 deletions(-)
--
2.20.1
For now ,we use ext4_punch_hole() during fast commit replay delete range
procedure. But it will be affected by inode->i_size, which may not
correct during fast commit replay procedure. The following test will
failed.
-create & write foo (len 1000K)
-falloc FALLOC_FL_ZERO_RANGE foo (range 400K - 600K)
-create & fsync bar
-falloc FALLOC_FL_PUNCH_HOLE foo (range 300K-500K)
-fsync foo
-crash before a full commit
After the fast_commit reply procedure, the range 400K-500K will not be
removed. Because in this case, when calling ext4_punch_hole() the
inode->i_size is 0, and it just retruns with doing nothing.
Change to use ext4_ext_remove_space() instead of ext4_punch_hole()
to remove blocks of inode directly.
Signed-off-by: Xin Yin <[email protected]>
---
fs/ext4/fast_commit.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
index aa05b23f9c14..3deb97b22ca4 100644
--- a/fs/ext4/fast_commit.c
+++ b/fs/ext4/fast_commit.c
@@ -1708,11 +1708,14 @@ ext4_fc_replay_del_range(struct super_block *sb, struct ext4_fc_tl *tl,
}
}
- ret = ext4_punch_hole(inode,
- le32_to_cpu(lrange.fc_lblk) << sb->s_blocksize_bits,
- le32_to_cpu(lrange.fc_len) << sb->s_blocksize_bits);
- if (ret)
- jbd_debug(1, "ext4_punch_hole returned %d", ret);
+ down_write(&EXT4_I(inode)->i_data_sem);
+ ret = ext4_ext_remove_space(inode, lrange.fc_lblk,
+ lrange.fc_lblk + lrange.fc_len - 1);
+ up_write(&EXT4_I(inode)->i_data_sem);
+ if (ret) {
+ iput(inode);
+ return 0;
+ }
ext4_ext_replay_shrink_inode(inode,
i_size_read(inode) >> sb->s_blocksize_bits);
ext4_mark_inode_dirty(NULL, inode);
--
2.20.1
If use FALLOC_FL_KEEP_SIZE to alloc unwritten range at bottom, the
inode->i_size will not include the unwritten range. When call
ftruncate with fast commit enabled, it will miss to track the
unwritten range.
Change to trace the full range during ftruncate.
Signed-off-by: Xin Yin <[email protected]>
---
fs/ext4/inode.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 82f555d26980..1d2ba63874ad 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -5423,8 +5423,7 @@ int ext4_setattr(struct user_namespace *mnt_userns, struct dentry *dentry,
ext4_fc_track_range(handle, inode,
(attr->ia_size > 0 ? attr->ia_size - 1 : 0) >>
inode->i_sb->s_blocksize_bits,
- (oldsize > 0 ? oldsize - 1 : 0) >>
- inode->i_sb->s_blocksize_bits);
+ EXT_MAX_BLOCKS - 1);
else
ext4_fc_track_range(
handle, inode,
--
2.20.1
Looks good to me.
Reviewed-by: Harshad Shirwadkar <[email protected]>
On Wed, Dec 22, 2021 at 7:24 PM Xin Yin <[email protected]> wrote:
>
> For now ,we use ext4_punch_hole() during fast commit replay delete range
> procedure. But it will be affected by inode->i_size, which may not
> correct during fast commit replay procedure. The following test will
> failed.
>
> -create & write foo (len 1000K)
> -falloc FALLOC_FL_ZERO_RANGE foo (range 400K - 600K)
> -create & fsync bar
> -falloc FALLOC_FL_PUNCH_HOLE foo (range 300K-500K)
> -fsync foo
> -crash before a full commit
>
> After the fast_commit reply procedure, the range 400K-500K will not be
> removed. Because in this case, when calling ext4_punch_hole() the
> inode->i_size is 0, and it just retruns with doing nothing.
>
> Change to use ext4_ext_remove_space() instead of ext4_punch_hole()
> to remove blocks of inode directly.
>
> Signed-off-by: Xin Yin <[email protected]>
> ---
> fs/ext4/fast_commit.c | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
> index aa05b23f9c14..3deb97b22ca4 100644
> --- a/fs/ext4/fast_commit.c
> +++ b/fs/ext4/fast_commit.c
> @@ -1708,11 +1708,14 @@ ext4_fc_replay_del_range(struct super_block *sb, struct ext4_fc_tl *tl,
> }
> }
>
> - ret = ext4_punch_hole(inode,
> - le32_to_cpu(lrange.fc_lblk) << sb->s_blocksize_bits,
> - le32_to_cpu(lrange.fc_len) << sb->s_blocksize_bits);
> - if (ret)
> - jbd_debug(1, "ext4_punch_hole returned %d", ret);
> + down_write(&EXT4_I(inode)->i_data_sem);
> + ret = ext4_ext_remove_space(inode, lrange.fc_lblk,
> + lrange.fc_lblk + lrange.fc_len - 1);
> + up_write(&EXT4_I(inode)->i_data_sem);
> + if (ret) {
> + iput(inode);
> + return 0;
> + }
> ext4_ext_replay_shrink_inode(inode,
> i_size_read(inode) >> sb->s_blocksize_bits);
> ext4_mark_inode_dirty(NULL, inode);
> --
> 2.20.1
>
Looks good to me.
Reviewed-by: Harshad Shirwadkar <[email protected]>
On Wed, Dec 22, 2021 at 7:24 PM Xin Yin <[email protected]> wrote:
>
> If use FALLOC_FL_KEEP_SIZE to alloc unwritten range at bottom, the
> inode->i_size will not include the unwritten range. When call
> ftruncate with fast commit enabled, it will miss to track the
> unwritten range.
>
> Change to trace the full range during ftruncate.
>
> Signed-off-by: Xin Yin <[email protected]>
> ---
> fs/ext4/inode.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 82f555d26980..1d2ba63874ad 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -5423,8 +5423,7 @@ int ext4_setattr(struct user_namespace *mnt_userns, struct dentry *dentry,
> ext4_fc_track_range(handle, inode,
> (attr->ia_size > 0 ? attr->ia_size - 1 : 0) >>
> inode->i_sb->s_blocksize_bits,
> - (oldsize > 0 ? oldsize - 1 : 0) >>
> - inode->i_sb->s_blocksize_bits);
> + EXT_MAX_BLOCKS - 1);
> else
> ext4_fc_track_range(
> handle, inode,
> --
> 2.20.1
>
Thanks for the patches, these are good finds.
Reviewed-by: Harshad Shirwadkar <[email protected]>
On Wed, Dec 22, 2021 at 7:24 PM Xin Yin <[email protected]> wrote:
>
> This patch sets fix 2 crash-consistency issues of fast commit.
> First patch change to use ext4_ext_remove_space instead of
> ext4_punch_hole during replay delete range procedure. This
> avoid replay procedure being affeced by incorrect inode->i_size.
> Second patch correct the trank range logic for ftruncte.
>
> After testing this patch sets with xfstests-bld, in the "log" and
> "quick" group with config "fast_commit" is selected. No regressions
> was found.
>
> Signed-off-by: Xin Yin <[email protected]>
>
> Xin Yin (2):
> ext4: use ext4_ext_remove_space() for fast commit replay delete range
> ext4: fast commit may miss tracking unwritten range during ftruncate
>
> fs/ext4/fast_commit.c | 13 ++++++++-----
> fs/ext4/inode.c | 3 +--
> 2 files changed, 9 insertions(+), 7 deletions(-)
>
> --
> 2.20.1
>
On Thu, 23 Dec 2021 11:23:35 +0800, Xin Yin wrote:
> This patch sets fix 2 crash-consistency issues of fast commit.
> First patch change to use ext4_ext_remove_space instead of
> ext4_punch_hole during replay delete range procedure. This
> avoid replay procedure being affeced by incorrect inode->i_size.
> Second patch correct the trank range logic for ftruncte.
>
> After testing this patch sets with xfstests-bld, in the "log" and
> "quick" group with config "fast_commit" is selected. No regressions
> was found.
>
> [...]
Applied, thanks!
[1/2] ext4: use ext4_ext_remove_space() for fast commit replay delete range
commit: 20a262d66487b27133c3f8b7ac0245c2fabaa526
[2/2] ext4: fast commit may miss tracking unwritten range during ftruncate
commit: 949197083545cbff930650f893135d30fd043d4d
Best regards,
--
Theodore Ts'o <[email protected]>