ext4: refactor mext_check_arguments()

When moving extents, mext_check_validity() performs some basic file
system and file checks. However, some essential checks need to be
performed after acquiring the i_rwsem are still scattered in
mext_check_arguments(). Move those checks into mext_check_validity() and
make it executes entirely under the i_rwsem to make the checks clearer.
Furthermore, rename mext_check_arguments() to mext_check_adjust_range(),
as it only performs checks and length adjustments on the move extent
range. Finally, also change the print message for the non-existent file
check to be consistent with other unsupported checks.

Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
Reviewed-by: Jan Kara <jack@suse.cz>
Message-ID: <20251013015128.499308-8-yi.zhang@huaweicloud.com>
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
This commit is contained in:
Zhang Yi
2025-10-13 09:51:23 +08:00
committed by Theodore Ts'o
parent 22218516e4
commit 57c1df07f1

View File

@@ -501,60 +501,36 @@ static int mext_check_validity(struct inode *orig_inode,
return -EOPNOTSUPP; return -EOPNOTSUPP;
} }
return 0; /* Ext4 move extent supports only extent based file */
} if (!(ext4_test_inode_flag(orig_inode, EXT4_INODE_EXTENTS)) ||
!(ext4_test_inode_flag(donor_inode, EXT4_INODE_EXTENTS))) {
/** ext4_msg(sb, KERN_ERR,
* mext_check_arguments - Check whether move extent can be done "Online defrag not supported for non-extent files");
* return -EOPNOTSUPP;
* @orig_inode: original inode }
* @donor_inode: donor inode
* @orig_start: logical start offset in block for orig
* @donor_start: logical start offset in block for donor
* @len: the number of blocks to be moved
*
* Check the arguments of ext4_move_extents() whether the files can be
* exchanged with each other.
* Return 0 on success, or a negative error value on failure.
*/
static int
mext_check_arguments(struct inode *orig_inode,
struct inode *donor_inode, __u64 orig_start,
__u64 donor_start, __u64 *len)
{
__u64 orig_eof, donor_eof;
if (donor_inode->i_mode & (S_ISUID|S_ISGID)) { if (donor_inode->i_mode & (S_ISUID|S_ISGID)) {
ext4_debug("ext4 move extent: suid or sgid is set" ext4_debug("ext4 move extent: suid or sgid is set to donor file [ino:orig %lu, donor %lu]\n",
" to donor file [ino:orig %lu, donor %lu]\n",
orig_inode->i_ino, donor_inode->i_ino); orig_inode->i_ino, donor_inode->i_ino);
return -EINVAL; return -EINVAL;
} }
if (IS_IMMUTABLE(donor_inode) || IS_APPEND(donor_inode)) if (IS_IMMUTABLE(donor_inode) || IS_APPEND(donor_inode)) {
ext4_debug("ext4 move extent: donor should not be immutable or append file [ino:orig %lu, donor %lu]\n",
orig_inode->i_ino, donor_inode->i_ino);
return -EPERM; return -EPERM;
}
/* Ext4 move extent does not support swap files */ /* Ext4 move extent does not support swap files */
if (IS_SWAPFILE(orig_inode) || IS_SWAPFILE(donor_inode)) { if (IS_SWAPFILE(orig_inode) || IS_SWAPFILE(donor_inode)) {
ext4_debug("ext4 move extent: The argument files should not be swap files [ino:orig %lu, donor %lu]\n", ext4_debug("ext4 move extent: The argument files should not be swap files [ino:orig %lu, donor %lu]\n",
orig_inode->i_ino, donor_inode->i_ino); orig_inode->i_ino, donor_inode->i_ino);
return -ETXTBSY; return -ETXTBSY;
} }
if (ext4_is_quota_file(orig_inode) || ext4_is_quota_file(donor_inode)) { if (ext4_is_quota_file(orig_inode) || ext4_is_quota_file(donor_inode)) {
ext4_debug("ext4 move extent: The argument files should not be quota files [ino:orig %lu, donor %lu]\n", ext4_debug("ext4 move extent: The argument files should not be quota files [ino:orig %lu, donor %lu]\n",
orig_inode->i_ino, donor_inode->i_ino); orig_inode->i_ino, donor_inode->i_ino);
return -EOPNOTSUPP;
}
/* Ext4 move extent supports only extent based file */
if (!(ext4_test_inode_flag(orig_inode, EXT4_INODE_EXTENTS))) {
ext4_debug("ext4 move extent: orig file is not extents "
"based file [ino:orig %lu]\n", orig_inode->i_ino);
return -EOPNOTSUPP;
} else if (!(ext4_test_inode_flag(donor_inode, EXT4_INODE_EXTENTS))) {
ext4_debug("ext4 move extent: donor file is not extents "
"based file [ino:donor %lu]\n", donor_inode->i_ino);
return -EOPNOTSUPP; return -EOPNOTSUPP;
} }
@@ -563,12 +539,25 @@ mext_check_arguments(struct inode *orig_inode,
return -EINVAL; return -EINVAL;
} }
return 0;
}
/*
* Check the moving range of ext4_move_extents() whether the files can be
* exchanged with each other, and adjust the length to fit within the file
* size. Return 0 on success, or a negative error value on failure.
*/
static int mext_check_adjust_range(struct inode *orig_inode,
struct inode *donor_inode, __u64 orig_start,
__u64 donor_start, __u64 *len)
{
__u64 orig_eof, donor_eof;
/* Start offset should be same */ /* Start offset should be same */
if ((orig_start & ~(PAGE_MASK >> orig_inode->i_blkbits)) != if ((orig_start & ~(PAGE_MASK >> orig_inode->i_blkbits)) !=
(donor_start & ~(PAGE_MASK >> orig_inode->i_blkbits))) { (donor_start & ~(PAGE_MASK >> orig_inode->i_blkbits))) {
ext4_debug("ext4 move extent: orig and donor's start " ext4_debug("ext4 move extent: orig and donor's start offsets are not aligned [ino:orig %lu, donor %lu]\n",
"offsets are not aligned [ino:orig %lu, donor %lu]\n", orig_inode->i_ino, donor_inode->i_ino);
orig_inode->i_ino, donor_inode->i_ino);
return -EINVAL; return -EINVAL;
} }
@@ -577,9 +566,9 @@ mext_check_arguments(struct inode *orig_inode,
(*len > EXT_MAX_BLOCKS) || (*len > EXT_MAX_BLOCKS) ||
(donor_start + *len >= EXT_MAX_BLOCKS) || (donor_start + *len >= EXT_MAX_BLOCKS) ||
(orig_start + *len >= EXT_MAX_BLOCKS)) { (orig_start + *len >= EXT_MAX_BLOCKS)) {
ext4_debug("ext4 move extent: Can't handle over [%u] blocks " ext4_debug("ext4 move extent: Can't handle over [%u] blocks [ino:orig %lu, donor %lu]\n",
"[ino:orig %lu, donor %lu]\n", EXT_MAX_BLOCKS, EXT_MAX_BLOCKS,
orig_inode->i_ino, donor_inode->i_ino); orig_inode->i_ino, donor_inode->i_ino);
return -EINVAL; return -EINVAL;
} }
@@ -594,9 +583,8 @@ mext_check_arguments(struct inode *orig_inode,
else if (donor_eof < donor_start + *len - 1) else if (donor_eof < donor_start + *len - 1)
*len = donor_eof - donor_start; *len = donor_eof - donor_start;
if (!*len) { if (!*len) {
ext4_debug("ext4 move extent: len should not be 0 " ext4_debug("ext4 move extent: len should not be 0 [ino:orig %lu, donor %lu]\n",
"[ino:orig %lu, donor %lu]\n", orig_inode->i_ino, orig_inode->i_ino, donor_inode->i_ino);
donor_inode->i_ino);
return -EINVAL; return -EINVAL;
} }
@@ -629,22 +617,22 @@ ext4_move_extents(struct file *o_filp, struct file *d_filp, __u64 orig_blk,
ext4_lblk_t d_start = donor_blk; ext4_lblk_t d_start = donor_blk;
int ret; int ret;
ret = mext_check_validity(orig_inode, donor_inode);
if (ret)
return ret;
/* Protect orig and donor inodes against a truncate */ /* Protect orig and donor inodes against a truncate */
lock_two_nondirectories(orig_inode, donor_inode); lock_two_nondirectories(orig_inode, donor_inode);
ret = mext_check_validity(orig_inode, donor_inode);
if (ret)
goto unlock;
/* Wait for all existing dio workers */ /* Wait for all existing dio workers */
inode_dio_wait(orig_inode); inode_dio_wait(orig_inode);
inode_dio_wait(donor_inode); inode_dio_wait(donor_inode);
/* Protect extent tree against block allocations via delalloc */ /* Protect extent tree against block allocations via delalloc */
ext4_double_down_write_data_sem(orig_inode, donor_inode); ext4_double_down_write_data_sem(orig_inode, donor_inode);
/* Check the filesystem environment whether move_extent can be done */ /* Check and adjust the specified move_extent range. */
ret = mext_check_arguments(orig_inode, donor_inode, orig_blk, ret = mext_check_adjust_range(orig_inode, donor_inode, orig_blk,
donor_blk, &len); donor_blk, &len);
if (ret) if (ret)
goto out; goto out;
o_end = o_start + len; o_end = o_start + len;
@@ -725,6 +713,7 @@ out:
ext4_free_ext_path(path); ext4_free_ext_path(path);
ext4_double_up_write_data_sem(orig_inode, donor_inode); ext4_double_up_write_data_sem(orig_inode, donor_inode);
unlock:
unlock_two_nondirectories(orig_inode, donor_inode); unlock_two_nondirectories(orig_inode, donor_inode);
return ret; return ret;