Commit 2147b1a6 authored by Akira Fujita's avatar Akira Fujita Committed by Theodore Ts'o

ext4: Replace BUG_ON() with ext4_error() in move_extents.c

Replace BUG_ON calls with a call to ext4_error()
to print an error message if EXT4_IOC_MOVE_EXT failed
with some kind of reasons.  This will help to debug.
Ted pointed this out, thanks.
Signed-off-by: default avatarAkira Fujita <a-fujita@rs.jp.nec.com>
Signed-off-by: default avatar"Theodore Ts'o" <tytso@mit.edu>
parent e8505970
...@@ -127,6 +127,31 @@ mext_next_extent(struct inode *inode, struct ext4_ext_path *path, ...@@ -127,6 +127,31 @@ mext_next_extent(struct inode *inode, struct ext4_ext_path *path,
return 1; return 1;
} }
/**
* mext_check_null_inode - NULL check for two inodes
*
* If inode1 or inode2 is NULL, return -EIO. Otherwise, return 0.
*/
static int
mext_check_null_inode(struct inode *inode1, struct inode *inode2,
const char *function)
{
int ret = 0;
if (inode1 == NULL) {
ext4_error(inode2->i_sb, function,
"Both inodes should not be NULL: "
"inode1 NULL inode2 %lu", inode2->i_ino);
ret = -EIO;
} else if (inode2 == NULL) {
ext4_error(inode1->i_sb, function,
"Both inodes should not be NULL: "
"inode1 %lu inode2 NULL", inode1->i_ino);
ret = -EIO;
}
return ret;
}
/** /**
* mext_double_down_read - Acquire two inodes' read semaphore * mext_double_down_read - Acquire two inodes' read semaphore
* *
...@@ -139,8 +164,6 @@ mext_double_down_read(struct inode *orig_inode, struct inode *donor_inode) ...@@ -139,8 +164,6 @@ mext_double_down_read(struct inode *orig_inode, struct inode *donor_inode)
{ {
struct inode *first = orig_inode, *second = donor_inode; struct inode *first = orig_inode, *second = donor_inode;
BUG_ON(orig_inode == NULL || donor_inode == NULL);
/* /*
* Use the inode number to provide the stable locking order instead * Use the inode number to provide the stable locking order instead
* of its address, because the C language doesn't guarantee you can * of its address, because the C language doesn't guarantee you can
...@@ -167,8 +190,6 @@ mext_double_down_write(struct inode *orig_inode, struct inode *donor_inode) ...@@ -167,8 +190,6 @@ mext_double_down_write(struct inode *orig_inode, struct inode *donor_inode)
{ {
struct inode *first = orig_inode, *second = donor_inode; struct inode *first = orig_inode, *second = donor_inode;
BUG_ON(orig_inode == NULL || donor_inode == NULL);
/* /*
* Use the inode number to provide the stable locking order instead * Use the inode number to provide the stable locking order instead
* of its address, because the C language doesn't guarantee you can * of its address, because the C language doesn't guarantee you can
...@@ -193,8 +214,6 @@ mext_double_down_write(struct inode *orig_inode, struct inode *donor_inode) ...@@ -193,8 +214,6 @@ mext_double_down_write(struct inode *orig_inode, struct inode *donor_inode)
static void static void
mext_double_up_read(struct inode *orig_inode, struct inode *donor_inode) mext_double_up_read(struct inode *orig_inode, struct inode *donor_inode)
{ {
BUG_ON(orig_inode == NULL || donor_inode == NULL);
up_read(&EXT4_I(orig_inode)->i_data_sem); up_read(&EXT4_I(orig_inode)->i_data_sem);
up_read(&EXT4_I(donor_inode)->i_data_sem); up_read(&EXT4_I(donor_inode)->i_data_sem);
} }
...@@ -209,8 +228,6 @@ mext_double_up_read(struct inode *orig_inode, struct inode *donor_inode) ...@@ -209,8 +228,6 @@ mext_double_up_read(struct inode *orig_inode, struct inode *donor_inode)
static void static void
mext_double_up_write(struct inode *orig_inode, struct inode *donor_inode) mext_double_up_write(struct inode *orig_inode, struct inode *donor_inode)
{ {
BUG_ON(orig_inode == NULL || donor_inode == NULL);
up_write(&EXT4_I(orig_inode)->i_data_sem); up_write(&EXT4_I(orig_inode)->i_data_sem);
up_write(&EXT4_I(donor_inode)->i_data_sem); up_write(&EXT4_I(donor_inode)->i_data_sem);
} }
...@@ -534,7 +551,15 @@ mext_leaf_block(handle_t *handle, struct inode *orig_inode, ...@@ -534,7 +551,15 @@ mext_leaf_block(handle_t *handle, struct inode *orig_inode,
* oext |-----------| * oext |-----------|
* new_ext |-------| * new_ext |-------|
*/ */
BUG_ON(le32_to_cpu(oext->ee_block) + oext_alen - 1 < new_ext_end); if (le32_to_cpu(oext->ee_block) + oext_alen - 1 < new_ext_end) {
ext4_error(orig_inode->i_sb, __func__,
"new_ext_end(%u) should be less than or equal to "
"oext->ee_block(%u) + oext_alen(%d) - 1",
new_ext_end, le32_to_cpu(oext->ee_block),
oext_alen);
ret = -EIO;
goto out;
}
/* /*
* Case: new_ext is smaller than original extent * Case: new_ext is smaller than original extent
...@@ -558,6 +583,7 @@ mext_leaf_block(handle_t *handle, struct inode *orig_inode, ...@@ -558,6 +583,7 @@ mext_leaf_block(handle_t *handle, struct inode *orig_inode,
ret = mext_insert_extents(handle, orig_inode, orig_path, o_start, ret = mext_insert_extents(handle, orig_inode, orig_path, o_start,
o_end, &start_ext, &new_ext, &end_ext); o_end, &start_ext, &new_ext, &end_ext);
out:
return ret; return ret;
} }
...@@ -668,7 +694,20 @@ mext_replace_branches(handle_t *handle, struct inode *orig_inode, ...@@ -668,7 +694,20 @@ mext_replace_branches(handle_t *handle, struct inode *orig_inode,
/* Loop for the donor extents */ /* Loop for the donor extents */
while (1) { while (1) {
/* The extent for donor must be found. */ /* The extent for donor must be found. */
BUG_ON(!dext || donor_off != le32_to_cpu(tmp_dext.ee_block)); if (!dext) {
ext4_error(donor_inode->i_sb, __func__,
"The extent for donor must be found");
err = -EIO;
goto out;
} else if (donor_off != le32_to_cpu(tmp_dext.ee_block)) {
ext4_error(donor_inode->i_sb, __func__,
"Donor offset(%u) and the first block of donor "
"extent(%u) should be equal",
donor_off,
le32_to_cpu(tmp_dext.ee_block));
err = -EIO;
goto out;
}
/* Set donor extent to orig extent */ /* Set donor extent to orig extent */
err = mext_leaf_block(handle, orig_inode, err = mext_leaf_block(handle, orig_inode,
...@@ -1050,18 +1089,23 @@ mext_check_arguments(struct inode *orig_inode, ...@@ -1050,18 +1089,23 @@ mext_check_arguments(struct inode *orig_inode,
* @inode1: the inode structure * @inode1: the inode structure
* @inode2: the inode structure * @inode2: the inode structure
* *
* Lock two inodes' i_mutex by i_ino order. This function is moved from * Lock two inodes' i_mutex by i_ino order.
* fs/inode.c. * If inode1 or inode2 is NULL, return -EIO. Otherwise, return 0.
*/ */
static void static int
mext_inode_double_lock(struct inode *inode1, struct inode *inode2) mext_inode_double_lock(struct inode *inode1, struct inode *inode2)
{ {
if (inode1 == NULL || inode2 == NULL || inode1 == inode2) { int ret = 0;
if (inode1)
mutex_lock(&inode1->i_mutex); BUG_ON(inode1 == NULL && inode2 == NULL);
else if (inode2)
mutex_lock(&inode2->i_mutex); ret = mext_check_null_inode(inode1, inode2, __func__);
return; if (ret < 0)
goto out;
if (inode1 == inode2) {
mutex_lock(&inode1->i_mutex);
goto out;
} }
if (inode1->i_ino < inode2->i_ino) { if (inode1->i_ino < inode2->i_ino) {
...@@ -1071,6 +1115,9 @@ mext_inode_double_lock(struct inode *inode1, struct inode *inode2) ...@@ -1071,6 +1115,9 @@ mext_inode_double_lock(struct inode *inode1, struct inode *inode2)
mutex_lock_nested(&inode2->i_mutex, I_MUTEX_PARENT); mutex_lock_nested(&inode2->i_mutex, I_MUTEX_PARENT);
mutex_lock_nested(&inode1->i_mutex, I_MUTEX_CHILD); mutex_lock_nested(&inode1->i_mutex, I_MUTEX_CHILD);
} }
out:
return ret;
} }
/** /**
...@@ -1079,17 +1126,28 @@ mext_inode_double_lock(struct inode *inode1, struct inode *inode2) ...@@ -1079,17 +1126,28 @@ mext_inode_double_lock(struct inode *inode1, struct inode *inode2)
* @inode1: the inode that is released first * @inode1: the inode that is released first
* @inode2: the inode that is released second * @inode2: the inode that is released second
* *
* This function is moved from fs/inode.c. * If inode1 or inode2 is NULL, return -EIO. Otherwise, return 0.
*/ */
static void static int
mext_inode_double_unlock(struct inode *inode1, struct inode *inode2) mext_inode_double_unlock(struct inode *inode1, struct inode *inode2)
{ {
int ret = 0;
BUG_ON(inode1 == NULL && inode2 == NULL);
ret = mext_check_null_inode(inode1, inode2, __func__);
if (ret < 0)
goto out;
if (inode1) if (inode1)
mutex_unlock(&inode1->i_mutex); mutex_unlock(&inode1->i_mutex);
if (inode2 && inode2 != inode1) if (inode2 && inode2 != inode1)
mutex_unlock(&inode2->i_mutex); mutex_unlock(&inode2->i_mutex);
out:
return ret;
} }
/** /**
...@@ -1146,21 +1204,23 @@ ext4_move_extents(struct file *o_filp, struct file *d_filp, ...@@ -1146,21 +1204,23 @@ ext4_move_extents(struct file *o_filp, struct file *d_filp,
ext4_lblk_t block_end, seq_start, add_blocks, file_end, seq_blocks = 0; ext4_lblk_t block_end, seq_start, add_blocks, file_end, seq_blocks = 0;
ext4_lblk_t rest_blocks; ext4_lblk_t rest_blocks;
pgoff_t orig_page_offset = 0, seq_end_page; pgoff_t orig_page_offset = 0, seq_end_page;
int ret, depth, last_extent = 0; int ret1, ret2, depth, last_extent = 0;
int blocks_per_page = PAGE_CACHE_SIZE >> orig_inode->i_blkbits; int blocks_per_page = PAGE_CACHE_SIZE >> orig_inode->i_blkbits;
int data_offset_in_page; int data_offset_in_page;
int block_len_in_page; int block_len_in_page;
int uninit; int uninit;
/* protect orig and donor against a truncate */ /* protect orig and donor against a truncate */
mext_inode_double_lock(orig_inode, donor_inode); ret1 = mext_inode_double_lock(orig_inode, donor_inode);
if (ret1 < 0)
return ret1;
mext_double_down_read(orig_inode, donor_inode); mext_double_down_read(orig_inode, donor_inode);
/* Check the filesystem environment whether move_extent can be done */ /* Check the filesystem environment whether move_extent can be done */
ret = mext_check_arguments(orig_inode, donor_inode, orig_start, ret1 = mext_check_arguments(orig_inode, donor_inode, orig_start,
donor_start, &len, *moved_len); donor_start, &len, *moved_len);
mext_double_up_read(orig_inode, donor_inode); mext_double_up_read(orig_inode, donor_inode);
if (ret) if (ret1)
goto out2; goto out2;
file_end = (i_size_read(orig_inode) - 1) >> orig_inode->i_blkbits; file_end = (i_size_read(orig_inode) - 1) >> orig_inode->i_blkbits;
...@@ -1168,19 +1228,19 @@ ext4_move_extents(struct file *o_filp, struct file *d_filp, ...@@ -1168,19 +1228,19 @@ ext4_move_extents(struct file *o_filp, struct file *d_filp,
if (file_end < block_end) if (file_end < block_end)
len -= block_end - file_end; len -= block_end - file_end;
ret = get_ext_path(orig_inode, block_start, &orig_path); ret1 = get_ext_path(orig_inode, block_start, &orig_path);
if (orig_path == NULL) if (orig_path == NULL)
goto out2; goto out2;
/* Get path structure to check the hole */ /* Get path structure to check the hole */
ret = get_ext_path(orig_inode, block_start, &holecheck_path); ret1 = get_ext_path(orig_inode, block_start, &holecheck_path);
if (holecheck_path == NULL) if (holecheck_path == NULL)
goto out; goto out;
depth = ext_depth(orig_inode); depth = ext_depth(orig_inode);
ext_cur = holecheck_path[depth].p_ext; ext_cur = holecheck_path[depth].p_ext;
if (ext_cur == NULL) { if (ext_cur == NULL) {
ret = -EINVAL; ret1 = -EINVAL;
goto out; goto out;
} }
...@@ -1193,13 +1253,13 @@ ext4_move_extents(struct file *o_filp, struct file *d_filp, ...@@ -1193,13 +1253,13 @@ ext4_move_extents(struct file *o_filp, struct file *d_filp,
last_extent = mext_next_extent(orig_inode, last_extent = mext_next_extent(orig_inode,
holecheck_path, &ext_cur); holecheck_path, &ext_cur);
if (last_extent < 0) { if (last_extent < 0) {
ret = last_extent; ret1 = last_extent;
goto out; goto out;
} }
last_extent = mext_next_extent(orig_inode, orig_path, last_extent = mext_next_extent(orig_inode, orig_path,
&ext_dummy); &ext_dummy);
if (last_extent < 0) { if (last_extent < 0) {
ret = last_extent; ret1 = last_extent;
goto out; goto out;
} }
} }
...@@ -1209,7 +1269,7 @@ ext4_move_extents(struct file *o_filp, struct file *d_filp, ...@@ -1209,7 +1269,7 @@ ext4_move_extents(struct file *o_filp, struct file *d_filp,
if (le32_to_cpu(ext_cur->ee_block) > block_end) { if (le32_to_cpu(ext_cur->ee_block) > block_end) {
ext4_debug("ext4 move extent: The specified range of file " ext4_debug("ext4 move extent: The specified range of file "
"may be the hole\n"); "may be the hole\n");
ret = -EINVAL; ret1 = -EINVAL;
goto out; goto out;
} }
...@@ -1229,7 +1289,7 @@ ext4_move_extents(struct file *o_filp, struct file *d_filp, ...@@ -1229,7 +1289,7 @@ ext4_move_extents(struct file *o_filp, struct file *d_filp,
last_extent = mext_next_extent(orig_inode, holecheck_path, last_extent = mext_next_extent(orig_inode, holecheck_path,
&ext_cur); &ext_cur);
if (last_extent < 0) { if (last_extent < 0) {
ret = last_extent; ret1 = last_extent;
break; break;
} }
add_blocks = ext4_ext_get_actual_len(ext_cur); add_blocks = ext4_ext_get_actual_len(ext_cur);
...@@ -1281,16 +1341,23 @@ ext4_move_extents(struct file *o_filp, struct file *d_filp, ...@@ -1281,16 +1341,23 @@ ext4_move_extents(struct file *o_filp, struct file *d_filp,
while (orig_page_offset <= seq_end_page) { while (orig_page_offset <= seq_end_page) {
/* Swap original branches with new branches */ /* Swap original branches with new branches */
ret = move_extent_per_page(o_filp, donor_inode, ret1 = move_extent_per_page(o_filp, donor_inode,
orig_page_offset, orig_page_offset,
data_offset_in_page, data_offset_in_page,
block_len_in_page, uninit); block_len_in_page, uninit);
if (ret < 0) if (ret1 < 0)
goto out; goto out;
orig_page_offset++; orig_page_offset++;
/* Count how many blocks we have exchanged */ /* Count how many blocks we have exchanged */
*moved_len += block_len_in_page; *moved_len += block_len_in_page;
BUG_ON(*moved_len > len); if (*moved_len > len) {
ext4_error(orig_inode->i_sb, __func__,
"We replaced blocks too much! "
"sum of replaced: %llu requested: %llu",
*moved_len, len);
ret1 = -EIO;
goto out;
}
data_offset_in_page = 0; data_offset_in_page = 0;
rest_blocks -= block_len_in_page; rest_blocks -= block_len_in_page;
...@@ -1303,7 +1370,7 @@ ext4_move_extents(struct file *o_filp, struct file *d_filp, ...@@ -1303,7 +1370,7 @@ ext4_move_extents(struct file *o_filp, struct file *d_filp,
/* Decrease buffer counter */ /* Decrease buffer counter */
if (holecheck_path) if (holecheck_path)
ext4_ext_drop_refs(holecheck_path); ext4_ext_drop_refs(holecheck_path);
ret = get_ext_path(orig_inode, seq_start, &holecheck_path); ret1 = get_ext_path(orig_inode, seq_start, &holecheck_path);
if (holecheck_path == NULL) if (holecheck_path == NULL)
break; break;
depth = holecheck_path->p_depth; depth = holecheck_path->p_depth;
...@@ -1311,7 +1378,7 @@ ext4_move_extents(struct file *o_filp, struct file *d_filp, ...@@ -1311,7 +1378,7 @@ ext4_move_extents(struct file *o_filp, struct file *d_filp,
/* Decrease buffer counter */ /* Decrease buffer counter */
if (orig_path) if (orig_path)
ext4_ext_drop_refs(orig_path); ext4_ext_drop_refs(orig_path);
ret = get_ext_path(orig_inode, seq_start, &orig_path); ret1 = get_ext_path(orig_inode, seq_start, &orig_path);
if (orig_path == NULL) if (orig_path == NULL)
break; break;
...@@ -1330,10 +1397,12 @@ ext4_move_extents(struct file *o_filp, struct file *d_filp, ...@@ -1330,10 +1397,12 @@ ext4_move_extents(struct file *o_filp, struct file *d_filp,
kfree(holecheck_path); kfree(holecheck_path);
} }
out2: out2:
mext_inode_double_unlock(orig_inode, donor_inode); ret2 = mext_inode_double_unlock(orig_inode, donor_inode);
if (ret) if (ret1)
return ret; return ret1;
else if (ret2)
return ret2;
return 0; return 0;
} }
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment