ext4: make online defrag error reporting consistent
PATCH 1/1 ext4: make online defrag error reporting consistent
Make the error reporting behavior resulting from the unsupported use of online defrag on files with data journaling enabled consistent with that implemented for bigalloc file systems. Difference found with ext4/308.
--- fs/ext4/move_extent.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)
fs/ext4/move_extent.c +7 -3
@@ -571,12 +571,16 @@ ext4_move_extents(struct file *o_filp, struct file *d_filp, __u64 orig_blk, orig_inode->i_ino, donor_inode->i_ino);return -EINVAL ; } -/* TODO: This is non obvious task to swap blocks for inodes with full -jornaling enabled */ + +/* TODO: it's not obvious how to swap blocks for inodes with full +journaling enabled */ if (ext4_should_journal_data (orig_inode) ||ext4_should_journal_data (donor_inode)) { -return -EINVAL ; +ext4_msg (orig_inode->i_sb,KERN_ERR , +"Online defrag not supported with data journaling" ); +return -EOPNOTSUPP ; } +/* Protect orig and donor inodes against a truncate */ lock_two_nondirectories (orig_inode, donor_inode);
Signature · 1 line
2.1.4
Quoted 21 lines · Eric Whitney
Make the error reporting behavior resulting from the unsupported use of online defrag on files with data journaling enabled consistent with that implemented for bigalloc file systems. Difference found with ext4/308.
Signed-off-by Eric Whitney <[email protected]>--- fs/ext4/move_extent.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)fs/ext4/move_extent.c +1
-2@@ -571,12 +571,16 @@ ext4_move_extents(struct file *o_filp, struct file *d_filp, __u64 orig_blk, orig_inode->i_ino, donor_inode->i_ino);return -EINVAL ; } -/* TODO: This is non obvious task to swap blocks for inodes with full -jornaling enabled */ +
fs/ext4/move_extent.c ext4_move_extents(struct file *o_filp, struct file *d_filp, __u64 orig_blk, · line 583
Quoted 10 lines
fs/ext4/move_extent.c +6
-1+/* TODO: it's not obvious how to swap blocks for inodes with full +journaling enabled */ if (ext4_should_journal_data (orig_inode) ||ext4_should_journal_data (donor_inode)) { -return -EINVAL ; +ext4_msg (orig_inode->i_sb,KERN_ERR , +"Online defrag not supported with data journaling" ); +return -EOPNOTSUPP ; } +
One minor nit: If it's solely the donor_inode that's data=journal (i.e. we didn't mount with data=journal and only the donor inode is chattr +j), the inode number reported in the message will be inaccurate.
I'm not sure why you'd chattr +j only the donor inode, so in practice this isn't likely to occur.
Other than that,
--D
Quoted 10 lines
fs/ext4/move_extent.c +0
-0/* Protect orig and donor inodes against a truncate */ lock_two_nondirectories (orig_inode, donor_inode);Signature · 6 lines
2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
* Darrick J. Wong <[email protected]>:
Quoted 37 lines · Eric Whitney
Quoted 21 lines
Make the error reporting behavior resulting from the unsupported use of online defrag on files with data journaling enabled consistent with that implemented for bigalloc file systems. Difference found with ext4/308.
Signed-off-by Eric Whitney <[email protected]>--- fs/ext4/move_extent.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)fs/ext4/move_extent.c +1
-2@@ -571,12 +571,16 @@ ext4_move_extents(struct file *o_filp, struct file *d_filp, __u64 orig_blk, orig_inode->i_ino, donor_inode->i_ino);return -EINVAL ; } -/* TODO: This is non obvious task to swap blocks for inodes with full -jornaling enabled */ +fs/ext4/move_extent.c ext4_move_extents(struct file *o_filp, struct file *d_filp, __u64 orig_blk, · line 583
Quoted 10 lines
fs/ext4/move_extent.c +6
-1+/* TODO: it's not obvious how to swap blocks for inodes with full +journaling enabled */ if (ext4_should_journal_data (orig_inode) ||ext4_should_journal_data (donor_inode)) { -return -EINVAL ; +ext4_msg (orig_inode->i_sb,KERN_ERR , +"Online defrag not supported with data journaling" ); +return -EOPNOTSUPP ; } +One minor nit: If it's solely the
donor_inodethat's data=journal (i.e. we didn't mount with data=journal and only the donor inode is chattr +j), the inode number reported in the message will be inaccurate.
Quoted 6 lines
I'm not sure why you'd chattr +j only the donor inode, so in practice this isn't likely to occur.
Other than that,
Reviewed-by Darrick J. Wong <[email protected]>
The message reports the file system's device rather than an inode number in this case. Code above this snippet verified that the original and donor inodes were from the same file system, so hopefully the reported device will be correct even if only the donor inode is chattr +j. Unless I'm missing something, of course...
Thanks for the review!
Eric
Quoted 12 lines
--D
Quoted 10 lines
fs/ext4/move_extent.c +0
-0/* Protect orig and donor inodes against a truncate */ lock_two_nondirectories (orig_inode, donor_inode);Signature · 6 lines
2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 45 lines · Eric Whitney
* Darrick J. Wong <[email protected]>:
Quoted 37 lines · Eric Whitney
Quoted 21 lines
Make the error reporting behavior resulting from the unsupported use of online defrag on files with data journaling enabled consistent with that implemented for bigalloc file systems. Difference found with ext4/308.
Signed-off-by Eric Whitney <[email protected]>--- fs/ext4/move_extent.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)fs/ext4/move_extent.c +1
-2@@ -571,12 +571,16 @@ ext4_move_extents(struct file *o_filp, struct file *d_filp, __u64 orig_blk, orig_inode->i_ino, donor_inode->i_ino);return -EINVAL ; } -/* TODO: This is non obvious task to swap blocks for inodes with full -jornaling enabled */ +fs/ext4/move_extent.c ext4_move_extents(struct file *o_filp, struct file *d_filp, __u64 orig_blk, · line 583
Quoted 10 lines
fs/ext4/move_extent.c +6
-1+/* TODO: it's not obvious how to swap blocks for inodes with full +journaling enabled */ if (ext4_should_journal_data (orig_inode) ||ext4_should_journal_data (donor_inode)) { -return -EINVAL ; +ext4_msg (orig_inode->i_sb,KERN_ERR , +"Online defrag not supported with data journaling" ); +return -EOPNOTSUPP ; } +One minor nit: If it's solely the
donor_inodethat's data=journal (i.e. we didn't mount with data=journal and only the donor inode is chattr +j), the inode number reported in the message will be inaccurate.Quoted 6 lines
I'm not sure why you'd chattr +j only the donor inode, so in practice this isn't likely to occur.
Other than that,
Reviewed-by Darrick J. Wong <[email protected]>
Quoted 5 lines
The message reports the file system's device rather than an inode number in this case. Code above this snippet verified that the original and donor inodes were from the same file system, so hopefully the reported device will be correct even if only the donor inode is chattr +j. Unless I'm missing something, of course...
inode->i_sb ==> superblock, d'oh!
You're right, I misread that expression for the inode number, ignore my noise. :)
--D
Quoted 17 lines
Thanks for the review!
EricQuoted 12 lines
--D
Quoted 10 lines
fs/ext4/move_extent.c +0
-0/* Protect orig and donor inodes against a truncate */ lock_two_nondirectories (orig_inode, donor_inode);Signature · 6 lines
2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 6 lines · Eric Whitney
Make the error reporting behavior resulting from the unsupported use of online defrag on files with data journaling enabled consistent with that implemented for bigalloc file systems. Difference found with ext4/308.
Signed-off-by Eric Whitney <[email protected]>
Applied, thanks.
- Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in