On Fri, Sep 25, 2009 at 04:34:53AM -0600, Andreas Dilger wrote:
>
> Wouldn't it make more sense to just return an error (or no error
> but do nothing) in the case of source/target inode being the same?
> I don't see how that would be good to "defragment" the inode onto
> itself, just like "cp f f" and "rename f f" don't make sense either.
The code actually checks to make sure the source and target inodes are
different, but it does so too late. So the following patch should fix
the problem which Akira-san was attempting to solve, without
introducing any extra code complexity (we just move some lines of code
around instead.)
- Ted
commit 7cdf27b7241ef616b4158a26d7d85bd36f499462
Author: Theodore Ts'o <tytso@xxxxxxx>
Date: Mon Sep 28 15:58:29 2009 -0400
ext4: EXT4_IOC_MOVE_EXT: Check for different original and donor inodes first
Move the check to make sure the original and donor inodes are
different earlier, to avoid a potential deadlock by trying to lock the
same inode twice.
Signed-off-by: "Theodore Ts'o" <tytso@xxxxxxx>
diff --git a/fs/ext4/move_extent.c b/fs/ext4/move_extent.c
index 5332fd4..25b6b14 100644
--- a/fs/ext4/move_extent.c
+++ b/fs/ext4/move_extent.c
@@ -1001,14 +1001,6 @@ mext_check_arguments(struct inode *orig_inode,
return -EINVAL;
}
- /* orig and donor should be different file */
- if (orig_inode->i_ino == donor_inode->i_ino) {
- ext4_debug("ext4 move extent: The argument files should not "
- "be same file [ino:orig %lu, donor %lu]\n",
- orig_inode->i_ino, donor_inode->i_ino);
- return -EINVAL;
- }
-
/* Ext4 move extent supports only extent based file */
if (!(EXT4_I(orig_inode)->i_flags & EXT4_EXTENTS_FL)) {
ext4_debug("ext4 move extent: orig file is not extents "
@@ -1232,6 +1224,14 @@ ext4_move_extents(struct file *o_filp, struct file *d_filp,
int block_len_in_page;
int uninit;
+ /* orig and donor should be different file */
+ if (orig_inode->i_ino == donor_inode->i_ino) {
+ ext4_debug("ext4 move extent: The argument files should not "
+ "be same file [ino:orig %lu, donor %lu]\n",
+ orig_inode->i_ino, donor_inode->i_ino);
+ return -EINVAL;
+ }
+
/* protect orig and donor against a truncate */
ret1 = mext_inode_double_lock(orig_inode, donor_inode);
if (ret1 < 0)
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
