Re: [PATCH 2/2] Btrfs: kill leave_spinning
Please kindly ignore this patch as it did introduce soft lockup when
btrfs_search_slot() returns a spinning path and its callers goes to
sleep before releasing the path.
thanks,
liubo
On Thu, Aug 16, 2018 at 2:07 PM, Liu Bo wrote:
> As btrfs_clear_path_blocking() turns out to be a major source of lock
> contention, we've kill it and without it btrfs_search_slot() and
> btrfs_search_old_slot() are not able to return a path in spinning
> mode, lets kill leave_spinning, too.
>
> Signed-off-by: Liu Bo
> ---
> fs/btrfs/backref.c| 3 ---
> fs/btrfs/ctree.c | 16 +++-
> fs/btrfs/ctree.h | 1 -
> fs/btrfs/delayed-inode.c | 4
> fs/btrfs/dir-item.c | 1 -
> fs/btrfs/export.c | 1 -
> fs/btrfs/extent-tree.c| 7 ---
> fs/btrfs/extent_io.c | 1 -
> fs/btrfs/file-item.c | 4
> fs/btrfs/free-space-tree.c| 2 --
> fs/btrfs/inode-item.c | 6 --
> fs/btrfs/inode.c | 8
> fs/btrfs/ioctl.c | 3 ---
> fs/btrfs/qgroup.c | 2 --
> fs/btrfs/super.c | 2 --
> fs/btrfs/tests/qgroup-tests.c | 4
> 16 files changed, 3 insertions(+), 62 deletions(-)
>
> diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
> index ae750b1574a2..70c629b90710 100644
> --- a/fs/btrfs/backref.c
> +++ b/fs/btrfs/backref.c
> @@ -1613,13 +1613,11 @@ char *btrfs_ref_to_path(struct btrfs_root *fs_root,
> struct btrfs_path *path,
> s64 bytes_left = ((s64)size) - 1;
> struct extent_buffer *eb = eb_in;
> struct btrfs_key found_key;
> - int leave_spinning = path->leave_spinning;
> struct btrfs_inode_ref *iref;
>
> if (bytes_left >= 0)
> dest[bytes_left] = '\0';
>
> - path->leave_spinning = 1;
> while (1) {
> bytes_left -= name_len;
> if (bytes_left >= 0)
> @@ -1665,7 +1663,6 @@ char *btrfs_ref_to_path(struct btrfs_root *fs_root,
> struct btrfs_path *path,
> }
>
> btrfs_release_path(path);
> - path->leave_spinning = leave_spinning;
>
> if (ret)
> return ERR_PTR(ret);
> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> index 8b31caa60b0a..d2df7cfbec06 100644
> --- a/fs/btrfs/ctree.c
> +++ b/fs/btrfs/ctree.c
> @@ -2875,14 +2875,10 @@ int btrfs_search_slot(struct btrfs_trans_handle
> *trans, struct btrfs_root *root,
> }
> ret = 1;
> done:
> - /*
> -* we don't really know what they plan on doing with the path
> -* from here on, so for now just mark it as blocking
> -*/
> - if (!p->leave_spinning)
> - btrfs_set_path_blocking(p);
> if (ret < 0 && !p->skip_release_on_error)
> btrfs_release_path(p);
> +
> + /* path is supposed to be in blocking mode from now on. */
> return ret;
> }
>
> @@ -2987,11 +2983,10 @@ int btrfs_search_old_slot(struct btrfs_root *root,
> const struct btrfs_key *key,
> }
> ret = 1;
> done:
> - if (!p->leave_spinning)
> - btrfs_set_path_blocking(p);
> if (ret < 0)
> btrfs_release_path(p);
>
> + /* path is supposed to be in blocking mode from now on.*/
> return ret;
> }
>
> @@ -5628,7 +5623,6 @@ int btrfs_next_old_leaf(struct btrfs_root *root, struct
> btrfs_path *path,
> struct btrfs_key key;
> u32 nritems;
> int ret;
> - int old_spinning = path->leave_spinning;
> int next_rw_lock = 0;
>
> nritems = btrfs_header_nritems(path->nodes[0]);
> @@ -5643,7 +5637,6 @@ int btrfs_next_old_leaf(struct btrfs_root *root, struct
> btrfs_path *path,
> btrfs_release_path(path);
>
> path->keep_locks = 1;
> - path->leave_spinning = 1;
>
> if (time_seq)
> ret = btrfs_search_old_slot(root, &key, path, time_seq);
> @@ -5780,9 +5773,6 @@ int btrfs_next_old_leaf(struct btrfs_root *root, struct
> btrfs_path *path,
> ret = 0;
> done:
> unlock_up(path, 0, 1, 0, NULL);
> - path->leave_spinning = old_spinning;
> - if (!old_spinning)
> - btrfs_set_path_blocking(path);
>
> return ret;
> }
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index 1aeed3c0e949..e8bddf21b7f7 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -339,7 +339,6 @@ struct btrfs_path {
> unsigned int search_for_split:1;
> unsigned int keep_locks:1;
> unsigned int skip_locking:1;
> - unsigned int leave_spinning:1;
> unsigned int search_commit_root:1;
> unsigned int need_commit_sem:1;
> unsigned int skip_release_on_error:1;
> diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c
> index db9f45082c61..e6fbcdbc313e 100644
> --- a/fs/btrfs/delayed-inode.c
> +++ b/fs/btrfs/delayed-inode.c
> @@ -1138,7 +1138,6 @@ static int __btrfs_run_delayed_i
[PATCH 2/2] Btrfs: kill leave_spinning
As btrfs_clear_path_blocking() turns out to be a major source of lock
contention, we've kill it and without it btrfs_search_slot() and
btrfs_search_old_slot() are not able to return a path in spinning
mode, lets kill leave_spinning, too.
Signed-off-by: Liu Bo
---
fs/btrfs/backref.c| 3 ---
fs/btrfs/ctree.c | 16 +++-
fs/btrfs/ctree.h | 1 -
fs/btrfs/delayed-inode.c | 4
fs/btrfs/dir-item.c | 1 -
fs/btrfs/export.c | 1 -
fs/btrfs/extent-tree.c| 7 ---
fs/btrfs/extent_io.c | 1 -
fs/btrfs/file-item.c | 4
fs/btrfs/free-space-tree.c| 2 --
fs/btrfs/inode-item.c | 6 --
fs/btrfs/inode.c | 8
fs/btrfs/ioctl.c | 3 ---
fs/btrfs/qgroup.c | 2 --
fs/btrfs/super.c | 2 --
fs/btrfs/tests/qgroup-tests.c | 4
16 files changed, 3 insertions(+), 62 deletions(-)
diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
index ae750b1574a2..70c629b90710 100644
--- a/fs/btrfs/backref.c
+++ b/fs/btrfs/backref.c
@@ -1613,13 +1613,11 @@ char *btrfs_ref_to_path(struct btrfs_root *fs_root,
struct btrfs_path *path,
s64 bytes_left = ((s64)size) - 1;
struct extent_buffer *eb = eb_in;
struct btrfs_key found_key;
- int leave_spinning = path->leave_spinning;
struct btrfs_inode_ref *iref;
if (bytes_left >= 0)
dest[bytes_left] = '\0';
- path->leave_spinning = 1;
while (1) {
bytes_left -= name_len;
if (bytes_left >= 0)
@@ -1665,7 +1663,6 @@ char *btrfs_ref_to_path(struct btrfs_root *fs_root,
struct btrfs_path *path,
}
btrfs_release_path(path);
- path->leave_spinning = leave_spinning;
if (ret)
return ERR_PTR(ret);
diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index 8b31caa60b0a..d2df7cfbec06 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -2875,14 +2875,10 @@ int btrfs_search_slot(struct btrfs_trans_handle *trans,
struct btrfs_root *root,
}
ret = 1;
done:
- /*
-* we don't really know what they plan on doing with the path
-* from here on, so for now just mark it as blocking
-*/
- if (!p->leave_spinning)
- btrfs_set_path_blocking(p);
if (ret < 0 && !p->skip_release_on_error)
btrfs_release_path(p);
+
+ /* path is supposed to be in blocking mode from now on. */
return ret;
}
@@ -2987,11 +2983,10 @@ int btrfs_search_old_slot(struct btrfs_root *root,
const struct btrfs_key *key,
}
ret = 1;
done:
- if (!p->leave_spinning)
- btrfs_set_path_blocking(p);
if (ret < 0)
btrfs_release_path(p);
+ /* path is supposed to be in blocking mode from now on.*/
return ret;
}
@@ -5628,7 +5623,6 @@ int btrfs_next_old_leaf(struct btrfs_root *root, struct
btrfs_path *path,
struct btrfs_key key;
u32 nritems;
int ret;
- int old_spinning = path->leave_spinning;
int next_rw_lock = 0;
nritems = btrfs_header_nritems(path->nodes[0]);
@@ -5643,7 +5637,6 @@ int btrfs_next_old_leaf(struct btrfs_root *root, struct
btrfs_path *path,
btrfs_release_path(path);
path->keep_locks = 1;
- path->leave_spinning = 1;
if (time_seq)
ret = btrfs_search_old_slot(root, &key, path, time_seq);
@@ -5780,9 +5773,6 @@ int btrfs_next_old_leaf(struct btrfs_root *root, struct
btrfs_path *path,
ret = 0;
done:
unlock_up(path, 0, 1, 0, NULL);
- path->leave_spinning = old_spinning;
- if (!old_spinning)
- btrfs_set_path_blocking(path);
return ret;
}
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 1aeed3c0e949..e8bddf21b7f7 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -339,7 +339,6 @@ struct btrfs_path {
unsigned int search_for_split:1;
unsigned int keep_locks:1;
unsigned int skip_locking:1;
- unsigned int leave_spinning:1;
unsigned int search_commit_root:1;
unsigned int need_commit_sem:1;
unsigned int skip_release_on_error:1;
diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c
index db9f45082c61..e6fbcdbc313e 100644
--- a/fs/btrfs/delayed-inode.c
+++ b/fs/btrfs/delayed-inode.c
@@ -1138,7 +1138,6 @@ static int __btrfs_run_delayed_items(struct
btrfs_trans_handle *trans, int nr)
path = btrfs_alloc_path();
if (!path)
return -ENOMEM;
- path->leave_spinning = 1;
block_rsv = trans->block_rsv;
trans->block_rsv = &fs_info->delayed_block_rsv;
@@ -1203,7 +1202,6 @@ int btrfs_commit_inode_delayed_items(struct
btrfs_trans_handle *trans,
btrfs_release_delayed_node(delayed_node);
return -ENOMEM;
}
- path->leave_spi
