[PATCH 0/4] Btrfs: just bunch of patches to ioctl.c

[PATCH 0/4] Btrfs: just bunch of patches to ioctl.c

[Timofey Titovets]

1st patch, remove 16MiB restriction from extent_same ioctl(),
by doing iterations over passed range.

I did not see much difference in performance, so it's just remove
logic restriction.

2-3 pathes, update defrag ioctl():
 - Fix bad behaviour with full rewriting all compressed
   extents in defrag range. (that also make autodefrag on compressed fs
   not so expensive)
 - Allow userspace specify NONE as target compression type,
   that allow users to uncompress files by defragmentation with btrfs-progs
 - Make defrag ioctl understood requested compression type and current
   compression type of extents, to make btrfs fi def -rc<type>
   idempotent operation.
   i.e. now possible to say, make all extents compressed with lzo,
   and btrfs will not recompress lzo compressed data.
   Same for zlib, zstd, none.
   (patch to btrfs-progs in PR on kdave GitHub).

4th patch, reduce size of struct btrfs_inode
 - btrfs_inode store fields like: prop_compress, defrag_compress and
   after 3rd patch, change_compress.
   They use unsigned as a type, and use 12 bytes in sum.
   But change_compress is a bitflag, and prop_compress/defrag_compress
   only store compression type, that currently use 0-3 of 2^32-1.
   
   So, set a bitfields on that vars, and reduce size of btrfs_inode:
   1136 -> 1128.

Timofey Titovets (4):
  Btrfs: btrfs_dedupe_file_range() ioctl, remove 16MiB restriction
  Btrfs: make should_defrag_range() understood compressed extents
  Btrfs: allow btrfs_defrag_file() uncompress files on defragmentation
  Btrfs: reduce size of struct btrfs_inode

 fs/btrfs/btrfs_inode.h |   5 +-
 fs/btrfs/inode.c       |   4 +-
 fs/btrfs/ioctl.c       | 203 +++++++++++++++++++++++++++++++------------------
 3 files changed, 133 insertions(+), 79 deletions(-)

-- 
2.15.1

[PATCH 1/4] Btrfs: btrfs_dedupe_file_range() ioctl, remove 16MiB restriction

[Timofey Titovets]

At now btrfs_dedupe_file_range() restricted to 16MiB range for
limit locking time and memory requirement for dedup ioctl()

For too big input range code silently set range to 16MiB

Let's remove that restriction by do iterating over dedup range.
That's backward compatible and will not change anything for request
less then 16MiB.

Changes:
  v1 -> v2:
    - Refactor btrfs_cmp_data_prepare and btrfs_extent_same
    - Store memory of pages array between iterations
    - Lock inodes once, not on each iteration
    - Small inplace cleanups

Signed-off-by: Timofey Titovets <nefelim4ag@gmail.com>
---
 fs/btrfs/ioctl.c | 160 ++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 94 insertions(+), 66 deletions(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index be5bd81b3669..45a47d0891fc 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -2965,8 +2965,8 @@ static void btrfs_cmp_data_free(struct cmp_pages *cmp)
 			put_page(pg);
 		}
 	}
