[PATCH 0/3] Misc fixes mostly cleanup

[PATCH 0/3] Misc fixes mostly cleanup

[Anand Jain]

A set of unrelated miscellaneous cleanup patches.

Anand Jain (3):
  btrfs: use BTRFS_COMPRESS_NONE to specify no compression
  btrfs: cow_file_range() num_bytes and disk_num_bytes are same
  btrfs: consolidate auto defrag kick off policies

 fs/btrfs/inode.c | 44 ++++++++++++++++++++++++++------------------
 1 file changed, 26 insertions(+), 18 deletions(-)

-- 
2.10.0

[PATCH 1/3] btrfs: use BTRFS_COMPRESS_NONE to specify no compression

[Anand Jain]

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/inode.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 8e3a5a266917..96e5f8a49d4c 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -540,7 +540,7 @@ static noinline void compress_file_range(struct inode *inode,
 			 * to make an uncompressed inline extent.
 			 */
 			ret = cow_file_range_inline(root, inode, start, end,
-						    0, 0, NULL);
+					    0, BTRFS_COMPRESS_NONE, NULL);
 		} else {
 			/* try making a compressed inline extent */
 			ret = cow_file_range_inline(root, inode, start, end,
@@ -969,8 +969,8 @@ static noinline int cow_file_range(struct inode *inode,
 
 	if (start == 0) {
 		/* lets try to make an inline extent */
-		ret = cow_file_range_inline(root, inode, start, end, 0, 0,
-					    NULL);
+		ret = cow_file_range_inline(root, inode, start, end, 0,
+					BTRFS_COMPRESS_NONE, NULL);
 		if (ret == 0) {
 			extent_clear_unlock_delalloc(inode, start, end,
 				     delalloc_end, NULL,
-- 
2.10.0

[PATCH 2/3] btrfs: cow_file_range() num_bytes and disk_num_bytes are same

[Anand Jain]

This patch deletes local variable disk_num_bytes as its value
is same as num_bytes in the function cow_file_range().

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/inode.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 96e5f8a49d4c..79f073e94f2d 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -944,7 +944,6 @@ static noinline int cow_file_range(struct inode *inode,
 	u64 alloc_hint = 0;
 	u64 num_bytes;
 	unsigned long ram_size;
-	u64 disk_num_bytes;
 	u64 cur_alloc_size;
 	u64 blocksize = root->sectorsize;
 	struct btrfs_key ins;
@@ -960,7 +959,6 @@ static noinline int cow_file_range(struct inode *inode,
 
 	num_bytes = ALIGN(end - start + 1, blocksize);
 	num_bytes = max(blocksize,  num_bytes);
-	disk_num_bytes = num_bytes;
 
 	/* if this is a small write inside eof, kick off defrag */
 	if (num_bytes < SZ_64K &&
@@ -989,16 +987,16 @@ static noinline int cow_file_range(struct inode *inode,
 		}
 	}
 
-	BUG_ON(disk_num_bytes >
+	BUG_ON(num_bytes >
 	       btrfs_super_total_bytes(root->fs_info->super_copy));
 
 	alloc_hint = get_extent_allocation_hint(inode, start, num_bytes);
 	btrfs_drop_extent_cache(inode, start, start + num_bytes - 1, 0);
 
-	while (disk_num_bytes > 0) {
+	while (num_bytes > 0) {
 		unsigned long op;
 
-		cur_alloc_size = disk_num_bytes;
+		cur_alloc_size = num_bytes;
 		ret = btrfs_reserve_extent(root, cur_alloc_size, cur_alloc_size,
 					   root->sectorsize, 0, alloc_hint,
 					   &ins, 1, 1);
@@ -1055,7 +1053,7 @@ static noinline int cow_file_range(struct inode *inode,
 
 		btrfs_dec_block_group_reservations(root->fs_info, ins.objectid);
 
-		if (disk_num_bytes < cur_alloc_size)
+		if (num_bytes < cur_alloc_size)
 			break;
 
 		/* we're not doing compressed IO, don't unlock the first
@@ -1073,7 +1071,6 @@ static noinline int cow_file_range(struct inode *inode,
 					     delalloc_end, locked_page,
 					     EXTENT_LOCKED | EXTENT_DELALLOC,
 					     op);
-		disk_num_bytes -= cur_alloc_size;
 		num_bytes -= cur_alloc_size;
 		alloc_hint = ins.objectid + ins.offset;
 		start += cur_alloc_size;
-- 
2.10.0

Re: [PATCH 2/3] btrfs: cow_file_range() num_bytes and disk_num_bytes are same

[David Sterba]

On Tue, Dec 06, 2016 at 12:43:08PM +0800, Anand Jain wrote:
> This patch deletes local variable disk_num_bytes as its value
> is same as num_bytes in the function cow_file_range().
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>

Reviewed-by: David Sterba <dsterba@suse.com>

[PATCH 3/3] btrfs: consolidate auto defrag kick off policies

[Anand Jain]

As of now writes smaller than 64k for non compressed extents and 16k
for compressed extents inside eof are considered as candidate
for auto defrag, put them together at a place.
---
 fs/btrfs/inode.c | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 79f073e94f2d..b157575166c6 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -388,6 +388,22 @@ static inline int inode_need_compress(struct inode *inode)
 	return 0;
 }
 
+static inline void inode_should_defrag(struct inode *inode,
+		u64 start, u64 end, u64 num_bytes, int comp_type)
+{
+	u64 small_write = SZ_64K;
+	if (comp_type)
+		small_write = SZ_16K;
+
+	if (!num_bytes)
+		num_bytes = end - start + 1;
+
+	/* If this is a small write inside eof, kick off a defrag */
+	if (num_bytes < small_write &&
+	    (start > 0 || end + 1 < BTRFS_I(inode)->disk_i_size))
+		btrfs_add_inode_defrag(NULL, inode);
+}
+
 /*
  * we create compressed extents in two phases.  The first
  * phase compresses a range of pages that have already been
@@ -429,10 +445,7 @@ static noinline void compress_file_range(struct inode *inode,
 	int compress_type = root->fs_info->compress_type;
 	int redirty = 0;
 
-	/* if this is a small write inside eof, kick off a defrag */
-	if ((end - start + 1) < SZ_16K &&
-	    (start > 0 || end + 1 < BTRFS_I(inode)->disk_i_size))
-		btrfs_add_inode_defrag(NULL, inode);
+	inode_should_defrag(inode, start, end, 0, compress_type);
 
 	actual_end = min_t(u64, isize, end + 1);
 again:
@@ -960,10 +973,8 @@ static noinline int cow_file_range(struct inode *inode,
 	num_bytes = ALIGN(end - start + 1, blocksize);
 	num_bytes = max(blocksize,  num_bytes);
 
-	/* if this is a small write inside eof, kick off defrag */
-	if (num_bytes < SZ_64K &&
-	    (start > 0 || end + 1 < BTRFS_I(inode)->disk_i_size))
-		btrfs_add_inode_defrag(NULL, inode);
+	inode_should_defrag(inode, start, end, num_bytes,
+						BTRFS_COMPRESS_NONE);
 
 	if (start == 0) {
 		/* lets try to make an inline extent */
-- 
2.10.0

Re: [PATCH 3/3] btrfs: consolidate auto defrag kick off policies

[David Sterba]

On Tue, Dec 06, 2016 at 12:43:09PM +0800, Anand Jain wrote:
> As of now writes smaller than 64k for non compressed extents and 16k
> for compressed extents inside eof are considered as candidate
> for auto defrag, put them together at a place.

Missing sign-off.

> ---
>  fs/btrfs/inode.c | 27 +++++++++++++++++++--------
>  1 file changed, 19 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 79f073e94f2d..b157575166c6 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -388,6 +388,22 @@ static inline int inode_need_compress(struct inode *inode)
>  	return 0;
>  }
>  
> +static inline void inode_should_defrag(struct inode *inode,
> +		u64 start, u64 end, u64 num_bytes, int comp_type)
> +{
> +	u64 small_write = SZ_64K;
> +	if (comp_type)
> +		small_write = SZ_16K;

I think the small_write value should be passed directly, not the
compression type.

> +
> +	if (!num_bytes)
> +		num_bytes = end - start + 1;

And the callers should pass the range length. Calculating inside the
function makes it less clear. One of the callers passes an blocksize
aligned value and the other doest not, so both know what's the right
value.

Otherwise the cleanup is good. I'll merge the other two patches, please
update and resend this one. Thanks.

Re: [PATCH 3/3] btrfs: consolidate auto defrag kick off policies

[Anand Jain]

(sorry for the delay due to my vacation).

On 12/12/16 22:12, David Sterba wrote:
> On Tue, Dec 06, 2016 at 12:43:09PM +0800, Anand Jain wrote:
>> As of now writes smaller than 64k for non compressed extents and 16k
>> for compressed extents inside eof are considered as candidate
>> for auto defrag, put them together at a place.
>
> Missing sign-off.
sorry will fix.

>> ---
>>  fs/btrfs/inode.c | 27 +++++++++++++++++++--------
>>  1 file changed, 19 insertions(+), 8 deletions(-)
>>
>> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
>> index 79f073e94f2d..b157575166c6 100644
>> --- a/fs/btrfs/inode.c
>> +++ b/fs/btrfs/inode.c
>> @@ -388,6 +388,22 @@ static inline int inode_need_compress(struct inode *inode)
>>  	return 0;
>>  }
>>
>> +static inline void inode_should_defrag(struct inode *inode,
>> +		u64 start, u64 end, u64 num_bytes, int comp_type)
>> +{
>> +	u64 small_write = SZ_64K;
>> +	if (comp_type)
>> +		small_write = SZ_16K;
>
> I think the small_write value should be passed directly, not the
> compression type.

  Will do.

>> +
>> +	if (!num_bytes)
>> +		num_bytes = end - start + 1;
>
> And the callers should pass the range length. Calculating inside the
> function makes it less clear. One of the callers passes an blocksize
> aligned value and the other doest not, so both know what's the right
> value.

  agreed. I have sent out v2. Kindly find it.

Thanks, Anand



> Otherwise the cleanup is good. I'll merge the other two patches, please
> update and resend this one. Thanks.
> --
> 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
>

[PATCH 3/3 v2] btrfs: consolidate auto defrag kick off policies

[Anand Jain]

As of now writes smaller than 64k for non compressed extents and 16k
for compressed extents inside eof are considered as candidate
for auto defrag, put them together at a place.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
v2: pass value of small write, and current num_bytes.
    fix sign-off.

 fs/btrfs/inode.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 79f073e94f2d..d09c8276fef5 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -388,6 +388,15 @@ static inline int inode_need_compress(struct inode *inode)
 	return 0;
 }
 
+static inline void inode_should_defrag(struct inode *inode,
+		u64 start, u64 end, u64 num_bytes, u64 small_write)
+{
+	/* If this is a small write inside eof, kick off a defrag */
+	if (num_bytes < small_write &&
+	    (start > 0 || end + 1 < BTRFS_I(inode)->disk_i_size))
+		btrfs_add_inode_defrag(NULL, inode);
+}
+
 /*
  * we create compressed extents in two phases.  The first
  * phase compresses a range of pages that have already been
@@ -429,10 +438,7 @@ static noinline void compress_file_range(struct inode *inode,
 	int compress_type = root->fs_info->compress_type;
 	int redirty = 0;
 
-	/* if this is a small write inside eof, kick off a defrag */
-	if ((end - start + 1) < SZ_16K &&
-	    (start > 0 || end + 1 < BTRFS_I(inode)->disk_i_size))
-		btrfs_add_inode_defrag(NULL, inode);
+	inode_should_defrag(inode, start, end, end - start + 1, SZ_16K);
 
 	actual_end = min_t(u64, isize, end + 1);
 again:
@@ -960,10 +966,7 @@ static noinline int cow_file_range(struct inode *inode,
 	num_bytes = ALIGN(end - start + 1, blocksize);
 	num_bytes = max(blocksize,  num_bytes);
 
-	/* if this is a small write inside eof, kick off defrag */
-	if (num_bytes < SZ_64K &&
-	    (start > 0 || end + 1 < BTRFS_I(inode)->disk_i_size))
-		btrfs_add_inode_defrag(NULL, inode);
+	inode_should_defrag(inode, start, end, num_bytes, SZ_64K);
 
 	if (start == 0) {
 		/* lets try to make an inline extent */
-- 
2.10.0

Re: [PATCH 3/3 v2] btrfs: consolidate auto defrag kick off policies

[David Sterba]

On Mon, Dec 19, 2016 at 07:09:06PM +0800, Anand Jain wrote:
> As of now writes smaller than 64k for non compressed extents and 16k
> for compressed extents inside eof are considered as candidate
> for auto defrag, put them together at a place.
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>

Reviewed-by: David Sterba <dsterba@suse.com>