From: Filipe Manana <fdmanana@gmail.com>
To: Josef Bacik <josef@toxicpanda.com>
Cc: linux-btrfs <linux-btrfs@vger.kernel.org>, kernel-team@fb.com
Subject: Re: [PATCH 11/14] btrfs: pass root owner to read_tree_block
Date: Fri, 6 Nov 2020 11:55:31 +0000 [thread overview]
Message-ID: <CAL3q7H54rcjb-AumXTsfy9jHFwk12RGJxr2_GRHh-h3UBx8XQw@mail.gmail.com> (raw)
In-Reply-To: <9060bfcc7ea0806cb05ff8e7e8d7adb17d7b6e53.1604591048.git.josef@toxicpanda.com>
On Thu, Nov 5, 2020 at 3:49 PM Josef Bacik <josef@toxicpanda.com> wrote:
>
> In order to properly set the lockdep class of a newly allocated block we
> need to know the owner of the block. For non refcount'ed tree's this is
> straightforward, we always know in advance what tree we're reading from.
> For refcount'ed trees we don't necessarily know, however all refcount'ed
> trees share the same lockdep class name, tree-<level>.
>
> Fix all of the callers of read_tree_block() to pass in the root objectid
> we're using. In places like relocation and backref we could probably
> unconditionally use 0, but just in case use the root when we have it,
> otherwise use 0 in the cases we don't have the root as it's going to be
> a refcount'ed tree anyway.
>
> This is a preparation patch for further changes.
>
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Reviewed-by: Filipe Manana <fdmanana@suse.com>
I couldn't get anymore the lockdep splat I reported before (after
applying the whole patchset of course), it used to happen very often
with btrfs/033.
Looks good, thanks.
> ---
> fs/btrfs/backref.c | 6 +++---
> fs/btrfs/ctree.c | 8 +++++---
> fs/btrfs/disk-io.c | 14 +++++++++-----
> fs/btrfs/disk-io.h | 4 ++--
> fs/btrfs/extent-tree.c | 4 ++--
> fs/btrfs/print-tree.c | 1 +
> fs/btrfs/qgroup.c | 2 +-
> fs/btrfs/relocation.c | 4 ++--
> 8 files changed, 25 insertions(+), 18 deletions(-)
>
> diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
> index 86abe34e9444..02d7d7b2563b 100644
> --- a/fs/btrfs/backref.c
> +++ b/fs/btrfs/backref.c
> @@ -783,8 +783,8 @@ static int add_missing_keys(struct btrfs_fs_info *fs_info,
> BUG_ON(ref->key_for_search.type);
> BUG_ON(!ref->wanted_disk_byte);
>
> - eb = read_tree_block(fs_info, ref->wanted_disk_byte, 0,
> - ref->level - 1, NULL);
> + eb = read_tree_block(fs_info, ref->wanted_disk_byte,
> + ref->root_id, 0, ref->level - 1, NULL);
> if (IS_ERR(eb)) {
> free_pref(ref);
> return PTR_ERR(eb);
> @@ -1331,7 +1331,7 @@ static int find_parent_nodes(struct btrfs_trans_handle *trans,
> struct extent_buffer *eb;
>
> eb = read_tree_block(fs_info, ref->parent, 0,
> - ref->level, NULL);
> + 0, ref->level, NULL);
> if (IS_ERR(eb)) {
> ret = PTR_ERR(eb);
> goto out;
> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> index 5ec778e01222..35e5acbcc570 100644
> --- a/fs/btrfs/ctree.c
> +++ b/fs/btrfs/ctree.c
> @@ -1353,7 +1353,8 @@ get_old_root(struct btrfs_root *root, u64 time_seq)
> if (old_root && tm && tm->op != MOD_LOG_KEY_REMOVE_WHILE_FREEING) {
> btrfs_tree_read_unlock(eb_root);
> free_extent_buffer(eb_root);
> - old = read_tree_block(fs_info, logical, 0, level, NULL);
> + old = read_tree_block(fs_info, logical,
> + root->root_key.objectid, 0, level, NULL);
> if (WARN_ON(IS_ERR(old) || !extent_buffer_uptodate(old))) {
> if (!IS_ERR(old))
> free_extent_buffer(old);
> @@ -1763,6 +1764,7 @@ struct extent_buffer *btrfs_read_node_slot(struct extent_buffer *parent,
>
> btrfs_node_key_to_cpu(parent, &first_key, slot);
> eb = read_tree_block(parent->fs_info, btrfs_node_blockptr(parent, slot),
> + btrfs_header_owner(parent),
> btrfs_node_ptr_generation(parent, slot),
> level - 1, &first_key);
> if (!IS_ERR(eb) && !extent_buffer_uptodate(eb)) {
> @@ -2352,8 +2354,8 @@ read_block_for_search(struct btrfs_root *root, struct btrfs_path *p,
> reada_for_search(fs_info, p, level, slot, key->objectid);
>
> ret = -EAGAIN;
> - tmp = read_tree_block(fs_info, blocknr, gen, parent_level - 1,
> - &first_key);
> + tmp = read_tree_block(fs_info, blocknr, root->root_key.objectid,
> + gen, parent_level - 1, &first_key);
> if (!IS_ERR(tmp)) {
> /*
> * If the read above didn't mark this buffer up to date,
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index ec64e087520e..934723785ab8 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -958,13 +958,14 @@ struct extent_buffer *btrfs_find_create_tree_block(
> * Read tree block at logical address @bytenr and do variant basic but critical
> * verification.
> *
> + * @owner_root: the objectid of the root owner for this block.
> * @parent_transid: expected transid of this tree block, skip check if 0
> * @level: expected level, mandatory check
> * @first_key: expected key in slot 0, skip check if NULL
> */
> struct extent_buffer *read_tree_block(struct btrfs_fs_info *fs_info, u64 bytenr,
> - u64 parent_transid, int level,
> - struct btrfs_key *first_key)
> + u64 owner_root, u64 parent_transid,
> + int level, struct btrfs_key *first_key)
> {
> struct extent_buffer *buf = NULL;
> int ret;
> @@ -1287,7 +1288,7 @@ static struct btrfs_root *read_tree_root_path(struct btrfs_root *tree_root,
> level = btrfs_root_level(&root->root_item);
> root->node = read_tree_block(fs_info,
> btrfs_root_bytenr(&root->root_item),
> - generation, level, NULL);
> + key->objectid, generation, level, NULL);
> if (IS_ERR(root->node)) {
> ret = PTR_ERR(root->node);
> root->node = NULL;
> @@ -2241,8 +2242,9 @@ static int btrfs_replay_log(struct btrfs_fs_info *fs_info,
> return -ENOMEM;
>
> log_tree_root->node = read_tree_block(fs_info, bytenr,
> - fs_info->generation + 1,
> - level, NULL);
> + BTRFS_TREE_LOG_OBJECTID,
> + fs_info->generation + 1, level,
> + NULL);
> if (IS_ERR(log_tree_root->node)) {
> btrfs_warn(fs_info, "failed to read log tree");
> ret = PTR_ERR(log_tree_root->node);
> @@ -2628,6 +2630,7 @@ static int __cold init_tree_roots(struct btrfs_fs_info *fs_info)
> generation = btrfs_super_generation(sb);
> level = btrfs_super_root_level(sb);
> tree_root->node = read_tree_block(fs_info, btrfs_super_root(sb),
> + BTRFS_ROOT_TREE_OBJECTID,
> generation, level, NULL);
> if (IS_ERR(tree_root->node)) {
> handle_error = true;
> @@ -3116,6 +3119,7 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
>
> chunk_root->node = read_tree_block(fs_info,
> btrfs_super_chunk_root(disk_super),
> + BTRFS_CHUNK_TREE_OBJECTID,
> generation, level, NULL);
> if (IS_ERR(chunk_root->node) ||
> !extent_buffer_uptodate(chunk_root->node)) {
> diff --git a/fs/btrfs/disk-io.h b/fs/btrfs/disk-io.h
> index 009f505d6c97..41588babf2ed 100644
> --- a/fs/btrfs/disk-io.h
> +++ b/fs/btrfs/disk-io.h
> @@ -43,8 +43,8 @@ void btrfs_init_fs_info(struct btrfs_fs_info *fs_info);
> int btrfs_verify_level_key(struct extent_buffer *eb, int level,
> struct btrfs_key *first_key, u64 parent_transid);
> struct extent_buffer *read_tree_block(struct btrfs_fs_info *fs_info, u64 bytenr,
> - u64 parent_transid, int level,
> - struct btrfs_key *first_key);
> + u64 owner_root, u64 parent_transid,
> + int level, struct btrfs_key *first_key);
> struct extent_buffer *btrfs_find_create_tree_block(
> struct btrfs_fs_info *fs_info,
> u64 bytenr);
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index bf2f0af24e91..fe829bb528b5 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -5072,8 +5072,8 @@ static noinline int do_walk_down(struct btrfs_trans_handle *trans,
> if (!next) {
> if (reada && level == 1)
> reada_walk_down(trans, root, wc, path);
> - next = read_tree_block(fs_info, bytenr, generation, level - 1,
> - &first_key);
> + next = read_tree_block(fs_info, bytenr, root->root_key.objectid,
> + generation, level - 1, &first_key);
> if (IS_ERR(next)) {
> return PTR_ERR(next);
> } else if (!extent_buffer_uptodate(next)) {
> diff --git a/fs/btrfs/print-tree.c b/fs/btrfs/print-tree.c
> index 8db99117531d..fe5e0026129d 100644
> --- a/fs/btrfs/print-tree.c
> +++ b/fs/btrfs/print-tree.c
> @@ -390,6 +390,7 @@ void btrfs_print_tree(struct extent_buffer *c, bool follow)
>
> btrfs_node_key_to_cpu(c, &first_key, i);
> next = read_tree_block(fs_info, btrfs_node_blockptr(c, i),
> + btrfs_header_owner(c),
> btrfs_node_ptr_generation(c, i),
> level - 1, &first_key);
> if (IS_ERR(next)) {
> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
> index 25e3b7105e8a..8a78783213f9 100644
> --- a/fs/btrfs/qgroup.c
> +++ b/fs/btrfs/qgroup.c
> @@ -4140,7 +4140,7 @@ int btrfs_qgroup_trace_subtree_after_cow(struct btrfs_trans_handle *trans,
> spin_unlock(&blocks->lock);
>
> /* Read out reloc subtree root */
> - reloc_eb = read_tree_block(fs_info, block->reloc_bytenr,
> + reloc_eb = read_tree_block(fs_info, block->reloc_bytenr, 0,
> block->reloc_generation, block->level,
> &block->first_key);
> if (IS_ERR(reloc_eb)) {
> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index 465f5b4d3233..3e792bf31ecd 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -2413,7 +2413,7 @@ static int get_tree_block_key(struct btrfs_fs_info *fs_info,
> {
> struct extent_buffer *eb;
>
> - eb = read_tree_block(fs_info, block->bytenr, block->key.offset,
> + eb = read_tree_block(fs_info, block->bytenr, 0, block->key.offset,
> block->level, NULL);
> if (IS_ERR(eb)) {
> return PTR_ERR(eb);
> @@ -3039,7 +3039,7 @@ int add_data_references(struct reloc_control *rc,
> while ((ref_node = ulist_next(leaves, &leaf_uiter))) {
> struct extent_buffer *eb;
>
> - eb = read_tree_block(fs_info, ref_node->val, 0, 0, NULL);
> + eb = read_tree_block(fs_info, ref_node->val, 0, 0, 0, NULL);
> if (IS_ERR(eb)) {
> ret = PTR_ERR(eb);
> break;
> --
> 2.26.2
>
--
Filipe David Manana,
“Whether you think you can, or you think you can't — you're right.”
next prev parent reply other threads:[~2020-11-06 11:55 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-05 15:45 [PATCH 00/14][REBASED] Set the lockdep class on eb's at allocation time Josef Bacik
2020-11-05 15:45 ` [PATCH 01/14] btrfs: remove lockdep classes for the fs tree Josef Bacik
2020-11-06 11:50 ` Filipe Manana
2020-11-05 15:45 ` [PATCH 02/14] btrfs: cleanup extent buffer readahead Josef Bacik
2020-11-06 11:51 ` Filipe Manana
2020-11-09 15:09 ` David Sterba
2020-11-05 15:45 ` [PATCH 03/14] btrfs: use btrfs_read_node_slot in btrfs_realloc_node Josef Bacik
2020-11-06 11:51 ` Filipe Manana
2020-11-05 15:45 ` [PATCH 04/14] btrfs: use btrfs_read_node_slot in walk_down_reloc_tree Josef Bacik
2020-11-06 11:52 ` Filipe Manana
2020-11-05 15:45 ` [PATCH 05/14] btrfs: use btrfs_read_node_slot in do_relocation Josef Bacik
2020-11-06 11:52 ` Filipe Manana
2020-11-05 15:45 ` [PATCH 06/14] btrfs: use btrfs_read_node_slot in replace_path Josef Bacik
2020-11-06 11:53 ` Filipe Manana
2020-11-05 15:45 ` [PATCH 07/14] btrfs: use btrfs_read_node_slot in walk_down_tree Josef Bacik
2020-11-06 11:53 ` Filipe Manana
2020-11-05 15:45 ` [PATCH 08/14] btrfs: use btrfs_read_node_slot in qgroup_trace_extent_swap Josef Bacik
2020-11-06 11:53 ` Filipe Manana
2020-11-05 15:45 ` [PATCH 09/14] btrfs: use btrfs_read_node_slot in qgroup_trace_new_subtree_blocks Josef Bacik
2020-11-06 11:54 ` Filipe Manana
2020-11-05 15:45 ` [PATCH 10/14] btrfs: use btrfs_read_node_slot in btrfs_qgroup_trace_subtree Josef Bacik
2020-11-06 11:55 ` Filipe Manana
2020-11-05 15:45 ` [PATCH 11/14] btrfs: pass root owner to read_tree_block Josef Bacik
2020-11-06 11:55 ` Filipe Manana [this message]
2020-11-05 15:45 ` [PATCH 12/14] btrfs: pass the root owner and level around for reada Josef Bacik
2020-11-06 11:56 ` Filipe Manana
2020-11-05 15:45 ` [PATCH 13/14] btrfs: pass the owner_root and level to alloc_extent_buffer Josef Bacik
2020-11-06 11:54 ` Filipe Manana
2020-11-05 15:45 ` [PATCH 14/14] btrfs: set the lockdep class for ebs on creation Josef Bacik
2020-11-06 11:58 ` Filipe Manana
2020-11-09 16:50 ` [PATCH 00/14][REBASED] Set the lockdep class on eb's at allocation time David Sterba
-- strict thread matches above, loose matches on Subject: below --
2020-10-30 21:02 [PATCH 00/14] " Josef Bacik
2020-10-30 21:03 ` [PATCH 11/14] btrfs: pass root owner to read_tree_block Josef Bacik
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAL3q7H54rcjb-AumXTsfy9jHFwk12RGJxr2_GRHh-h3UBx8XQw@mail.gmail.com \
--to=fdmanana@gmail.com \
--cc=josef@toxicpanda.com \
--cc=kernel-team@fb.com \
--cc=linux-btrfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).