[PATCH v2 0/6] Compressed path cleanups

[PATCH v2 0/6] Compressed path cleanups

[Nikolay Borisov]

Here is v2 of the compressed path cleanups. The main objective was to remove 
the BUG_ON in cow_file_range_async. This has been achieved by pre-allocating 
an array large enough to hold all async_cow structs for the given range. In 
case this allocation fail we just unlock the range and error out the pages. 


Changes since v1: 

 * Fixed error handling in patch 1 - now properly cleanup on failure. 
 * Fixed subject of patch 3 
 * Added patches 4/5 as minor cleanups to the code. 

Nikolay Borisov (6):
  btrfs: Refactor cow_file_range_async
  btrfs: Remove fs_info from struct async_cow
  btrfs: Make compress_file_range take only struct async_cow
  btrfs: Replace clear_extent_bit with unlock_extent
  btrfs: Set iotree only once in submit_compressed_extents
  btrfs: Factor out common extent locking code in
    submit_compressed_extents

 fs/btrfs/inode.c | 129 +++++++++++++++++++++++++++--------------------
 1 file changed, 73 insertions(+), 56 deletions(-)

-- 
2.17.1

[PATCH v2 1/6] btrfs: Refactor cow_file_range_async

[Nikolay Borisov]

This commit changes the implementation of cow_file_range_async in order
to get rid of the BUG_ON in the middle of the loop. Additionally it
reworks the inner loop in the hopes of making it more understandable.

