Hi Peng, Peng Tao wrote: >>> After applying the two patches, I run my test case with first.img as the orig file (and middle.img or >>> last.img as donor file). My kernel panics and I find following message in /var/log/messages after reboot: >> I could not reproduce this panic. >> Would you tell me about your test environment (1-5)? >> >> 1. What is your kernel version? (2.6.31-rc2 + ext4 patch queue + my patch?) > Only 2.6.31-rc2 from linus tree + your two patches. I didn't apply ext4 patch queue. >> 2. What FS mount options are enabled? > rw,noatime,relatime,commit=360 >> 3. What options are enabled to create ext4? > [bergwolf@~]$sudo tune2fs -l /dev/sda9 > tune2fs 1.41.9 (22-Aug-2009) > Filesystem volume name: <none> > Last mounted on: /other > Filesystem UUID: 90548cb8-5748-4b18-bbe9-e7254439cb82 > Filesystem magic number: 0xEF53 > Filesystem revision #: 1 (dynamic) > Filesystem features: has_journal ext_attr resize_inode dir_index filetype needs_recovery extent flex_bg sparse_super large_file huge_file uninit_bg dir_nlink extra_isize > Filesystem flags: signed_directory_hash > Default mount options: (none) > Filesystem state: clean > Errors behavior: Continue > Filesystem OS type: Linux > Inode count: 125184 > Block count: 500015 > Reserved block count: 25000 > Free blocks: 299959 > Free inodes: 125162 > First block: 0 > Block size: 4096 > Fragment size: 4096 > Reserved GDT blocks: 122 > Blocks per group: 32768 > Fragments per group: 32768 > Inodes per group: 7824 > Inode blocks per group: 489 > Flex block group size: 16 > Filesystem created: Sun Sep 7 15:13:09 2008 > Last mount time: Tue Sep 8 15:19:44 2009 > Last write time: Tue Sep 8 15:19:44 2009 > Mount count: 13 > Maximum mount count: 36 > Last checked: Fri Sep 4 20:56:50 2009 > Check interval: 15552000 (6 months) > Next check after: Wed Mar 3 20:56:50 2010 > Lifetime writes: 1128 MB > Reserved blocks uid: 0 (user root) > Reserved blocks gid: 0 (group root) > First inode: 11 > Inode size: 256 > Required extra isize: 28 > Desired extra isize: 28 > Journal inode: 8 > Default directory hash: tea > Directory Hash Seed: 3c5f2a77-c446-4124-94f7-958ba6155f37 > Journal backup: inode blocks >> 4. Are image files (first.img, middle.img and last.img) >> same as your previous mail? >> http://marc.info/?l=linux-ext4&m=124992522305319&w=2 > Yes. >> 5. What arguments are set to EXT4_IOC_MOVE_EXT in your test case? > move_data.donor_fd = donor_fd; > move_data.orig_start = 0; > move_data.donor_start = 0; > move_data.len = SECTOR_TO_BLOCK(statbuf.st_blocks, fs.f_bsize); > > err = ioctl(orig_fd, EXT4_IOC_MOVE_EXT, &move_data); I tried to reproduce your problem with the following steps, but I couldn't. Please check my procedure. 1. Test environment linux2.6.31-rc2 + two patches as follows: http://marc.info/?l=linux-ext4&m=125186152727422&w=3 http://marc.info/?l=linux-ext4&m=125205817410548&w=3 2. Create ext4 filesystem and mount it mke2fs -t ext4 -b 4096 /dev/XXX mount -t ext4 -O rw,noatime,relatime,commit=360 /dev/XXX /mnt/XXX 3. Create orig and donor files dd if=/dev/urandom of=/mnt/XXX/first.img bs=10M count=1 dd if=/dev/zero of=/mnt/XXX/first.img bs=10M count=0 seek=50 dd if=/dev/urandom of=/mnt/XXX/last.img bs=10M count=1 seek=49 4. Call EXT4_MOVE_MOVE_EXT ioctl to files with the following arguments (orig_file = /mnt/XXX/first.img, donor_file = /mnt/XXX/last.img) move_data.orig_start = 0; move_data.donor_start = 0; move_data.len = 12152; ioctl(orig_fd, EXT4_IOC_MOVE_EXT, &move_data) However, EXT4_IOC_MOVE_EXT returned ENODATA (this is a fine result by my patch) and it didn't occur the kernel panic which you got. If I chose a wrong step, please correct me. I appreciate if you could test following environment. - 2.6.31-rc8 + ext4 patch queue (commit:bdacccd94d08a3f89e7fca676b28aa262c6d75fa) + attached patch Regards, Akira Fujita --- fs/ext4/move_extent.c | 72 +++++++++++++++++++++++++++++++++--------------- 1 files changed, 49 insertions(+), 23 deletions(-) diff --git a/fs/ext4/move_extent.c b/fs/ext4/move_extent.c index 7bfbd58..7266247 100644 --- a/fs/ext4/move_extent.c +++ b/fs/ext4/move_extent.c @@ -25,6 +25,8 @@ if (IS_ERR(path)) { \ ret = PTR_ERR(path); \ path = NULL; \ + } else if (path[ext_depth(inode)].p_ext == NULL) { \ + ret = -ENODATA; \ } \ } while (0) @@ -284,7 +286,7 @@ mext_insert_across_blocks(handle_t *handle, struct inode *orig_inode, if (new_flag) { get_ext_path(orig_path, orig_inode, eblock, err); - if (orig_path == NULL) + if (err) goto out; if (ext4_ext_insert_extent(handle, orig_inode, @@ -295,7 +297,7 @@ mext_insert_across_blocks(handle_t *handle, struct inode *orig_inode, if (end_flag) { get_ext_path(orig_path, orig_inode, le32_to_cpu(end_ext->ee_block) - 1, err); - if (orig_path == NULL) + if (err) goto out; if (ext4_ext_insert_extent(handle, orig_inode, @@ -554,8 +556,10 @@ mext_leaf_block(handle_t *handle, struct inode *orig_inode, * @orig_off: block offset of original inode * @donor_off: block offset of donor inode * @max_count: the maximun length of extents + * + * Return 0 on success, or a negative error value on failure. */ -static void +static int mext_calc_swap_extents(struct ext4_extent *tmp_dext, struct ext4_extent *tmp_oext, ext4_lblk_t orig_off, ext4_lblk_t donor_off, @@ -564,6 +568,19 @@ mext_calc_swap_extents(struct ext4_extent *tmp_dext, ext4_lblk_t diff, orig_diff; struct ext4_extent dext_old, oext_old; + BUG_ON(orig_off != donor_off); + + /* original and donor extents have to cover the same block offset */ + if (orig_off < le32_to_cpu(tmp_oext->ee_block) || + le32_to_cpu(tmp_oext->ee_block) + + ext4_ext_get_actual_len(tmp_oext) - 1 < orig_off) + return -ENODATA; + + if (orig_off < le32_to_cpu(tmp_dext->ee_block) || + le32_to_cpu(tmp_dext->ee_block) + + ext4_ext_get_actual_len(tmp_dext) - 1 < orig_off) + return -ENODATA; + dext_old = *tmp_dext; oext_old = *tmp_oext; @@ -591,6 +608,8 @@ mext_calc_swap_extents(struct ext4_extent *tmp_dext, copy_extent_status(&oext_old, tmp_dext); copy_extent_status(&dext_old, tmp_oext); + + return 0; } /** @@ -632,12 +651,12 @@ mext_replace_branches(handle_t *handle, struct inode *orig_inode, /* Get the original extent for the block "orig_off" */ get_ext_path(orig_path, orig_inode, orig_off, err); - if (orig_path == NULL) + if (err) goto out; /* Get the donor extent for the head */ get_ext_path(donor_path, donor_inode, donor_off, err); - if (donor_path == NULL) + if (err) goto out; depth = ext_depth(orig_inode); oext = orig_path[depth].p_ext; @@ -647,8 +666,10 @@ mext_replace_branches(handle_t *handle, struct inode *orig_inode, dext = donor_path[depth].p_ext; tmp_dext = *dext; - mext_calc_swap_extents(&tmp_dext, &tmp_oext, orig_off, + err = mext_calc_swap_extents(&tmp_dext, &tmp_oext, orig_off, donor_off, count); + if (err) + goto out; /* Loop for the donor extents */ while (1) { @@ -679,7 +700,7 @@ mext_replace_branches(handle_t *handle, struct inode *orig_inode, if (orig_path) ext4_ext_drop_refs(orig_path); get_ext_path(orig_path, orig_inode, orig_off, err); - if (orig_path == NULL) + if (err) goto out; depth = ext_depth(orig_inode); oext = orig_path[depth].p_ext; @@ -694,7 +715,7 @@ mext_replace_branches(handle_t *handle, struct inode *orig_inode, ext4_ext_drop_refs(donor_path); get_ext_path(donor_path, donor_inode, donor_off, err); - if (donor_path == NULL) + if (err) goto out; depth = ext_depth(donor_inode); dext = donor_path[depth].p_ext; @@ -705,9 +726,10 @@ mext_replace_branches(handle_t *handle, struct inode *orig_inode, } tmp_dext = *dext; - mext_calc_swap_extents(&tmp_dext, &tmp_oext, orig_off, - donor_off, - count - replaced_count); + err = mext_calc_swap_extents(&tmp_dext, &tmp_oext, orig_off, + donor_off, count - replaced_count); + if (err) + goto out; } out: @@ -1156,27 +1178,27 @@ ext4_move_extents(struct file *o_filp, struct file *d_filp, len -= block_end - file_end; get_ext_path(orig_path, orig_inode, block_start, ret); - if (orig_path == NULL) + if (ret) goto out2; /* Get path structure to check the hole */ get_ext_path(holecheck_path, orig_inode, block_start, ret); - if (holecheck_path == NULL) + if (ret) goto out; depth = ext_depth(orig_inode); ext_cur = holecheck_path[depth].p_ext; - if (ext_cur == NULL) { - ret = -EINVAL; - goto out; - } /* - * Get proper extent whose ee_block is beyond block_start - * if block_start was within the hole. + * Get proper starting location of block replacement if block_start was + * within the hole. */ if (le32_to_cpu(ext_cur->ee_block) + ext4_ext_get_actual_len(ext_cur) - 1 < block_start) { + /* + * The hole exists between extents or the tail of + * original file. + */ last_extent = mext_next_extent(orig_inode, holecheck_path, &ext_cur); if (last_extent < 0) { @@ -1189,8 +1211,12 @@ ext4_move_extents(struct file *o_filp, struct file *d_filp, ret = last_extent; goto out; } - } - seq_start = block_start; + seq_start = le32_to_cpu(ext_cur->ee_block); + } else if (le32_to_cpu(ext_cur->ee_block) > block_start) + /* The hole exists at the beginning of original file. */ + seq_start = le32_to_cpu(ext_cur->ee_block); + else + seq_start = block_start; /* No blocks within the specified range. */ if (le32_to_cpu(ext_cur->ee_block) > block_end) { @@ -1292,7 +1318,7 @@ ext4_move_extents(struct file *o_filp, struct file *d_filp, ext4_ext_drop_refs(holecheck_path); get_ext_path(holecheck_path, orig_inode, seq_start, ret); - if (holecheck_path == NULL) + if (ret) break; depth = holecheck_path->p_depth; @@ -1300,7 +1326,7 @@ ext4_move_extents(struct file *o_filp, struct file *d_filp, if (orig_path) ext4_ext_drop_refs(orig_path); get_ext_path(orig_path, orig_inode, seq_start, ret); - if (orig_path == NULL) + if (ret) break; ext_cur = holecheck_path[depth].p_ext; -- 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
