[PATCH] Btrfs: set leave_spinning in btrfs_get_extent

[PATCH] Btrfs: set leave_spinning in btrfs_get_extent

[Liu Bo]

Unless it's going to read inline extents from btree leaf to page,
btrfs_get_extent won't sleep during the period of holding path lock.

This sets leave_spinning at first and sets path to blocking mode right
before reading inline extent if that's the case.  The benefit is that a
path in spinning mode typically has less impact (faster) on waiters
rather than that in blocking mode.

Signed-off-by: Liu Bo <bo.liu@linux.alibaba.com>
---
 fs/btrfs/inode.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 10e86f7b5398..4219be9c21f9 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -6843,6 +6843,12 @@ struct extent_map *btrfs_get_extent(struct btrfs_inode *inode,
 	 */
 	path->reada = READA_FORWARD;
 
+	/*
+	 * Unless we're going to uncompress inline extent, no sleep would
+	 * happen.
+	 */
+	path->leave_spinning = 1;
+
 	ret = btrfs_lookup_file_extent(NULL, root, path, objectid, start, 0);
 	if (ret < 0) {
 		err = ret;
@@ -6945,6 +6951,8 @@ struct extent_map *btrfs_get_extent(struct btrfs_inode *inode,
 		em->orig_block_len = em->len;
 		em->orig_start = em->start;
 		ptr = btrfs_file_extent_inline_start(item) + extent_offset;
+
+		btrfs_set_path_blocking(path);
 		if (!PageUptodate(page)) {
 			if (btrfs_file_extent_compression(leaf, item) !=
 			    BTRFS_COMPRESS_NONE) {
@@ -6992,10 +7000,10 @@ struct extent_map *btrfs_get_extent(struct btrfs_inode *inode,
 	err = btrfs_add_extent_mapping(fs_info, em_tree, &em, start, len);
 	write_unlock(&em_tree->lock);
 out:
+	btrfs_free_path(path);
 
 	trace_btrfs_get_extent(root, inode, em);
 
-	btrfs_free_path(path);
 	if (err) {
 		free_extent_map(em);
 		return ERR_PTR(err);
-- 
1.8.3.1

[PATCH v2] Btrfs: set leave_spinning in btrfs_get_extent

[Liu Bo]

Unless it's going to read inline extents from btree leaf to page,
btrfs_get_extent won't sleep during the period of holding path lock.

This sets leave_spinning at first and sets path to blocking mode right
before reading inline extent if that's the case.  The benefit is that a
path in spinning mode typically has less impact (faster) on waiters
rather than that in blocking mode.

Also fixes the misalignment of the prototype, which is too trivial for
a single patch.

Signed-off-by: Liu Bo <bo.liu@linux.alibaba.com>
---
v2: send out the correct patch.

 fs/btrfs/ctree.h |  4 ++--
 fs/btrfs/inode.c | 14 +++++++++++---
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 1aeed3c0e949..00e506de70ba 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -3177,8 +3177,8 @@ int btrfs_merge_bio_hook(struct page *page, unsigned long offset,
 struct inode *btrfs_iget(struct super_block *s, struct btrfs_key *location,
 			 struct btrfs_root *root, int *was_new);
 struct extent_map *btrfs_get_extent(struct btrfs_inode *inode,
-		struct page *page, size_t pg_offset,
-		u64 start, u64 end, int create);
+				    struct page *page, size_t pg_offset,
+				    u64 start, u64 end, int create);
 int btrfs_update_inode(struct btrfs_trans_handle *trans,
 			      struct btrfs_root *root,
 			      struct inode *inode);
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 10e86f7b5398..31d43355f052 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -6787,9 +6787,9 @@ static noinline int uncompress_inline(struct btrfs_path *path,
  * This also copies inline extents directly into the page.
  */
 struct extent_map *btrfs_get_extent(struct btrfs_inode *inode,
-		struct page *page,
-	    size_t pg_offset, u64 start, u64 len,
-		int create)
+				    struct page *page,
+				    size_t pg_offset, u64 start, u64 len,
+				    int create)
 {
 	struct btrfs_fs_info *fs_info = inode->root->fs_info;
 	int ret;
@@ -6843,6 +6843,12 @@ struct extent_map *btrfs_get_extent(struct btrfs_inode *inode,
 	 */
 	path->reada = READA_FORWARD;
 
+	/*
+	 * Unless we're going to uncompress inline extent, no sleep would
+	 * happen.
+	 */
+	path->leave_spinning = 1;
+
 	ret = btrfs_lookup_file_extent(NULL, root, path, objectid, start, 0);
 	if (ret < 0) {
 		err = ret;
@@ -6945,6 +6951,8 @@ struct extent_map *btrfs_get_extent(struct btrfs_inode *inode,
 		em->orig_block_len = em->len;
 		em->orig_start = em->start;
 		ptr = btrfs_file_extent_inline_start(item) + extent_offset;
+
+		btrfs_set_path_blocking(path);
 		if (!PageUptodate(page)) {
 			if (btrfs_file_extent_compression(leaf, item) !=
 			    BTRFS_COMPRESS_NONE) {
-- 
1.8.3.1

Re: [PATCH v2] Btrfs: set leave_spinning in btrfs_get_extent

[David Sterba]

On Thu, Aug 23, 2018 at 07:41:05AM +0800, Liu Bo wrote:
> Unless it's going to read inline extents from btree leaf to page,
> btrfs_get_extent won't sleep during the period of holding path lock.
> 
> This sets leave_spinning at first and sets path to blocking mode right
> before reading inline extent if that's the case.  The benefit is that a
> path in spinning mode typically has less impact (faster) on waiters
> rather than that in blocking mode.
> 
> Also fixes the misalignment of the prototype, which is too trivial for
> a single patch.
> 
> Signed-off-by: Liu Bo <bo.liu@linux.alibaba.com>
> ---
> v2: send out the correct patch.
> 
>  fs/btrfs/ctree.h |  4 ++--
>  fs/btrfs/inode.c | 14 +++++++++++---
>  2 files changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index 1aeed3c0e949..00e506de70ba 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -3177,8 +3177,8 @@ int btrfs_merge_bio_hook(struct page *page, unsigned long offset,
>  struct inode *btrfs_iget(struct super_block *s, struct btrfs_key *location,
>  			 struct btrfs_root *root, int *was_new);
>  struct extent_map *btrfs_get_extent(struct btrfs_inode *inode,
> -		struct page *page, size_t pg_offset,
> -		u64 start, u64 end, int create);
> +				    struct page *page, size_t pg_offset,
> +				    u64 start, u64 end, int create);

Please don't add unrelated changes.

>  int btrfs_update_inode(struct btrfs_trans_handle *trans,
>  			      struct btrfs_root *root,
>  			      struct inode *inode);
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 10e86f7b5398..31d43355f052 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -6787,9 +6787,9 @@ static noinline int uncompress_inline(struct btrfs_path *path,
>   * This also copies inline extents directly into the page.
>   */
>  struct extent_map *btrfs_get_extent(struct btrfs_inode *inode,
> -		struct page *page,
> -	    size_t pg_offset, u64 start, u64 len,
> -		int create)
> +				    struct page *page,
> +				    size_t pg_offset, u64 start, u64 len,
> +				    int create)

Ok, this looks ugly and the reformatting makes sense, both declaration
and prototype, but please send it as a separate patch.

Re: [PATCH v2] Btrfs: set leave_spinning in btrfs_get_extent

[Liu Bo]

On Fri, Aug 24, 2018 at 04:59:49PM +0200, David Sterba wrote:
> On Thu, Aug 23, 2018 at 07:41:05AM +0800, Liu Bo wrote:
> > Unless it's going to read inline extents from btree leaf to page,
> > btrfs_get_extent won't sleep during the period of holding path lock.
> > 
> > This sets leave_spinning at first and sets path to blocking mode right
> > before reading inline extent if that's the case.  The benefit is that a
> > path in spinning mode typically has less impact (faster) on waiters
> > rather than that in blocking mode.
> > 
> > Also fixes the misalignment of the prototype, which is too trivial for
> > a single patch.
> > 
> > Signed-off-by: Liu Bo <bo.liu@linux.alibaba.com>
> > ---
> > v2: send out the correct patch.
> > 
> >  fs/btrfs/ctree.h |  4 ++--
> >  fs/btrfs/inode.c | 14 +++++++++++---
> >  2 files changed, 13 insertions(+), 5 deletions(-)
> > 
> > diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> > index 1aeed3c0e949..00e506de70ba 100644
> > --- a/fs/btrfs/ctree.h
> > +++ b/fs/btrfs/ctree.h
> > @@ -3177,8 +3177,8 @@ int btrfs_merge_bio_hook(struct page *page, unsigned long offset,
> >  struct inode *btrfs_iget(struct super_block *s, struct btrfs_key *location,
> >  			 struct btrfs_root *root, int *was_new);
> >  struct extent_map *btrfs_get_extent(struct btrfs_inode *inode,
> > -		struct page *page, size_t pg_offset,
> > -		u64 start, u64 end, int create);
> > +				    struct page *page, size_t pg_offset,
> > +				    u64 start, u64 end, int create);
> 
> Please don't add unrelated changes.
> 
> >  int btrfs_update_inode(struct btrfs_trans_handle *trans,
> >  			      struct btrfs_root *root,
> >  			      struct inode *inode);
> > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> > index 10e86f7b5398..31d43355f052 100644
> > --- a/fs/btrfs/inode.c
> > +++ b/fs/btrfs/inode.c
> > @@ -6787,9 +6787,9 @@ static noinline int uncompress_inline(struct btrfs_path *path,
> >   * This also copies inline extents directly into the page.
> >   */
> >  struct extent_map *btrfs_get_extent(struct btrfs_inode *inode,
> > -		struct page *page,
> > -	    size_t pg_offset, u64 start, u64 len,
> > -		int create)
> > +				    struct page *page,
> > +				    size_t pg_offset, u64 start, u64 len,
> > +				    int create)
> 
> Ok, this looks ugly and the reformatting makes sense, both declaration
> and prototype, but please send it as a separate patch.

Sure.

Note that there're still some format issues introduced by the old
patch set of "inode -> btrfs_inode".

thanks,
-liubo

[PATCH v3] Btrfs: set leave_spinning in btrfs_get_extent

[Liu Bo]

Unless it's going to read inline extents from btree leaf to page,
btrfs_get_extent won't sleep during the period of holding path lock.

This sets leave_spinning at first and sets path to blocking mode right
before reading inline extent if that's the case.  The benefit is that a
path in spinning mode typically has less impact (faster) on waiters
rather than that in blocking mode.

Also fixes the misalignment of the prototype, which is too trivial for
a single patch.


Signed-off-by: Liu Bo <bo.liu@linux.alibaba.com>
---
v3: separate alignment fix out of this patch.

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

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 10e86f7b5398..1e0af0cc56fc 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -6843,6 +6843,12 @@ struct extent_map *btrfs_get_extent(struct btrfs_inode *inode,
 	 */
 	path->reada = READA_FORWARD;
 
+	/*
+	 * Unless we're going to uncompress inline extent, no sleep would
+	 * happen.
+	 */
+	path->leave_spinning = 1;
+
 	ret = btrfs_lookup_file_extent(NULL, root, path, objectid, start, 0);
 	if (ret < 0) {
 		err = ret;
@@ -6945,6 +6951,8 @@ struct extent_map *btrfs_get_extent(struct btrfs_inode *inode,
 		em->orig_block_len = em->len;
 		em->orig_start = em->start;
 		ptr = btrfs_file_extent_inline_start(item) + extent_offset;
+
+		btrfs_set_path_blocking(path);
 		if (!PageUptodate(page)) {
 			if (btrfs_file_extent_compression(leaf, item) !=
 			    BTRFS_COMPRESS_NONE) {
-- 
1.8.3.1

Re: [PATCH v3] Btrfs: set leave_spinning in btrfs_get_extent

[David Sterba]

On Sat, Aug 25, 2018 at 01:47:09PM +0800, Liu Bo wrote:
> Unless it's going to read inline extents from btree leaf to page,
> btrfs_get_extent won't sleep during the period of holding path lock.
> 
> This sets leave_spinning at first and sets path to blocking mode right
> before reading inline extent if that's the case.  The benefit is that a
> path in spinning mode typically has less impact (faster) on waiters
> rather than that in blocking mode.
> 
> Also fixes the misalignment of the prototype, which is too trivial for
> a single patch.

^^^ removed as it refers to the hunks from v2.

> Signed-off-by: Liu Bo <bo.liu@linux.alibaba.com>

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