[PATCH] ext4: defragmentation code cleanup

[PATCH] ext4: defragmentation code cleanup

[Dmitry Monakhov]

- grab_cache_page_write_begin() may not wait on page's writeback since
  (1d1d1a767206). But it is still reasonable to wait on page's writeback
  here in order to be on the safe side.

- Fix miss typo: pass 'length' instead of 'end' to __block_write_begin()
  https://bugzilla.kernel.org/show_bug.cgi?id=56241

TESTCASE: git://oss.sgi.com/xfs/cmds/xfstests.git
MKFS_OPTIONS="-b1024" ; ./check ext4/304

Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/ext4/move_extent.c |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/fs/ext4/move_extent.c b/fs/ext4/move_extent.c
index 33e1c08..58b33e9 100644
--- a/fs/ext4/move_extent.c
+++ b/fs/ext4/move_extent.c
@@ -737,6 +737,7 @@ mext_replace_branches(handle_t *handle, struct inode *orig_inode,
 		donor_off += dext_alen;
 		orig_off += dext_alen;
 
+		BUG_ON(replaced_count > count);
 		/* Already moved the expected blocks */
 		if (replaced_count >= count)
 			break;
@@ -814,7 +815,13 @@ mext_page_double_lock(struct inode *inode1, struct inode *inode2,
 		page_cache_release(page[0]);
 		return -ENOMEM;
 	}

Re: [PATCH] ext4: defragmentation code cleanup

[Akira Fujita]

Hi,

(2013/04/10 0:15), Dmitry Monakhov wrote:
> - grab_cache_page_write_begin() may not wait on page's writeback since
>    (1d1d1a767206). But it is still reasonable to wait on page's writeback
>    here in order to be on the safe side.
> 
> - Fix miss typo: pass 'length' instead of 'end' to __block_write_begin()
>    https://bugzilla.kernel.org/show_bug.cgi?id=56241

Looks good to me, thanks.

Reviewed-by: Akira Fujita <a-fujita.rs.jp.nec.com>
 
> TESTCASE: git://oss.sgi.com/xfs/cmds/xfstests.git
> MKFS_OPTIONS="-b1024" ; ./check ext4/304
> 
> Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
> ---
>   fs/ext4/move_extent.c |   11 +++++++++--
>   1 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/ext4/move_extent.c b/fs/ext4/move_extent.c
> index 33e1c08..58b33e9 100644
> --- a/fs/ext4/move_extent.c
> +++ b/fs/ext4/move_extent.c
> @@ -737,6 +737,7 @@ mext_replace_branches(handle_t *handle, struct inode *orig_inode,
>   		donor_off += dext_alen;
>   		orig_off += dext_alen;
>   
> +		BUG_ON(replaced_count > count);
>   		/* Already moved the expected blocks */
>   		if (replaced_count >= count)
>   			break;
> @@ -814,7 +815,13 @@ mext_page_double_lock(struct inode *inode1, struct inode *inode2,
>   		page_cache_release(page[0]);
>   		return -ENOMEM;
>   	}
> -
> +	/*
> +	 * grab_cache_page_write_begin() may not wait on page's writeback if
> +	 * BDI not demand that. But it is reasonable to be very conservative
> +	 * here and explicitly wait on page's writeback
> +	 */
> +	wait_on_page_writeback(page[0]);
> +	wait_on_page_writeback(page[1]);
>   	if (inode1 > inode2) {
>   		struct page *tmp;
>   		tmp = page[0];
> @@ -1033,7 +1040,7 @@ data_copy:
>   	}
>   	/* Perform all necessary steps similar write_begin()/write_end()
>   	 * but keeping in mind that i_size will not change */
> -	*err = __block_write_begin(pagep[0], from, from + replaced_size,
> +	*err = __block_write_begin(pagep[0], from, replaced_size,
>   				   ext4_get_block);
>   	if (!*err)
>   		*err = block_commit_write(pagep[0], from, from + replaced_size);
> 

Re: [PATCH] ext4: defragmentation code cleanup

[Theodore Ts'o]

On Wed, Apr 10, 2013 at 03:43:09PM +0900, Akira Fujita wrote:
> Hi,
> 
> (2013/04/10 0:15), Dmitry Monakhov wrote:
> > - grab_cache_page_write_begin() may not wait on page's writeback since
> >    (1d1d1a767206). But it is still reasonable to wait on page's writeback
> >    here in order to be on the safe side.
> > 
> > - Fix miss typo: pass 'length' instead of 'end' to __block_write_begin()
> >    https://bugzilla.kernel.org/show_bug.cgi?id=56241
> 
> Looks good to me, thanks.
> 
> Reviewed-by: Akira Fujita <a-fujita.rs.jp.nec.com>

Applied, thanks.

					- Ted