Vasily Averin <v...@virtuozzo.com> writes: > Dima, > please take look at comment below. > > On 2017-02-25 18:16, Dmitry Monakhov wrote: >> Inode preallocation consists of two parts (used and unused) fully controlled >> by inode, so it must be discarded before swap extents. >> Currently we may skip drop_preallocation if file is sparse. >> >> This patch does: >> - Moves ext4_discard_preallocations to ext4_swap_extents. >> This makes more readable and reliable for future changes. >> - Cleanup main move_extent loop >> >> https://jira.sw.ru/browse/PSBM-57003 >> xfstests:ext4/024 (pended: >> https://github.com/dmonakhov/xfstests/commit/7a4763963f73ea5d5bba45eefa484494aa3df7cf) >> Signed-off-by: Dmitry Monakhov <dmonak...@openvz.org> >> --- >> fs/ext4/extents.c | 3 +++ >> fs/ext4/move_extent.c | 17 +++++++---------- >> 2 files changed, 10 insertions(+), 10 deletions(-) >> >> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c >> index 85c4d4e..fd49ab0 100644 >> --- a/fs/ext4/extents.c >> +++ b/fs/ext4/extents.c >> @@ -4371,6 +4371,9 @@ ext4_swap_extents(handle_t *handle, struct inode >> *inode1, >> BUG_ON(!mutex_is_locked(&inode1->i_mutex)); >> BUG_ON(!mutex_is_locked(&inode1->i_mutex)); >> >> + ext4_discard_preallocations(inode1); >> + ext4_discard_preallocations(inode2); >> + >> while (count) { >> struct ext4_extent *ex1, *ex2, tmp_ex; >> ext4_lblk_t e1_blk, e2_blk; >> diff --git a/fs/ext4/move_extent.c b/fs/ext4/move_extent.c >> index 39eaa8f..97a7db5 100644 >> --- a/fs/ext4/move_extent.c >> +++ b/fs/ext4/move_extent.c >> @@ -628,6 +628,7 @@ ext4_move_extents(struct file *o_filp, struct file >> *d_filp, __u64 orig_blk, >> ext4_lblk_t o_end, o_start = orig_blk; >> ext4_lblk_t d_start = donor_blk; >> int ret; >> + __u64 m_len = *moved_len; >> >> if (orig_inode->i_sb != donor_inode->i_sb) { >> ext4_debug("ext4 move extent: The argument files " >> @@ -696,7 +697,7 @@ ext4_move_extents(struct file *o_filp, struct file >> *d_filp, __u64 orig_blk, >> if (next_blk == EXT_MAX_BLOCKS) { >> o_start = o_end; >> ret = -ENODATA; >> - goto out; >> + break; >> } >> d_start += next_blk - o_start; >> o_start = next_blk; >> @@ -708,7 +709,7 @@ ext4_move_extents(struct file *o_filp, struct file >> *d_filp, __u64 orig_blk, >> o_start = cur_blk; >> /* Extent inside requested range ?*/ >> if (cur_blk >= o_end) >> - goto out; >> + break; >> } else { /* in_range(o_start, o_blk, o_len) */ >> cur_len += cur_blk - o_start; >> } >> @@ -743,6 +744,7 @@ ext4_move_extents(struct file *o_filp, struct file >> *d_filp, __u64 orig_blk, >> break; >> o_start += cur_len; >> d_start += cur_len; >> + m_len += cur_len; >> repeat: >> if (path) { >> ext4_ext_drop_refs(path); >> @@ -755,15 +757,10 @@ ext4_move_extents(struct file *o_filp, struct file >> *d_filp, __u64 orig_blk, >> *moved_len = len; >> >> out: >> - if (*moved_len) { >> - ext4_discard_preallocations(orig_inode); >> - ext4_discard_preallocations(donor_inode); >> - } >> + WARN_ON(m_len > len); >> + if (ret == 0) >> + *moved_len = m_len; >> >> - if (path) { >> - ext4_ext_drop_refs(path); >> - kfree(path); >> - } > > I do not understand why kfree for path is dropped here. > Rest places looks reasonable for me, > but this one looks like some mistake. Yes, this is copy-paste mistake. Please see updated verstion
From: Dmitry Monakhov <dmonak...@openvz.org> To: devel@openvz.org Cc: dmonak...@openvz.org, v...@virtuozzo.com Subject: [PATCH] vz6 ext4: Discard preallocated block before swap_extents v2 Date: Mon, 27 Feb 2017 15:33:07 +0400 Message-Id: <1488195187-26606-1-git-send-email-dmonak...@openvz.org> > > Take a look -- path is still was freed inside cycle, > why it should not be freed at finish too? > >> up_write(&EXT4_I(orig_inode)->i_data_sem); >> up_write(&EXT4_I(donor_inode)->i_data_sem); >> up_write(&orig_inode->i_alloc_sem); >> _______________________________________________ Devel mailing list Devel@openvz.org https://lists.openvz.org/mailman/listinfo/devel