-	kfree(cmp->src_pages);
-	kfree(cmp->dst_pages);
+
+	cmp->num_pages = 0;
 }
 
 static int btrfs_cmp_data_prepare(struct inode *src, u64 loff,
@@ -2974,41 +2974,22 @@ static int btrfs_cmp_data_prepare(struct inode *src, u64 loff,
 				  u64 len, struct cmp_pages *cmp)
 {
 	int ret;
-	int num_pages = PAGE_ALIGN(len) >> PAGE_SHIFT;
-	struct page **src_pgarr, **dst_pgarr;
-
-	/*
-	 * We must gather up all the pages before we initiate our
-	 * extent locking. We use an array for the page pointers. Size
-	 * of the array is bounded by len, which is in turn bounded by
-	 * BTRFS_MAX_DEDUPE_LEN.
-	 */
-	src_pgarr = kcalloc(num_pages, sizeof(struct page *), GFP_KERNEL);
-	dst_pgarr = kcalloc(num_pages, sizeof(struct page *), GFP_KERNEL);
-	if (!src_pgarr || !dst_pgarr) {
-		kfree(src_pgarr);
-		kfree(dst_pgarr);
-		return -ENOMEM;
-	}
-	cmp->num_pages = num_pages;
-	cmp->src_pages = src_pgarr;
-	cmp->dst_pages = dst_pgarr;
 
 	/*
 	 * If deduping ranges in the same inode, locking rules make it mandatory
 	 * to always lock pages in ascending order to avoid deadlocks with
 	 * concurrent tasks (such as starting writeback/delalloc).
 	 */
-	if (src == dst && dst_loff < loff) {
-		swap(src_pgarr, dst_pgarr);
+	if (src == dst && dst_loff < loff)
 		swap(loff, dst_loff);
-	}
 
-	ret = gather_extent_pages(src, src_pgarr, cmp->num_pages, loff);
+	cmp->num_pages = PAGE_ALIGN(len) >> PAGE_SHIFT;
+
+	ret = gather_extent_pages(src, cmp->src_pages, cmp->num_pages, loff);
 	if (ret)
 		goto out;
 
-	ret = gather_extent_pages(dst, dst_pgarr, cmp->num_pages, dst_loff);
+	ret = gather_extent_pages(dst, cmp->dst_pages, cmp->num_pages, dst_loff);
 
 out:
 	if (ret)
@@ -3078,31 +3059,23 @@ static int extent_same_check_offsets(struct inode *inode, u64 off, u64 *plen,
 	return 0;
 }
 
-static int btrfs_extent_same(struct inode *src, u64 loff, u64 olen,
-			     struct inode *dst, u64 dst_loff)
+static int __btrfs_extent_same(struct inode *src, u64 loff, u64 olen,
+			       struct inode *dst, u64 dst_loff,
+			       struct cmp_pages *cmp)
 {
 	int ret;
 	u64 len = olen;
-	struct cmp_pages cmp;
 	bool same_inode = (src == dst);
 	u64 same_lock_start = 0;
 	u64 same_lock_len = 0;
 
-	if (len == 0)
-		return 0;
-
-	if (same_inode)
-		inode_lock(src);
-	else
-		btrfs_double_inode_lock(src, dst);
-
 	ret = extent_same_check_offsets(src, loff, &len, olen);
 	if (ret)
-		goto out_unlock;
+		return ret;
 
 	ret = extent_same_check_offsets(dst, dst_loff, &len, olen);
 	if (ret)
-		goto out_unlock;
+		return ret;
 
 	if (same_inode) {
 		/*
@@ -3119,32 +3092,21 @@ static int btrfs_extent_same(struct inode *src, u64 loff, u64 olen,
 		 * allow an unaligned length so long as it ends at
 		 * i_size.
 		 */
-		if (len != olen) {
-			ret = -EINVAL;
-			goto out_unlock;
-		}
+		if (len != olen)
+			return -EINVAL;
 
 		/* Check for overlapping ranges */
-		if (dst_loff + len > loff && dst_loff < loff + len) {
-			ret = -EINVAL;
-			goto out_unlock;
-		}
+		if (dst_loff + len > loff && dst_loff < loff + len)
+			return -EINVAL;
 
 		same_lock_start = min_t(u64, loff, dst_loff);
 		same_lock_len = max_t(u64, loff, dst_loff) + len - same_lock_start;
 	}
 
-	/* don't make the dst file partly checksummed */
-	if ((BTRFS_I(src)->flags & BTRFS_INODE_NODATASUM) !=
-	    (BTRFS_I(dst)->flags & BTRFS_INODE_NODATASUM)) {
-		ret = -EINVAL;
-		goto out_unlock;
-	}
-
 again:
-	ret = btrfs_cmp_data_prepare(src, loff, dst, dst_loff, olen, &cmp);
+	ret = btrfs_cmp_data_prepare(src, loff, dst, dst_loff, olen, cmp);
 	if (ret)
-		goto out_unlock;
+		return ret;
 
 	if (same_inode)
 		ret = lock_extent_range(src, same_lock_start, same_lock_len,
@@ -3165,7 +3127,7 @@ static int btrfs_extent_same(struct inode *src, u64 loff, u64 olen,
 		 * Ranges in the io trees already unlocked. Now unlock all
 		 * pages before waiting for all IO to complete.
 		 */
-		btrfs_cmp_data_free(&cmp);
+		btrfs_cmp_data_free(cmp);
 		if (same_inode) {
 			btrfs_wait_ordered_range(src, same_lock_start,
 						 same_lock_len);
@@ -3178,12 +3140,12 @@ static int btrfs_extent_same(struct inode *src, u64 loff, u64 olen,
 	ASSERT(ret == 0);
 	if (WARN_ON(ret)) {
 		/* ranges in the io trees already unlocked */
-		btrfs_cmp_data_free(&cmp);
+		btrfs_cmp_data_free(cmp);
 		return ret;
 	}
 
 	/* pass original length for comparison so we stay within i_size */
-	ret = btrfs_cmp_data(olen, &cmp);
+	ret = btrfs_cmp_data(olen, cmp);
 	if (ret == 0)
 		ret = btrfs_clone(src, dst, loff, olen, len, dst_loff, 1);
 
@@ -3193,8 +3155,79 @@ static int btrfs_extent_same(struct inode *src, u64 loff, u64 olen,
 	else
 		btrfs_double_extent_unlock(src, loff, dst, dst_loff, len);
 
-	btrfs_cmp_data_free(&cmp);
-out_unlock:
+	btrfs_cmp_data_free(cmp);
+
+	return ret;
+}
+
+#define BTRFS_MAX_DEDUPE_LEN	SZ_16M
+
+static int btrfs_extent_same(struct inode *src, u64 loff, u64 olen,
+				 struct inode *dst, u64 dst_loff)
+{
+	int ret;
+	int num_pages;
+	bool same_inode = (src == dst);
+	u64 i, tail_len, chunk_count;
+	struct cmp_pages cmp;
+
+	if (olen == 0)
+		return 0;
+
+	/* don't make the dst file partly checksummed */
+	if ((BTRFS_I(src)->flags & BTRFS_INODE_NODATASUM) !=
+	    (BTRFS_I(dst)->flags & BTRFS_INODE_NODATASUM)) {
+		return -EINVAL;
+	}
+
+	if (same_inode)
+		inode_lock(src);
+	else
+		btrfs_double_inode_lock(src, dst);
+
+	tail_len = olen % BTRFS_MAX_DEDUPE_LEN;
+	chunk_count = div_u64(olen, BTRFS_MAX_DEDUPE_LEN);
+
+	if (chunk_count > 0) {
+		num_pages = PAGE_ALIGN(BTRFS_MAX_DEDUPE_LEN) >> PAGE_SHIFT;
+	} else {
+		num_pages = PAGE_ALIGN(tail_len) >> PAGE_SHIFT;
+	}
+	/*
+	 * We must gather up all the pages before we initiate our
+	 * extent locking. We use an array for the page pointers. Size
+	 * of the array is bounded by len, which is in turn bounded by
+	 * BTRFS_MAX_DEDUPE_LEN.
+	 */
+	ret = -ENOMEM;
+	cmp.src_pages = kcalloc(num_pages, sizeof(struct page *),
+				GFP_KERNEL);
+	if (!cmp.src_pages)
+		goto out;
+	cmp.dst_pages = kcalloc(num_pages, sizeof(struct page *),
+				GFP_KERNEL);
+	if (!cmp.dst_pages)
+		goto out;
+
+
+	for (i = 0; i < chunk_count; i++) {
+		ret = __btrfs_extent_same(src, loff, BTRFS_MAX_DEDUPE_LEN,
+					  dst, dst_loff, &cmp);
+		if (ret)
+			goto out;
+
+		loff += BTRFS_MAX_DEDUPE_LEN;
+		dst_loff += BTRFS_MAX_DEDUPE_LEN;
+	}
+
+	if (tail_len > 0)
+		ret = __btrfs_extent_same(src, loff, tail_len,
+					  dst, dst_loff, &cmp);
+
+out:
+	kfree(cmp.src_pages);
+	kfree(cmp.dst_pages);
+
 	if (same_inode)
 		inode_unlock(src);
 	else
@@ -3203,8 +3236,6 @@ static int btrfs_extent_same(struct inode *src, u64 loff, u64 olen,
 	return ret;
 }
 
-#define BTRFS_MAX_DEDUPE_LEN	SZ_16M
-
 ssize_t btrfs_dedupe_file_range(struct file *src_file, u64 loff, u64 olen,
 				struct file *dst_file, u64 dst_loff)
 {
@@ -3213,9 +3244,6 @@ ssize_t btrfs_dedupe_file_range(struct file *src_file, u64 loff, u64 olen,
 	u64 bs = BTRFS_I(src)->root->fs_info->sb->s_blocksize;
 	ssize_t res;
 
-	if (olen > BTRFS_MAX_DEDUPE_LEN)
-		olen = BTRFS_MAX_DEDUPE_LEN;
-
 	if (WARN_ON_ONCE(bs < PAGE_SIZE)) {
 		/*
 		 * Btrfs does not support blocksize < page_size. As a
-- 
2.15.1

Re: [PATCH 1/4] Btrfs: btrfs_dedupe_file_range() ioctl, remove 16MiB restriction

[Darrick J. Wong]

On Tue, Dec 19, 2017 at 01:02:44PM +0300, Timofey Titovets wrote:
> At now btrfs_dedupe_file_range() restricted to 16MiB range for
> limit locking time and memory requirement for dedup ioctl()
> 
> For too big input range code silently set range to 16MiB
> 
> Let's remove that restriction by do iterating over dedup range.
> That's backward compatible and will not change anything for request
> less then 16MiB.
> 
> Changes:
>   v1 -> v2:
>     - Refactor btrfs_cmp_data_prepare and btrfs_extent_same
>     - Store memory of pages array between iterations
>     - Lock inodes once, not on each iteration
>     - Small inplace cleanups

/me wonders if you could take advantage of vfs_clone_file_prep_inodes,
which takes care of the content comparison (and flushing files, and inode
checks, etc.) ?

(ISTR Qu Wenruo(??) or someone remarking that this might not work well
with btrfs locking model, but I could be mistaken about all that...)

--D

> 
> Signed-off-by: Timofey Titovets <nefelim4ag@gmail.com>
> ---
>  fs/btrfs/ioctl.c | 160 ++++++++++++++++++++++++++++++++-----------------------
>  1 file changed, 94 insertions(+), 66 deletions(-)
> 
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index be5bd81b3669..45a47d0891fc 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -2965,8 +2965,8 @@ static void btrfs_cmp_data_free(struct cmp_pages *cmp)
>  			put_page(pg);
>  		}
>  	}
> -	kfree(cmp->src_pages);
> -	kfree(cmp->dst_pages);
> +
> +	cmp->num_pages = 0;
>  }
>  
>  static int btrfs_cmp_data_prepare(struct inode *src, u64 loff,
> @@ -2974,41 +2974,22 @@ static int btrfs_cmp_data_prepare(struct inode *src, u64 loff,
>  				  u64 len, struct cmp_pages *cmp)
>  {
>  	int ret;
> -	int num_pages = PAGE_ALIGN(len) >> PAGE_SHIFT;
> -	struct page **src_pgarr, **dst_pgarr;
> -
> -	/*
> -	 * We must gather up all the pages before we initiate our
> -	 * extent locking. We use an array for the page pointers. Size
> -	 * of the array is bounded by len, which is in turn bounded by
> -	 * BTRFS_MAX_DEDUPE_LEN.
> -	 */
> -	src_pgarr = kcalloc(num_pages, sizeof(struct page *), GFP_KERNEL);
> -	dst_pgarr = kcalloc(num_pages, sizeof(struct page *), GFP_KERNEL);
> -	if (!src_pgarr || !dst_pgarr) {
> -		kfree(src_pgarr);
> -		kfree(dst_pgarr);
> -		return -ENOMEM;
> -	}
> -	cmp->num_pages = num_pages;
> -	cmp->src_pages = src_pgarr;
> -	cmp->dst_pages = dst_pgarr;
>  
>  	/*
>  	 * If deduping ranges in the same inode, locking rules make it mandatory
>  	 * to always lock pages in ascending order to avoid deadlocks with
>  	 * concurrent tasks (such as starting writeback/delalloc).
>  	 */
> -	if (src == dst && dst_loff < loff) {
> -		swap(src_pgarr, dst_pgarr);
> +	if (src == dst && dst_loff < loff)
>  		swap(loff, dst_loff);
> -	}
>  
> -	ret = gather_extent_pages(src, src_pgarr, cmp->num_pages, loff);
> +	cmp->num_pages = PAGE_ALIGN(len) >> PAGE_SHIFT;
> +
> +	ret = gather_extent_pages(src, cmp->src_pages, cmp->num_pages, loff);
>  	if (ret)
>  		goto out;
>  
> -	ret = gather_extent_pages(dst, dst_pgarr, cmp->num_pages, dst_loff);
> +	ret = gather_extent_pages(dst, cmp->dst_pages, cmp->num_pages, dst_loff);
>  
>  out:
>  	if (ret)
> @@ -3078,31 +3059,23 @@ static int extent_same_check_offsets(struct inode *inode, u64 off, u64 *plen,
>  	return 0;
>  }
>  
> -static int btrfs_extent_same(struct inode *src, u64 loff, u64 olen,
> -			     struct inode *dst, u64 dst_loff)
> +static int __btrfs_extent_same(struct inode *src, u64 loff, u64 olen,
> +			       struct inode *dst, u64 dst_loff,
> +			       struct cmp_pages *cmp)
>  {
>  	int ret;
>  	u64 len = olen;
> -	struct cmp_pages cmp;
>  	bool same_inode = (src == dst);
>  	u64 same_lock_start = 0;
>  	u64 same_lock_len = 0;
>  
> -	if (len == 0)
> -		return 0;
> -
> -	if (same_inode)
> -		inode_lock(src);
> -	else
> -		btrfs_double_inode_lock(src, dst);
> -
>  	ret = extent_same_check_offsets(src, loff, &len, olen);
>  	if (ret)
> -		goto out_unlock;
> +		return ret;
>  
>  	ret = extent_same_check_offsets(dst, dst_loff, &len, olen);
>  	if (ret)
> -		goto out_unlock;
> +		return ret;
>  
>  	if (same_inode) {
>  		/*
> @@ -3119,32 +3092,21 @@ static int btrfs_extent_same(struct inode *src, u64 loff, u64 olen,
>  		 * allow an unaligned length so long as it ends at
>  		 * i_size.
>  		 */
> -		if (len != olen) {
> -			ret = -EINVAL;
> -			goto out_unlock;
> -		}
> +		if (len != olen)
> +			return -EINVAL;
>  
>  		/* Check for overlapping ranges */
> -		if (dst_loff + len > loff && dst_loff < loff + len) {
> -			ret = -EINVAL;
> -			goto out_unlock;
> -		}
> +		if (dst_loff + len > loff && dst_loff < loff + len)
> +			return -EINVAL;
>  
>  		same_lock_start = min_t(u64, loff, dst_loff);
>  		same_lock_len = max_t(u64, loff, dst_loff) + len - same_lock_start;
>  	}
>  
> -	/* don't make the dst file partly checksummed */
> -	if ((BTRFS_I(src)->flags & BTRFS_INODE_NODATASUM) !=
> -	    (BTRFS_I(dst)->flags & BTRFS_INODE_NODATASUM)) {
> -		ret = -EINVAL;
> -		goto out_unlock;
> -	}
> -
>  again:
> -	ret = btrfs_cmp_data_prepare(src, loff, dst, dst_loff, olen, &cmp);
> +	ret = btrfs_cmp_data_prepare(src, loff, dst, dst_loff, olen, cmp);
>  	if (ret)
> -		goto out_unlock;
> +		return ret;
>  
>  	if (same_inode)
>  		ret = lock_extent_range(src, same_lock_start, same_lock_len,
> @@ -3165,7 +3127,7 @@ static int btrfs_extent_same(struct inode *src, u64 loff, u64 olen,
>  		 * Ranges in the io trees already unlocked. Now unlock all
>  		 * pages before waiting for all IO to complete.
>  		 */
> -		btrfs_cmp_data_free(&cmp);
> +		btrfs_cmp_data_free(cmp);
>  		if (same_inode) {
>  			btrfs_wait_ordered_range(src, same_lock_start,
>  						 same_lock_len);
> @@ -3178,12 +3140,12 @@ static int btrfs_extent_same(struct inode *src, u64 loff, u64 olen,
>  	ASSERT(ret == 0);
>  	if (WARN_ON(ret)) {
>  		/* ranges in the io trees already unlocked */
> -		btrfs_cmp_data_free(&cmp);
> +		btrfs_cmp_data_free(cmp);
>  		return ret;
>  	}
>  
>  	/* pass original length for comparison so we stay within i_size */
> -	ret = btrfs_cmp_data(olen, &cmp);
> +	ret = btrfs_cmp_data(olen, cmp);
>  	if (ret == 0)
>  		ret = btrfs_clone(src, dst, loff, olen, len, dst_loff, 1);
>  
> @@ -3193,8 +3155,79 @@ static int btrfs_extent_same(struct inode *src, u64 loff, u64 olen,
>  	else
>  		btrfs_double_extent_unlock(src, loff, dst, dst_loff, len);
>  
> -	btrfs_cmp_data_free(&cmp);
> -out_unlock:
> +	btrfs_cmp_data_free(cmp);
> +
> +	return ret;
> +}
> +
> +#define BTRFS_MAX_DEDUPE_LEN	SZ_16M
> +
> +static int btrfs_extent_same(struct inode *src, u64 loff, u64 olen,
> +				 struct inode *dst, u64 dst_loff)
> +{
> +	int ret;
> +	int num_pages;
> +	bool same_inode = (src == dst);
> +	u64 i, tail_len, chunk_count;
> +	struct cmp_pages cmp;
> +
> +	if (olen == 0)
> +		return 0;
> +
> +	/* don't make the dst file partly checksummed */
> +	if ((BTRFS_I(src)->flags & BTRFS_INODE_NODATASUM) !=
> +	    (BTRFS_I(dst)->flags & BTRFS_INODE_NODATASUM)) {
> +		return -EINVAL;
> +	}
> +
> +	if (same_inode)
> +		inode_lock(src);
> +	else
> +		btrfs_double_inode_lock(src, dst);
> +
> +	tail_len = olen % BTRFS_MAX_DEDUPE_LEN;
> +	chunk_count = div_u64(olen, BTRFS_MAX_DEDUPE_LEN);
> +
> +	if (chunk_count > 0) {
> +		num_pages = PAGE_ALIGN(BTRFS_MAX_DEDUPE_LEN) >> PAGE_SHIFT;
> +	} else {
> +		num_pages = PAGE_ALIGN(tail_len) >> PAGE_SHIFT;
> +	}
> +	/*
> +	 * We must gather up all the pages before we initiate our
> +	 * extent locking. We use an array for the page pointers. Size
> +	 * of the array is bounded by len, which is in turn bounded by
> +	 * BTRFS_MAX_DEDUPE_LEN.
> +	 */
> +	ret = -ENOMEM;
> +	cmp.src_pages = kcalloc(num_pages, sizeof(struct page *),
> +				GFP_KERNEL);
> +	if (!cmp.src_pages)
> +		goto out;
> +	cmp.dst_pages = kcalloc(num_pages, sizeof(struct page *),
> +				GFP_KERNEL);
> +	if (!cmp.dst_pages)
> +		goto out;
> +
> +
> +	for (i = 0; i < chunk_count; i++) {
> +		ret = __btrfs_extent_same(src, loff, BTRFS_MAX_DEDUPE_LEN,
> +					  dst, dst_loff, &cmp);
> +		if (ret)
> +			goto out;
> +
> +		loff += BTRFS_MAX_DEDUPE_LEN;
> +		dst_loff += BTRFS_MAX_DEDUPE_LEN;
> +	}
> +
> +	if (tail_len > 0)
> +		ret = __btrfs_extent_same(src, loff, tail_len,
> +					  dst, dst_loff, &cmp);
> +
> +out:
> +	kfree(cmp.src_pages);
> +	kfree(cmp.dst_pages);
> +
>  	if (same_inode)
>  		inode_unlock(src);
>  	else
> @@ -3203,8 +3236,6 @@ static int btrfs_extent_same(struct inode *src, u64 loff, u64 olen,
>  	return ret;
>  }
>  
> -#define BTRFS_MAX_DEDUPE_LEN	SZ_16M
> -
>  ssize_t btrfs_dedupe_file_range(struct file *src_file, u64 loff, u64 olen,
>  				struct file *dst_file, u64 dst_loff)
>  {
> @@ -3213,9 +3244,6 @@ ssize_t btrfs_dedupe_file_range(struct file *src_file, u64 loff, u64 olen,
>  	u64 bs = BTRFS_I(src)->root->fs_info->sb->s_blocksize;
>  	ssize_t res;
>  
> -	if (olen > BTRFS_MAX_DEDUPE_LEN)
> -		olen = BTRFS_MAX_DEDUPE_LEN;
> -
>  	if (WARN_ON_ONCE(bs < PAGE_SIZE)) {
>  		/*
>  		 * Btrfs does not support blocksize < page_size. As a
> -- 
> 2.15.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/4] Btrfs: btrfs_dedupe_file_range() ioctl, remove 16MiB restriction

[Timofey Titovets]

2017-12-20 0:23 GMT+03:00 Darrick J. Wong <darrick.wong@oracle.com>:
> On Tue, Dec 19, 2017 at 01:02:44PM +0300, Timofey Titovets wrote:
>> At now btrfs_dedupe_file_range() restricted to 16MiB range for
>> limit locking time and memory requirement for dedup ioctl()
>>
>> For too big input range code silently set range to 16MiB
>>
>> Let's remove that restriction by do iterating over dedup range.
>> That's backward compatible and will not change anything for request
>> less then 16MiB.
>>
>> Changes:
>>   v1 -> v2:
>>     - Refactor btrfs_cmp_data_prepare and btrfs_extent_same
>>     - Store memory of pages array between iterations
>>     - Lock inodes once, not on each iteration
>>     - Small inplace cleanups
>
> /me wonders if you could take advantage of vfs_clone_file_prep_inodes,
> which takes care of the content comparison (and flushing files, and inode
> checks, etc.) ?
>
> (ISTR Qu Wenruo(??) or someone remarking that this might not work well
> with btrfs locking model, but I could be mistaken about all that...)
>
> --D

Sorry, not enough knowledge to give an authoritative answer.
I can only say that, i try lightly test that, by add call before
btrfs_extent_same() with inode_locks,
at least that works.

Thanks.

>>
>> Signed-off-by: Timofey Titovets <nefelim4ag@gmail.com>
>> ---
>>  fs/btrfs/ioctl.c | 160 ++++++++++++++++++++++++++++++++-----------------------
>>  1 file changed, 94 insertions(+), 66 deletions(-)
>>
>> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
>> index be5bd81b3669..45a47d0891fc 100644
>> --- a/fs/btrfs/ioctl.c
>> +++ b/fs/btrfs/ioctl.c
>> @@ -2965,8 +2965,8 @@ static void btrfs_cmp_data_free(struct cmp_pages *cmp)
>>                       put_page(pg);
>>               }
>>       }
>> -     kfree(cmp->src_pages);
>> -     kfree(cmp->dst_pages);
>> +
>> +     cmp->num_pages = 0;
>>  }
>>
>>  static int btrfs_cmp_data_prepare(struct inode *src, u64 loff,
>> @@ -2974,41 +2974,22 @@ static int btrfs_cmp_data_prepare(struct inode *src, u64 loff,
>>                                 u64 len, struct cmp_pages *cmp)
>>  {
>>       int ret;
>> -     int num_pages = PAGE_ALIGN(len) >> PAGE_SHIFT;
>> -     struct page **src_pgarr, **dst_pgarr;
>> -
>> -     /*
>> -      * We must gather up all the pages before we initiate our
>> -      * extent locking. We use an array for the page pointers. Size
>> -      * of the array is bounded by len, which is in turn bounded by
>> -      * BTRFS_MAX_DEDUPE_LEN.
>> -      */
>> -     src_pgarr = kcalloc(num_pages, sizeof(struct page *), GFP_KERNEL);
>> -     dst_pgarr = kcalloc(num_pages, sizeof(struct page *), GFP_KERNEL);
>> -     if (!src_pgarr || !dst_pgarr) {
>> -             kfree(src_pgarr);
>> -             kfree(dst_pgarr);
>> -             return -ENOMEM;
>> -     }
>> -     cmp->num_pages = num_pages;
>> -     cmp->src_pages = src_pgarr;
>> -     cmp->dst_pages = dst_pgarr;
>>
>>       /*
>>        * If deduping ranges in the same inode, locking rules make it mandatory
>>        * to always lock pages in ascending order to avoid deadlocks with
>>        * concurrent tasks (such as starting writeback/delalloc).
>>        */
>> -     if (src == dst && dst_loff < loff) {
>> -             swap(src_pgarr, dst_pgarr);
>> +     if (src == dst && dst_loff < loff)
>>               swap(loff, dst_loff);
>> -     }
>>
>> -     ret = gather_extent_pages(src, src_pgarr, cmp->num_pages, loff);
>> +     cmp->num_pages = PAGE_ALIGN(len) >> PAGE_SHIFT;
>> +
>> +     ret = gather_extent_pages(src, cmp->src_pages, cmp->num_pages, loff);
>>       if (ret)
>>               goto out;
>>
>> -     ret = gather_extent_pages(dst, dst_pgarr, cmp->num_pages, dst_loff);
>> +     ret = gather_extent_pages(dst, cmp->dst_pages, cmp->num_pages, dst_loff);
>>
>>  out:
>>       if (ret)
>> @@ -3078,31 +3059,23 @@ static int extent_same_check_offsets(struct inode *inode, u64 off, u64 *plen,
>>       return 0;
>>  }
>>
>> -static int btrfs_extent_same(struct inode *src, u64 loff, u64 olen,
>> -                          struct inode *dst, u64 dst_loff)
>> +static int __btrfs_extent_same(struct inode *src, u64 loff, u64 olen,
>> +                            struct inode *dst, u64 dst_loff,
>> +                            struct cmp_pages *cmp)
>>  {
>>       int ret;
>>       u64 len = olen;
>> -     struct cmp_pages cmp;
>>       bool same_inode = (src == dst);
>>       u64 same_lock_start = 0;
>>       u64 same_lock_len = 0;
>>
>> -     if (len == 0)
>> -             return 0;
>> -
>> -     if (same_inode)
>> -             inode_lock(src);
>> -     else
>> -             btrfs_double_inode_lock(src, dst);
>> -
>>       ret = extent_same_check_offsets(src, loff, &len, olen);
>>       if (ret)
>> -             goto out_unlock;
>> +             return ret;
>>
>>       ret = extent_same_check_offsets(dst, dst_loff, &len, olen);
>>       if (ret)
>> -             goto out_unlock;
>> +             return ret;
>>
>>       if (same_inode) {
>>               /*
>> @@ -3119,32 +3092,21 @@ static int btrfs_extent_same(struct inode *src, u64 loff, u64 olen,
>>                * allow an unaligned length so long as it ends at
>>                * i_size.
>>                */
>> -             if (len != olen) {
>> -                     ret = -EINVAL;
>> -                     goto out_unlock;
>> -             }
>> +             if (len != olen)
>> +                     return -EINVAL;
>>
>>               /* Check for overlapping ranges */
>> -             if (dst_loff + len > loff && dst_loff < loff + len) {
>> -                     ret = -EINVAL;
>> -                     goto out_unlock;
>> -             }
>> +             if (dst_loff + len > loff && dst_loff < loff + len)
>> +                     return -EINVAL;
>>
>>               same_lock_start = min_t(u64, loff, dst_loff);
>>               same_lock_len = max_t(u64, loff, dst_loff) + len - same_lock_start;
>>       }
>>
>> -     /* don't make the dst file partly checksummed */
>> -     if ((BTRFS_I(src)->flags & BTRFS_INODE_NODATASUM) !=
>> -         (BTRFS_I(dst)->flags & BTRFS_INODE_NODATASUM)) {
>> -             ret = -EINVAL;
>> -             goto out_unlock;
>> -     }
>> -
>>  again:
>> -     ret = btrfs_cmp_data_prepare(src, loff, dst, dst_loff, olen, &cmp);
>> +     ret = btrfs_cmp_data_prepare(src, loff, dst, dst_loff, olen, cmp);
>>       if (ret)
>> -             goto out_unlock;
>> +             return ret;
>>
>>       if (same_inode)
>>               ret = lock_extent_range(src, same_lock_start, same_lock_len,
>> @@ -3165,7 +3127,7 @@ static int btrfs_extent_same(struct inode *src, u64 loff, u64 olen,
>>                * Ranges in the io trees already unlocked. Now unlock all
>>                * pages before waiting for all IO to complete.
>>                */
>> -             btrfs_cmp_data_free(&cmp);
>> +             btrfs_cmp_data_free(cmp);
>>               if (same_inode) {
>>                       btrfs_wait_ordered_range(src, same_lock_start,
>>                                                same_lock_len);
>> @@ -3178,12 +3140,12 @@ static int btrfs_extent_same(struct inode *src, u64 loff, u64 olen,
>>       ASSERT(ret == 0);
>>       if (WARN_ON(ret)) {
>>               /* ranges in the io trees already unlocked */
>> -             btrfs_cmp_data_free(&cmp);
>> +             btrfs_cmp_data_free(cmp);
>>               return ret;
>>       }
>>
>>       /* pass original length for comparison so we stay within i_size */
>> -     ret = btrfs_cmp_data(olen, &cmp);
>> +     ret = btrfs_cmp_data(olen, cmp);
>>       if (ret == 0)
>>               ret = btrfs_clone(src, dst, loff, olen, len, dst_loff, 1);
>>
>> @@ -3193,8 +3155,79 @@ static int btrfs_extent_same(struct inode *src, u64 loff, u64 olen,
>>       else
>>               btrfs_double_extent_unlock(src, loff, dst, dst_loff, len);
>>
>> -     btrfs_cmp_data_free(&cmp);
>> -out_unlock:
>> +     btrfs_cmp_data_free(cmp);
>> +
>> +     return ret;
>> +}
>> +
>> +#define BTRFS_MAX_DEDUPE_LEN SZ_16M
>> +
>> +static int btrfs_extent_same(struct inode *src, u64 loff, u64 olen,
>> +                              struct inode *dst, u64 dst_loff)
>> +{
>> +     int ret;
>> +     int num_pages;
>> +     bool same_inode = (src == dst);
>> +     u64 i, tail_len, chunk_count;
>> +     struct cmp_pages cmp;
>> +
>> +     if (olen == 0)
>> +             return 0;
>> +
>> +     /* don't make the dst file partly checksummed */
>> +     if ((BTRFS_I(src)->flags & BTRFS_INODE_NODATASUM) !=
>> +         (BTRFS_I(dst)->flags & BTRFS_INODE_NODATASUM)) {
>> +             return -EINVAL;
>> +     }
>> +
>> +     if (same_inode)
>> +             inode_lock(src);
>> +     else
>> +             btrfs_double_inode_lock(src, dst);
>> +
>> +     tail_len = olen % BTRFS_MAX_DEDUPE_LEN;
>> +     chunk_count = div_u64(olen, BTRFS_MAX_DEDUPE_LEN);
>> +
>> +     if (chunk_count > 0) {
>> +             num_pages = PAGE_ALIGN(BTRFS_MAX_DEDUPE_LEN) >> PAGE_SHIFT;
>> +     } else {
>> +             num_pages = PAGE_ALIGN(tail_len) >> PAGE_SHIFT;
>> +     }
>> +     /*
>> +      * We must gather up all the pages before we initiate our
>> +      * extent locking. We use an array for the page pointers. Size
>> +      * of the array is bounded by len, which is in turn bounded by
>> +      * BTRFS_MAX_DEDUPE_LEN.
>> +      */
>> +     ret = -ENOMEM;
>> +     cmp.src_pages = kcalloc(num_pages, sizeof(struct page *),
>> +                             GFP_KERNEL);
>> +     if (!cmp.src_pages)
>> +             goto out;
>> +     cmp.dst_pages = kcalloc(num_pages, sizeof(struct page *),
>> +                             GFP_KERNEL);
>> +     if (!cmp.dst_pages)
>> +             goto out;
>> +
>> +
>> +     for (i = 0; i < chunk_count; i++) {
>> +             ret = __btrfs_extent_same(src, loff, BTRFS_MAX_DEDUPE_LEN,
>> +                                       dst, dst_loff, &cmp);
>> +             if (ret)
>> +                     goto out;
>> +
>> +             loff += BTRFS_MAX_DEDUPE_LEN;
>> +             dst_loff += BTRFS_MAX_DEDUPE_LEN;
>> +     }
>> +
>> +     if (tail_len > 0)
>> +             ret = __btrfs_extent_same(src, loff, tail_len,
>> +                                       dst, dst_loff, &cmp);
>> +
>> +out:
>> +     kfree(cmp.src_pages);
>> +     kfree(cmp.dst_pages);
>> +
>>       if (same_inode)
>>               inode_unlock(src);
>>       else
>> @@ -3203,8 +3236,6 @@ static int btrfs_extent_same(struct inode *src, u64 loff, u64 olen,
>>       return ret;
>>  }
>>
>> -#define BTRFS_MAX_DEDUPE_LEN SZ_16M
>> -
>>  ssize_t btrfs_dedupe_file_range(struct file *src_file, u64 loff, u64 olen,
>>                               struct file *dst_file, u64 dst_loff)
>>  {
>> @@ -3213,9 +3244,6 @@ ssize_t btrfs_dedupe_file_range(struct file *src_file, u64 loff, u64 olen,
>>       u64 bs = BTRFS_I(src)->root->fs_info->sb->s_blocksize;
>>       ssize_t res;
>>
>> -     if (olen > BTRFS_MAX_DEDUPE_LEN)
>> -             olen = BTRFS_MAX_DEDUPE_LEN;
>> -
>>       if (WARN_ON_ONCE(bs < PAGE_SIZE)) {
>>               /*
>>                * Btrfs does not support blocksize < page_size. As a
>> --
>> 2.15.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Have a nice day,
Timofey.

[PATCH 2/4] Btrfs: make should_defrag_range() understood compressed extents

[Timofey Titovets]

 Both, defrag ioctl and autodefrag - call btrfs_defrag_file()
 for file defragmentation.

 Kernel default target extent size - 256KiB.
 Btrfs progs default - 32MiB.

 Both bigger then maximum size of compressed extent - 128KiB.
 That lead to rewrite all compressed data on disk.

 Fix that by check compression extents with different logic.

 As addition, make should_defrag_range() understood compressed extent type,
 if requested target compression are same as current extent compression type.
 Just don't recompress/rewrite extents.
 To avoid useless recompression of compressed extents.

Signed-off-by: Timofey Titovets <nefelim4ag@gmail.com>
---
 fs/btrfs/ioctl.c | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 45a47d0891fc..b29ea1f0f621 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -1008,7 +1008,7 @@ static bool defrag_check_next_extent(struct inode *inode, struct extent_map *em)
 
 static int should_defrag_range(struct inode *inode, u64 start, u32 thresh,
 			       u64 *last_len, u64 *skip, u64 *defrag_end,
-			       int compress)
+			       int compress, int compress_type)
 {
 	struct extent_map *em;
 	int ret = 1;
@@ -1043,8 +1043,29 @@ static int should_defrag_range(struct inode *inode, u64 start, u32 thresh,
 	 * real extent, don't bother defragging it
 	 */
 	if (!compress && (*last_len == 0 || *last_len >= thresh) &&
-	    (em->len >= thresh || (!next_mergeable && !prev_mergeable)))
+	    (em->len >= thresh || (!next_mergeable && !prev_mergeable))) {
 		ret = 0;
+		goto out;
+	}
+
+
+	/*
+	 * Try not recompress compressed extents
+	 * thresh >= BTRFS_MAX_UNCOMPRESSED will lead to
+	 * recompress all compressed extents
+	 */
+	if (em->compress_type != 0 && thresh >= BTRFS_MAX_UNCOMPRESSED) {
+		if (!compress) {
+			if (em->len == BTRFS_MAX_UNCOMPRESSED)
+				ret = 0;
+		} else {
+			if (em->compress_type != compress_type)
+				goto out;
+			if (em->len == BTRFS_MAX_UNCOMPRESSED)
+				ret = 0;
+		}
+	}
+
 out:
 	/*
 	 * last_len ends up being a counter of how many bytes we've defragged.
@@ -1342,7 +1363,8 @@ int btrfs_defrag_file(struct inode *inode, struct file *file,
 
 		if (!should_defrag_range(inode, (u64)i << PAGE_SHIFT,
 					 extent_thresh, &last_len, &skip,
-					 &defrag_end, do_compress)){
+					 &defrag_end, do_compress,
+					 compress_type)){
 			unsigned long next;
 			/*
 			 * the should_defrag function tells us how much to skip
-- 
2.15.1

Re: [PATCH 2/4] Btrfs: make should_defrag_range() understood compressed extents

[Timofey Titovets]

вт, 19 дек. 2017 г. в 13:02, Timofey Titovets <nefelim4ag@gmail.com>:

>   Both, defrag ioctl and autodefrag - call btrfs_defrag_file()
>   for file defragmentation.

>   Kernel default target extent size - 256KiB.
>   Btrfs progs default - 32MiB.

>   Both bigger then maximum size of compressed extent - 128KiB.
>   That lead to rewrite all compressed data on disk.

>   Fix that by check compression extents with different logic.

>   As addition, make should_defrag_range() understood compressed extent
type,
>   if requested target compression are same as current extent compression
type.
>   Just don't recompress/rewrite extents.
>   To avoid useless recompression of compressed extents.

> Signed-off-by: Timofey Titovets <nefelim4ag@gmail.com>
> ---
>   fs/btrfs/ioctl.c | 28 +++++++++++++++++++++++++---
>   1 file changed, 25 insertions(+), 3 deletions(-)

> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index 45a47d0891fc..b29ea1f0f621 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -1008,7 +1008,7 @@ static bool defrag_check_next_extent(struct inode
*inode, struct extent_map *em)

>   static int should_defrag_range(struct inode *inode, u64 start, u32
thresh,
>                                 u64 *last_len, u64 *skip, u64 *defrag_end,
> -                              int compress)
> +                              int compress, int compress_type)
>   {
>          struct extent_map *em;
>          int ret = 1;
> @@ -1043,8 +1043,29 @@ static int should_defrag_range(struct inode
*inode, u64 start, u32 thresh,
>           * real extent, don't bother defragging it
>           */
>          if (!compress && (*last_len == 0 || *last_len >= thresh) &&
> -           (em->len >= thresh || (!next_mergeable && !prev_mergeable)))
> +           (em->len >= thresh || (!next_mergeable && !prev_mergeable))) {
>                  ret = 0;
> +               goto out;
> +       }
> +
> +
> +       /*
> +        * Try not recompress compressed extents
> +        * thresh >= BTRFS_MAX_UNCOMPRESSED will lead to
> +        * recompress all compressed extents
> +        */
> +       if (em->compress_type != 0 && thresh >= BTRFS_MAX_UNCOMPRESSED) {
> +               if (!compress) {
> +                       if (em->len == BTRFS_MAX_UNCOMPRESSED)
> +                               ret = 0;
> +               } else {
> +                       if (em->compress_type != compress_type)
> +                               goto out;
> +                       if (em->len == BTRFS_MAX_UNCOMPRESSED)
> +                               ret = 0;
> +               }
> +       }
> +
>   out:
>          /*
>           * last_len ends up being a counter of how many bytes we've
defragged.
> @@ -1342,7 +1363,8 @@ int btrfs_defrag_file(struct inode *inode, struct
file *file,

>                  if (!should_defrag_range(inode, (u64)i << PAGE_SHIFT,
>                                           extent_thresh, &last_len, &skip,
> -                                        &defrag_end, do_compress)){
> +                                        &defrag_end, do_compress,
> +                                        compress_type)){
>                          unsigned long next;
>                          /*
>                           * the should_defrag function tells us how much
to skip
> --
> 2.15.1

May be, then, if we don't want add some duck tape for compressed extents
and defrag,
we can just change default kernel target extent size 256KiB -> 128KiB?

That will also fix the issue with autodefrag and compression enabled.

Thanks.
-- 
Have a nice day,
Timofey.

[PATCH 3/4] Btrfs: allow btrfs_defrag_file() uncompress files on defragmentation

[Timofey Titovets]

Currently defrag ioctl only support recompress files with specified
compression type.
Allow set compression type to none, while call defrag, and use
BTRFS_DEFRAG_RANGE_COMPRESS as flag, that user request change of compression type.

Signed-off-by: Timofey Titovets <nefelim4ag@gmail.com>
---
 fs/btrfs/btrfs_inode.h |  1 +
 fs/btrfs/inode.c       |  4 ++--
 fs/btrfs/ioctl.c       | 17 ++++++++++-------
 3 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
index 63f0ccc92a71..9eb0c92ee4b4 100644
--- a/fs/btrfs/btrfs_inode.h
+++ b/fs/btrfs/btrfs_inode.h
@@ -187,6 +187,7 @@ struct btrfs_inode {
 	 * different from prop_compress and takes precedence if set
 	 */
 	unsigned defrag_compress;
+	unsigned change_compress;
 
 	struct btrfs_delayed_node *delayed_node;
 
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 46df5e2a64e7..7af8f1784788 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -412,8 +412,8 @@ static inline int inode_need_compress(struct inode *inode, u64 start, u64 end)
 	if (btrfs_test_opt(fs_info, FORCE_COMPRESS))
 		return 1;
 	/* defrag ioctl */
-	if (BTRFS_I(inode)->defrag_compress)
-		return 1;
+	if (BTRFS_I(inode)->change_compress)
+		return BTRFS_I(inode)->defrag_compress;
 	/* bad compression ratios */
 	if (BTRFS_I(inode)->flags & BTRFS_INODE_NOCOMPRESS)
 		return 0;
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index b29ea1f0f621..40f5e5678eac 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -1276,7 +1276,7 @@ int btrfs_defrag_file(struct inode *inode, struct file *file,
 	unsigned long cluster = max_cluster;
 	u64 new_align = ~((u64)SZ_128K - 1);
 	struct page **pages = NULL;
-	bool do_compress = range->flags & BTRFS_DEFRAG_RANGE_COMPRESS;
+	bool change_compress = range->flags & BTRFS_DEFRAG_RANGE_COMPRESS;
 
 	if (isize == 0)
 		return 0;
@@ -1284,11 +1284,10 @@ int btrfs_defrag_file(struct inode *inode, struct file *file,
 	if (range->start >= isize)
 		return -EINVAL;
 
-	if (do_compress) {
+	if (change_compress) {
 		if (range->compress_type > BTRFS_COMPRESS_TYPES)
 			return -EINVAL;
-		if (range->compress_type)
-			compress_type = range->compress_type;
+		compress_type = range->compress_type;
 	}
 
 	if (extent_thresh == 0)
@@ -1363,7 +1362,7 @@ int btrfs_defrag_file(struct inode *inode, struct file *file,
 
 		if (!should_defrag_range(inode, (u64)i << PAGE_SHIFT,
 					 extent_thresh, &last_len, &skip,
-					 &defrag_end, do_compress,
+					 &defrag_end, change_compress,
 					 compress_type)){
 			unsigned long next;
 			/*
@@ -1392,8 +1391,11 @@ int btrfs_defrag_file(struct inode *inode, struct file *file,
 		}
 
 		inode_lock(inode);
-		if (do_compress)
+		if (change_compress) {
+			BTRFS_I(inode)->change_compress = change_compress;
 			BTRFS_I(inode)->defrag_compress = compress_type;
+		}
+
 		ret = cluster_pages_for_defrag(inode, pages, i, cluster);
 		if (ret < 0) {
 			inode_unlock(inode);
@@ -1449,8 +1451,9 @@ int btrfs_defrag_file(struct inode *inode, struct file *file,
 	ret = defrag_count;
 
 out_ra:
-	if (do_compress) {
+	if (change_compress) {
 		inode_lock(inode);
+		BTRFS_I(inode)->change_compress = 0;
 		BTRFS_I(inode)->defrag_compress = BTRFS_COMPRESS_NONE;
 		inode_unlock(inode);
 	}
-- 
2.15.1

[PATCH 4/4] Btrfs: reduce size of struct btrfs_inode

[Timofey Titovets]

Currently btrfs_inode have size equal 1136 bytes. (On x86_64).

struct btrfs_inode store several vars releated to compression code,
all states use 1 or 2 bits.

Lets declare bitfields for compression releated vars, to reduce
sizeof btrfs_inode to 1128 bytes.

Signed-off-by: Timofey Titovets <nefelim4ag@gmail.com>
---
 fs/btrfs/btrfs_inode.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
index 9eb0c92ee4b4..9d29d7e68757 100644
--- a/fs/btrfs/btrfs_inode.h
+++ b/fs/btrfs/btrfs_inode.h
@@ -181,13 +181,13 @@ struct btrfs_inode {
 	/*
 	 * Cached values of inode properties
 	 */
-	unsigned prop_compress;		/* per-file compression algorithm */
+	unsigned prop_compress : 2;	/* per-file compression algorithm */
 	/*
 	 * Force compression on the file using the defrag ioctl, could be
 	 * different from prop_compress and takes precedence if set
 	 */
-	unsigned defrag_compress;
-	unsigned change_compress;
+	unsigned defrag_compress : 2;
+	unsigned change_compress : 1;
 
 	struct btrfs_delayed_node *delayed_node;
 
-- 
2.15.1

Re: [PATCH 0/4] Btrfs: just bunch of patches to ioctl.c

[Timofey Titovets]

Gentle ping

2017-12-19 13:02 GMT+03:00 Timofey Titovets <nefelim4ag@gmail.com>:
> 1st patch, remove 16MiB restriction from extent_same ioctl(),
> by doing iterations over passed range.
>
> I did not see much difference in performance, so it's just remove
> logic restriction.
>
> 2-3 pathes, update defrag ioctl():
>  - Fix bad behaviour with full rewriting all compressed
>    extents in defrag range. (that also make autodefrag on compressed fs
>    not so expensive)
>  - Allow userspace specify NONE as target compression type,
>    that allow users to uncompress files by defragmentation with btrfs-progs
>  - Make defrag ioctl understood requested compression type and current
>    compression type of extents, to make btrfs fi def -rc<type>
>    idempotent operation.
>    i.e. now possible to say, make all extents compressed with lzo,
>    and btrfs will not recompress lzo compressed data.
>    Same for zlib, zstd, none.
>    (patch to btrfs-progs in PR on kdave GitHub).
>
> 4th patch, reduce size of struct btrfs_inode
>  - btrfs_inode store fields like: prop_compress, defrag_compress and
>    after 3rd patch, change_compress.
>    They use unsigned as a type, and use 12 bytes in sum.
>    But change_compress is a bitflag, and prop_compress/defrag_compress
>    only store compression type, that currently use 0-3 of 2^32-1.
>
>    So, set a bitfields on that vars, and reduce size of btrfs_inode:
>    1136 -> 1128.
>
> Timofey Titovets (4):
>   Btrfs: btrfs_dedupe_file_range() ioctl, remove 16MiB restriction
>   Btrfs: make should_defrag_range() understood compressed extents
>   Btrfs: allow btrfs_defrag_file() uncompress files on defragmentation
>   Btrfs: reduce size of struct btrfs_inode
>
>  fs/btrfs/btrfs_inode.h |   5 +-
>  fs/btrfs/inode.c       |   4 +-
>  fs/btrfs/ioctl.c       | 203 +++++++++++++++++++++++++++++++------------------
>  3 files changed, 133 insertions(+), 79 deletions(-)
>
> --
> 2.15.1



-- 
Have a nice day,
Timofey.

Re: [PATCH 0/4] Btrfs: just bunch of patches to ioctl.c

[Timofey Titovets]

Gentle ping

2018-01-09 13:53 GMT+03:00 Timofey Titovets <nefelim4ag@gmail.com>:
> Gentle ping
>
> 2017-12-19 13:02 GMT+03:00 Timofey Titovets <nefelim4ag@gmail.com>:
>> 1st patch, remove 16MiB restriction from extent_same ioctl(),
>> by doing iterations over passed range.
>>
>> I did not see much difference in performance, so it's just remove
>> logic restriction.
>>
>> 2-3 pathes, update defrag ioctl():
>>  - Fix bad behaviour with full rewriting all compressed
>>    extents in defrag range. (that also make autodefrag on compressed fs
>>    not so expensive)
>>  - Allow userspace specify NONE as target compression type,
>>    that allow users to uncompress files by defragmentation with btrfs-progs
>>  - Make defrag ioctl understood requested compression type and current
>>    compression type of extents, to make btrfs fi def -rc<type>
>>    idempotent operation.
>>    i.e. now possible to say, make all extents compressed with lzo,
>>    and btrfs will not recompress lzo compressed data.
>>    Same for zlib, zstd, none.
>>    (patch to btrfs-progs in PR on kdave GitHub).
>>
>> 4th patch, reduce size of struct btrfs_inode
>>  - btrfs_inode store fields like: prop_compress, defrag_compress and
>>    after 3rd patch, change_compress.
>>    They use unsigned as a type, and use 12 bytes in sum.
>>    But change_compress is a bitflag, and prop_compress/defrag_compress
>>    only store compression type, that currently use 0-3 of 2^32-1.
>>
>>    So, set a bitfields on that vars, and reduce size of btrfs_inode:
>>    1136 -> 1128.
>>
>> Timofey Titovets (4):
>>   Btrfs: btrfs_dedupe_file_range() ioctl, remove 16MiB restriction
>>   Btrfs: make should_defrag_range() understood compressed extents
>>   Btrfs: allow btrfs_defrag_file() uncompress files on defragmentation
>>   Btrfs: reduce size of struct btrfs_inode
>>
>>  fs/btrfs/btrfs_inode.h |   5 +-
>>  fs/btrfs/inode.c       |   4 +-
>>  fs/btrfs/ioctl.c       | 203 +++++++++++++++++++++++++++++++------------------
>>  3 files changed, 133 insertions(+), 79 deletions(-)
>>
>> --
>> 2.15.1
>
>
>
> --
> Have a nice day,
> Timofey.



-- 
Have a nice day,
Timofey.