ext4: don't track ranges in fast_commit if inode has inlined data
PATCH 1/1 ext4: don't track ranges in fast_commit if inode has inlined data
When fast-commit needs to track ranges, it has to handle inodes that have inlined data in a different way because ext4_fc_write_inode_data(), in the actual commit path, will attempt to map the required blocks for the range. However, inodes that have inlined data will have it's data stored in inode->i_block and, eventually, in the extended attribute space.
Unfortunately, because fast commit doesn't currently support extended attributes, the solution is to mark this commit as ineligible.
--- fs/ext4/fast_commit.c | 6 ++++++ 1 file changed, 6 insertions(+)
fs/ext4/fast_commit.c +6 -0
@@ -649,6 +649,12 @@ void ext4_fc_track_range(handle_t *handle, struct inode *inode, ext4_lblk_t starif (ext4_test_mount_flag (inode->i_sb,EXT4_MF_FC_INELIGIBLE ))return ; +if (ext4_has_inline_data (inode)) { +ext4_fc_mark_ineligible (inode->i_sb,EXT4_FC_REASON_XATTR , + handle); +return ; + } + args.start = start; args.end = end;
Quoted 11 lines · Luis Henriques (SUSE)
When fast-commit needs to track ranges, it has to handle inodes that have inlined data in a different way because
ext4_fc_write_inode_data(), in the actual commit path, will attempt to map the required blocks for the range. However, inodes that have inlined data will have it's data stored ininode->i_blockand, eventually, in the extended attribute space.Unfortunately, because fast commit doesn't currently support extended attributes, the solution is to mark this commit as ineligible.
I think this should also have:
unless you think the problem is even older than that.
Ben.
Quoted 21 lines
--- fs/ext4/fast_commit.c | 6 ++++++ 1 file changed, 6 insertions(+)fs/ext4/fast_commit.c +6
-0@@ -649,6 +649,12 @@ void ext4_fc_track_range(handle_t *handle, struct inode *inode, ext4_lblk_t starif (ext4_test_mount_flag (inode->i_sb,EXT4_MF_FC_INELIGIBLE ))return ; +if (ext4_has_inline_data (inode)) { +ext4_fc_mark_ineligible (inode->i_sb,EXT4_FC_REASON_XATTR , + handle); +return ; + } + args.start = start; args.end = end;
Signature · 3 lines
Ben Hutchings For every complex problem there is a solution that is simple, neat, and wrong.
On Wed 19 Jun 2024 12:30:38 AM +02, Ben Hutchings wrote;
Quoted 7 lines · Luis Henriques (SUSE)
Quoted 6 lines
When fast-commit needs to track ranges, it has to handle inodes that have inlined data in a different way because
ext4_fc_write_inode_data(), in the actual commit path, will attempt to map the required blocks for the range. However, inodes that have inlined data will have it's data stored ininode->i_blockand, eventually, in the extended attribute space.
Quoted 9 lines
Quoted 5 lines
Unfortunately, because fast commit doesn't currently support extended attributes, the solution is to mark this commit as ineligible.
Reported-by Hervé Werner <[email protected]>Tested-by Ben Hutchings <[email protected]>
Thanks a lot, Ben.
Quoted 5 lines
I think this should also have:
Fixes 9725958bb75c ("ext4: fast commit may miss tracking unwritten range during ftruncate")unless you think the problem is even older than that.
If my understanding is correct (hopefully someone will confirm that!), I think the problem goes further back. That commit just makes it more likely to be visible, but handling of inlined data is incorrect since the fast_commit merge. So, I guess that's better to simply add:
Cheers,
Signature · 1 line
Luís
Quoted 30 lines
Ben.
Quoted 21 lines
--- fs/ext4/fast_commit.c | 6 ++++++ 1 file changed, 6 insertions(+)fs/ext4/fast_commit.c +6
-0@@ -649,6 +649,12 @@ void ext4_fc_track_range(handle_t *handle, struct inode *inode, ext4_lblk_t starif (ext4_test_mount_flag (inode->i_sb,EXT4_MF_FC_INELIGIBLE ))return ; +if (ext4_has_inline_data (inode)) { +ext4_fc_mark_ineligible (inode->i_sb,EXT4_FC_REASON_XATTR , + handle); +return ; + } + args.start = start; args.end = end;Signature · 3 lines
Ben Hutchings For every complex problem there is a solution that is simple, neat, and wrong.
Quoted 25 lines · Luis Henriques
On Wed 19 Jun 2024 12:30:38 AM +02, Ben Hutchings wrote;
Quoted 7 lines · Luis Henriques (SUSE)
Quoted 6 lines
When fast-commit needs to track ranges, it has to handle inodes that have inlined data in a different way because
ext4_fc_write_inode_data(), in the actual commit path, will attempt to map the required blocks for the range. However, inodes that have inlined data will have it's data stored ininode->i_blockand, eventually, in the extended attribute space.Quoted 9 lines
Quoted 5 lines
Unfortunately, because fast commit doesn't currently support extended attributes, the solution is to mark this commit as ineligible.
Reported-by Hervé Werner <[email protected]>Tested-by Ben Hutchings <[email protected]>Thanks a lot, Ben.
Quoted 4 lines
I think this should also have:
Fixes 9725958bb75c ("ext4: fast commit may miss tracking unwritten range during ftruncate")
Quoted 8 lines
Quoted 1 line
unless you think the problem is even older than that.
If my understanding is correct (hopefully someone will confirm that!), I think the problem goes further back. That commit just makes it more likely to be visible, but handling of inlined data is incorrect since the
fast_commitmerge. So, I guess that's better to simply add:
That makes sense to me.
Ben.
Signature · 3 lines
Ben Hutchings For every complex problem there is a solution that is simple, neat, and wrong.
Quoted 10 lines · Luis Henriques (SUSE)
When fast-commit needs to track ranges, it has to handle inodes that have inlined data in a different way because
ext4_fc_write_inode_data(), in the actual commit path, will attempt to map the required blocks for the range. However, inodes that have inlined data will have it's data stored ininode->i_blockand, eventually, in the extended attribute space.Unfortunately, because fast commit doesn't currently support extended attributes, the solution is to mark this commit as ineligible.
[...]
Applied, thanks!
[1/1] ext4: don't track ranges in fast_commit if inode has inlined data
commit: 7882b0187bbeb647967a7b5998ce4ad26ef68a9a
Best regards,
Signature · 1 line
Theodore Ts'o <[email protected]>