Main change is that the number of chunks required to handle the given
range is calculated before going into the loop and the logic of the loop
just iterates the chunk count. Furthermore, the way memory is allocated
is reworked and now the code does a single kmalloc with enough space to
handle all chunks. Depending on whether compression is enabled or not
chunks are either 1 (in non-compress case) or the range divided by 512k.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 fs/btrfs/inode.c | 84 +++++++++++++++++++++++++++++++++---------------
 1 file changed, 58 insertions(+), 26 deletions(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 05bbfd02ea49..546000779310 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -375,6 +375,7 @@ struct async_cow {
 	unsigned int write_flags;
 	struct list_head extents;
 	struct btrfs_work work;
+	atomic_t *pending;
 };
 
 static noinline int add_async_extent(struct async_cow *cow,
@@ -1181,7 +1182,12 @@ static noinline void async_cow_free(struct btrfs_work *work)
 	async_cow = container_of(work, struct async_cow, work);
 	if (async_cow->inode)
 		btrfs_add_delayed_iput(async_cow->inode);
-	kfree(async_cow);
+	/*
+	 * Since the pointer to 'pending' is at the beginning of the array of
+	 * async_cow's, freeing it ensures the whole array has been freed.
+	 */
+	if (atomic_dec_and_test(async_cow->pending))
+		kfree(async_cow->pending);
 }
 
 static int cow_file_range_async(struct inode *inode, struct page *locked_page,
@@ -1193,42 +1199,68 @@ static int cow_file_range_async(struct inode *inode, struct page *locked_page,
 	struct async_cow *async_cow;
 	unsigned long nr_pages;
 	u64 cur_end;
+	u64 num_chunks = DIV_ROUND_UP(end - start, SZ_512K);
+	int i;
+	bool should_compress;
+	atomic_t *p;
+
 
 	clear_extent_bit(&BTRFS_I(inode)->io_tree, start, end, EXTENT_LOCKED,
 			 1, 0, NULL);
-	while (start < end) {
-		async_cow = kmalloc(sizeof(*async_cow), GFP_NOFS);
-		BUG_ON(!async_cow); /* -ENOMEM */
-		/*
-		 * igrab is called higher up in the call chain, take only the
-		 * lightweight reference for the callback lifetime
-		 */
-		ihold(inode);
-		async_cow->inode = inode;
-		async_cow->fs_info = fs_info;
-		async_cow->locked_page = locked_page;
-		async_cow->start = start;
-		async_cow->write_flags = write_flags;
-
-		if (BTRFS_I(inode)->flags & BTRFS_INODE_NOCOMPRESS &&
-		    !btrfs_test_opt(fs_info, FORCE_COMPRESS))
-			cur_end = end;
-		else
-			cur_end = min(end, start + SZ_512K - 1);
 
-		async_cow->end = cur_end;
-		INIT_LIST_HEAD(&async_cow->extents);
+	if (BTRFS_I(inode)->flags & BTRFS_INODE_NOCOMPRESS &&
+	    !btrfs_test_opt(fs_info, FORCE_COMPRESS)) {
+		num_chunks = 1;
+		should_compress = false;
+	} else {
+		should_compress = true;
+	}
+
+	/* Layout is [atomic_t][async_cow1][async_cowN].... */
+	async_cow = kmalloc(sizeof(atomic_t) + num_chunks*sizeof(*async_cow),
+			    GFP_NOFS);
+	if (!async_cow) {
+		unsigned clear_bits = EXTENT_LOCKED | EXTENT_DELALLOC |
+			EXTENT_DELALLOC_NEW | EXTENT_DEFRAG |
+			EXTENT_DO_ACCOUNTING;
+		unsigned long page_ops = PAGE_UNLOCK | PAGE_CLEAR_DIRTY |
+			PAGE_SET_WRITEBACK | PAGE_END_WRITEBACK |
+			PAGE_SET_ERROR;
+		extent_clear_unlock_delalloc(inode, start, end, 0, locked_page,
+					     clear_bits, page_ops);
+		return -ENOMEM;
+	}
+
+	p = (atomic_t *)async_cow;
+	async_cow = (struct async_cow *)((char *)async_cow + sizeof(atomic_t));
+	atomic_set(p, num_chunks);
+
+	for (i = 0; i < num_chunks; i++) {
 
-		btrfs_init_work(&async_cow->work,
+		if (should_compress)
+			cur_end = min(end, start + SZ_512K - 1);
+		else
+			cur_end = end;
+
+		ihold(inode);
+		async_cow[i].pending= p;
+		async_cow[i].inode = inode;
+		async_cow[i].start = start;
+		async_cow[i].end = cur_end;
+		async_cow[i].fs_info = fs_info;
+		async_cow[i].locked_page = locked_page;
+		async_cow[i].write_flags = write_flags;
+		INIT_LIST_HEAD(&async_cow[i].extents);
+
+		btrfs_init_work(&async_cow[i].work,
 				btrfs_delalloc_helper,
 				async_cow_start, async_cow_submit,
 				async_cow_free);
 
-		nr_pages = (cur_end - start + PAGE_SIZE) >>
-			PAGE_SHIFT;
+		nr_pages = DIV_ROUND_UP(cur_end - start, PAGE_SIZE);
 		atomic_add(nr_pages, &fs_info->async_delalloc_pages);
 
-		btrfs_queue_work(fs_info->delalloc_workers, &async_cow->work);
+		btrfs_queue_work(fs_info->delalloc_workers, &async_cow[i].work);
 
 		*nr_written += nr_pages;
 		start = cur_end + 1;
-- 
2.17.1

Re: [PATCH v2 1/6] btrfs: Refactor cow_file_range_async

[Johannes Thumshirn]

On 20/02/2019 16:11, Nikolay Borisov wrote:
> This commit changes the implementation of cow_file_range_async in order
> to get rid of the BUG_ON in the middle of the loop. Additionally it
> reworks the inner loop in the hopes of making it more understandable.
> 
> Main change is that the number of chunks required to handle the given
> range is calculated before going into the loop and the logic of the loop
> just iterates the chunk count. Furthermore, the way memory is allocated
> is reworked and now the code does a single kmalloc with enough space to
> handle all chunks. Depending on whether compression is enabled or not
> chunks are either 1 (in non-compress case) or the range divided by 512k.
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
>  fs/btrfs/inode.c | 84 +++++++++++++++++++++++++++++++++---------------
>  1 file changed, 58 insertions(+), 26 deletions(-)
> 
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 05bbfd02ea49..546000779310 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -375,6 +375,7 @@ struct async_cow {
>  	unsigned int write_flags;
>  	struct list_head extents;
>  	struct btrfs_work work;
> +	atomic_t *pending;
>  };
>  
>  static noinline int add_async_extent(struct async_cow *cow,
> @@ -1181,7 +1182,12 @@ static noinline void async_cow_free(struct btrfs_work *work)
>  	async_cow = container_of(work, struct async_cow, work);
>  	if (async_cow->inode)
>  		btrfs_add_delayed_iput(async_cow->inode);
> -	kfree(async_cow);
> +	/*
> +	 * Since the pointer to 'pending' is at the beginning of the array of
> +	 * async_cow's, freeing it ensures the whole array has been freed.
> +	 */
> +	if (atomic_dec_and_test(async_cow->pending))
> +		kfree(async_cow->pending);
>  }

[...]

> +	/* Layout is [atomic_t][async_cow1][async_cowN].... */
> +	async_cow = kmalloc(sizeof(atomic_t) + num_chunks*sizeof(*async_cow),
> +			    GFP_NOFS);
> +	if (!async_cow) {
> +		unsigned clear_bits = EXTENT_LOCKED | EXTENT_DELALLOC |
> +			EXTENT_DELALLOC_NEW | EXTENT_DEFRAG |
> +			EXTENT_DO_ACCOUNTING;
> +		unsigned long page_ops = PAGE_UNLOCK | PAGE_CLEAR_DIRTY |
> +			PAGE_SET_WRITEBACK | PAGE_END_WRITEBACK |
> +			PAGE_SET_ERROR;
> +		extent_clear_unlock_delalloc(inode, start, end, 0, locked_page,
> +					     clear_bits, page_ops);
> +		return -ENOMEM;
> +	}
> +
> +	p = (atomic_t *)async_cow;
> +	async_cow = (struct async_cow *)((char *)async_cow + sizeof(atomic_t));
> +	atomic_set(p, num_chunks);

Ugh! You're abusing an atomic_t * for something you want to actually
make this:

@@ -375,6 +375,7 @@ struct async_cow {
 	unsigned int write_flags;
 	struct list_head extents;
 	struct btrfs_work work;
+	atomic_t pending;
+	struct async_cow list[0];
};

and then have:

tmp = krealloc(async_cow, num_chunks * sizeof(*async_cow), GFP_NOFS);
if (!tmp) {
[...]
}
kfree(async_cow);
async_cow = tmp;


-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

[PATCH v2 2/6] btrfs: Remove fs_info from struct async_cow

[Nikolay Borisov]

We always pass an inode alongside async_cow. fs_info can be referenced
from this inode so it makes the explicit fs_info member in
struct async_cow redundant, remove it. No functional changes.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 fs/btrfs/inode.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 546000779310..c6d4e0f67d14 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -368,7 +368,6 @@ struct async_extent {
 
 struct async_cow {
 	struct inode *inode;
-	struct btrfs_fs_info *fs_info;
 	struct page *locked_page;
 	u64 start;
 	u64 end;
@@ -1151,13 +1150,10 @@ static noinline void async_cow_start(struct btrfs_work *work)
  */
 static noinline void async_cow_submit(struct btrfs_work *work)
 {
-	struct btrfs_fs_info *fs_info;
-	struct async_cow *async_cow;
+	struct async_cow *async_cow = container_of(work, struct async_cow, work);
+	struct btrfs_fs_info *fs_info = btrfs_work_owner(work);
 	unsigned long nr_pages;
 
-	async_cow = container_of(work, struct async_cow, work);
-
-	fs_info = async_cow->fs_info;
 	nr_pages = (async_cow->end - async_cow->start + PAGE_SIZE) >>
 		PAGE_SHIFT;
 
@@ -1247,7 +1243,6 @@ static int cow_file_range_async(struct inode *inode, struct page *locked_page,
 		async_cow[i].inode = inode;
 		async_cow[i].start = start;
 		async_cow[i].end = cur_end;
-		async_cow[i].fs_info = fs_info;
 		async_cow[i].locked_page = locked_page;
 		async_cow[i].write_flags = write_flags;
 		INIT_LIST_HEAD(&async_cow[i].extents);
-- 
2.17.1

Re: [PATCH v2 2/6] btrfs: Remove fs_info from struct async_cow

[Johannes Thumshirn]

On 20/02/2019 16:11, Nikolay Borisov wrote:
> We always pass an inode alongside async_cow. fs_info can be referenced
> from this inode so it makes the explicit fs_info member in
> struct async_cow redundant, remove it. No functional changes.

[...]

> @@ -1151,13 +1150,10 @@ static noinline void async_cow_start(struct btrfs_work *work)
>   */
>  static noinline void async_cow_submit(struct btrfs_work *work)
>  {
> -	struct btrfs_fs_info *fs_info;
> -	struct async_cow *async_cow;
> +	struct async_cow *async_cow = container_of(work, struct async_cow, work);
> +	struct btrfs_fs_info *fs_info = btrfs_work_owner(work);
>  	unsigned long nr_pages;

Nit, in your changelog you write that fs_info can be retrieved from the
inode that is passed with 'struct async_cow' but you're getting fs_info
from the btrfs_work structure.

-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH v2 2/6] btrfs: Remove fs_info from struct async_cow

[Nikolay Borisov]

On 20.02.19 г. 17:25 ч., Johannes Thumshirn wrote:
> On 20/02/2019 16:11, Nikolay Borisov wrote:
>> We always pass an inode alongside async_cow. fs_info can be referenced
>> from this inode so it makes the explicit fs_info member in
>> struct async_cow redundant, remove it. No functional changes.
> 
> [...]
> 
>> @@ -1151,13 +1150,10 @@ static noinline void async_cow_start(struct btrfs_work *work)
>>   */
>>  static noinline void async_cow_submit(struct btrfs_work *work)
>>  {
>> -	struct btrfs_fs_info *fs_info;
>> -	struct async_cow *async_cow;
>> +	struct async_cow *async_cow = container_of(work, struct async_cow, work);
>> +	struct btrfs_fs_info *fs_info = btrfs_work_owner(work);
>>  	unsigned long nr_pages;
> 
> Nit, in your changelog you write that fs_info can be retrieved from the
> inode that is passed with 'struct async_cow' but you're getting fs_info
> from the btrfs_work structure.

Ah blimey, indeed, this was because the inode could indeed be null so we
can actually take it from the btrfs_work struct which is always
guaranteed to have it initialized. I'll wait for more review comments to
accumulate before resend

> 

[PATCH v2 3/6] btrfs: Make compress_file_range take only struct async_cow

[Nikolay Borisov]

All context this function needs is held within struct async_cow.
Currently we not only pass the struct but also every individual member.
This is redundant, simplify it by only passing struct async_cow and
leaving it to compress_file_range to extract the values it requires.
No functional changes.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 fs/btrfs/inode.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index c6d4e0f67d14..02666e584e28 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -444,14 +444,14 @@ static inline void inode_should_defrag(struct btrfs_inode *inode,
  * are written in the same order that the flusher thread sent them
  * down.
  */
-static noinline void compress_file_range(struct inode *inode,
-					struct page *locked_page,
-					u64 start, u64 end,
-					struct async_cow *async_cow,
-					int *num_added)
+static noinline void compress_file_range(struct async_cow *async_cow,
+					 int *num_added)
 {
+	struct inode *inode = async_cow->inode;
 	struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
 	u64 blocksize = fs_info->sectorsize;
+	u64 start = async_cow->start;
+	u64 end = async_cow->end;
 	u64 actual_end;
 	int ret = 0;
 	struct page **pages = NULL;
@@ -670,9 +670,9 @@ static noinline void compress_file_range(struct inode *inode,
 	 * to our extent and set things up for the async work queue to run
 	 * cow_file_range to do the normal delalloc dance.
 	 */
-	if (page_offset(locked_page) >= start &&
-	    page_offset(locked_page) <= end)
-		__set_page_dirty_nobuffers(locked_page);
+	if (page_offset(async_cow->locked_page) >= start &&
+	    page_offset(async_cow->locked_page) <= end)
+		__set_page_dirty_nobuffers(async_cow->locked_page);
 		/* unlocked later on in the async handlers */
 
 	if (redirty)
@@ -1136,9 +1136,7 @@ static noinline void async_cow_start(struct btrfs_work *work)
 	int num_added = 0;
 	async_cow = container_of(work, struct async_cow, work);
 
-	compress_file_range(async_cow->inode, async_cow->locked_page,
-			    async_cow->start, async_cow->end, async_cow,
-			    &num_added);
+	compress_file_range(async_cow, &num_added);
 	if (num_added == 0) {
 		btrfs_add_delayed_iput(async_cow->inode);
 		async_cow->inode = NULL;
-- 
2.17.1

Re: [PATCH v2 3/6] btrfs: Make compress_file_range take only struct async_cow

[Johannes Thumshirn]

Hmm "... and num_added" for the Subject?

-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH v2 3/6] btrfs: Make compress_file_range take only struct async_cow

[Nikolay Borisov]

On 20.02.19 г. 17:27 ч., Johannes Thumshirn wrote:
> Hmm "... and num_added" for the Subject?

Nah, that's obvious, the focus here is instead of taking 4-5 more
arguments they are replaced with async_cow.

Re: [PATCH v2 3/6] btrfs: Make compress_file_range take only struct async_cow

[Johannes Thumshirn]

On 21/02/2019 12:55, Nikolay Borisov wrote:
> 
> 
> On 20.02.19 г. 17:27 ч., Johannes Thumshirn wrote:
>> Hmm "... and num_added" for the Subject?
> 
> Nah, that's obvious, the focus here is instead of taking 4-5 more
> arguments they are replaced with async_cow.
> 

Fair enough

-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

[PATCH v2 4/6] btrfs: Replace clear_extent_bit with unlock_extent

[Nikolay Borisov]

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 fs/btrfs/inode.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 02666e584e28..c6ee1863cd20 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -1199,8 +1199,7 @@ static int cow_file_range_async(struct inode *inode, struct page *locked_page,
 	atomic_t *p;
 
 
-	clear_extent_bit(&BTRFS_I(inode)->io_tree, start, end, EXTENT_LOCKED,
-			 1, 0, NULL);
+	unlock_extent(&BTRFS_I(inode)->io_tree, start, end);
 
 	if (BTRFS_I(inode)->flags & BTRFS_INODE_NOCOMPRESS &&
 	    !btrfs_test_opt(fs_info, FORCE_COMPRESS)) {
-- 
2.17.1

Re: [PATCH v2 4/6] btrfs: Replace clear_extent_bit with unlock_extent

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

[PATCH v2 5/6] btrfs: Set iotree only once in submit_compressed_extents

[Nikolay Borisov]

The inode never changes so it's sufficient to dereference it and get
the iotree only once, before the execution of the main loop. No
functional changes, only the size of the function is decreased:

add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-44 (-44)
Function                                     old     new   delta
submit_compressed_extents                   1240    1196     -44
Total: Before=88476, After=88432, chg -0.05%

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 fs/btrfs/inode.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index c6ee1863cd20..06408a278200 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -722,7 +722,7 @@ static noinline void submit_compressed_extents(struct async_cow *async_cow)
 	struct btrfs_key ins;
 	struct extent_map *em;
 	struct btrfs_root *root = BTRFS_I(inode)->root;
-	struct extent_io_tree *io_tree;
+	struct extent_io_tree *io_tree = &BTRFS_I(inode)->io_tree;
 	int ret = 0;
 
 again:
@@ -730,9 +730,6 @@ static noinline void submit_compressed_extents(struct async_cow *async_cow)
 		async_extent = list_entry(async_cow->extents.next,
 					  struct async_extent, list);
 		list_del(&async_extent->list);
-
-		io_tree = &BTRFS_I(inode)->io_tree;
-
 retry:
 		/* did the compression code fall back to uncompressed IO? */
 		if (!async_extent->pages) {
-- 
2.17.1

Re: [PATCH v2 5/6] btrfs: Set iotree only once in submit_compressed_extents

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

[PATCH v2 6/6] btrfs: Factor out common extent locking code in submit_compressed_extents

[Nikolay Borisov]

Irrespective of whether the compress code fell back to uncompressed or
a compressed extent has to be submitted, the extent range is always
locked. So factor out the common lock_extent call at the beginning of
the loop. No functional changes just removes one duplicate lock_extent
call.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 fs/btrfs/inode.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 06408a278200..02b393328a7c 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -731,15 +731,14 @@ static noinline void submit_compressed_extents(struct async_cow *async_cow)
 					  struct async_extent, list);
 		list_del(&async_extent->list);
 retry:
+
+		lock_extent(io_tree, async_extent->start,
+			    async_extent->start + async_extent->ram_size - 1);
 		/* did the compression code fall back to uncompressed IO? */
 		if (!async_extent->pages) {
 			int page_started = 0;
 			unsigned long nr_written = 0;
 
-			lock_extent(io_tree, async_extent->start,
-					 async_extent->start +
-					 async_extent->ram_size - 1);
-
 			/* allocate blocks */
 			ret = cow_file_range(inode, async_cow->locked_page,
 					     async_extent->start,
@@ -771,9 +770,6 @@ static noinline void submit_compressed_extents(struct async_cow *async_cow)
 			continue;
 		}
 
-		lock_extent(io_tree, async_extent->start,
-			    async_extent->start + async_extent->ram_size - 1);
-
 		ret = btrfs_reserve_extent(root, async_extent->ram_size,
 					   async_extent->compressed_size,
 					   async_extent->compressed_size,
-- 
2.17.1

Re: [PATCH v2 6/6] btrfs: Factor out common extent locking code in submit_compressed_extents

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850