On 24.04.2018 17:18, Nikolay Borisov wrote:
> Now that the initialization part and the critical section code have
> been split it's a lot easier to open code add_delayed_tree_ref. Do
> so in the following manner:
>
> 1. The commin init code is put immediately after memory-to-be-init is
> allocate, followed by the ref-specific member initialization.
>
> 2. The only piece of code that remains in the critical section is
> insert_delayed_ref call.
>
> 3. Tracing and memory freeing code is put outside of the critical
> section as well.
>
> The only real change here is an overall shorter critical section when
> dealing with delayed tree refs. From functional point of view - the
> code is unchanged.
>
> Signed-off-by: Nikolay Borisov <nborisov@xxxxxxxx>
> ---
> fs/btrfs/delayed-ref.c | 66 ++++++++++++++++----------------------------------
> 1 file changed, 21 insertions(+), 45 deletions(-)
>
> diff --git a/fs/btrfs/delayed-ref.c b/fs/btrfs/delayed-ref.c
> index 3ab2ba94d6f4..827ca01cf5af 100644
> --- a/fs/btrfs/delayed-ref.c
> +++ b/fs/btrfs/delayed-ref.c
> @@ -696,49 +696,6 @@ static void init_delayed_ref_common(struct btrfs_fs_info *fs_info,
> }
>
> /*
> - * helper to insert a delayed tree ref into the rbtree.
> - */
> -static noinline void
> -add_delayed_tree_ref(struct btrfs_fs_info *fs_info,
> - struct btrfs_trans_handle *trans,
> - struct btrfs_delayed_ref_head *head_ref,
> - struct btrfs_delayed_ref_node *ref, u64 bytenr,
> - u64 num_bytes, u64 parent, u64 ref_root, int level,
> - int action)
> -{
> - struct btrfs_delayed_tree_ref *full_ref;
> - struct btrfs_delayed_ref_root *delayed_refs;
> - u8 ref_type;
> - int ret;
> -
> - delayed_refs = &trans->transaction->delayed_refs;
> - full_ref = btrfs_delayed_node_to_tree_ref(ref);
> - if (parent)
> - ref_type = BTRFS_SHARED_BLOCK_REF_KEY;
> - else
> - ref_type = BTRFS_TREE_BLOCK_REF_KEY;
> -
> - init_delayed_ref_common(fs_info, ref, bytenr, num_bytes, ref_root,
> - action, ref_type);
> - full_ref->root = ref_root;
> - full_ref->parent = parent;
> - full_ref->level = level;
> -
> - trace_add_delayed_tree_ref(fs_info, ref, full_ref,
> - action == BTRFS_ADD_DELAYED_EXTENT ?
> - BTRFS_ADD_DELAYED_REF : action);
> -
> - ret = insert_delayed_ref(trans, delayed_refs, head_ref, ref);
> -
> - /*
> - * XXX: memory should be freed at the same level allocated.
> - * But bad practice is anywhere... Follow it now. Need cleanup.
> - */
> - if (ret > 0)
> - kmem_cache_free(btrfs_delayed_tree_ref_cachep, full_ref);
> -}
> -
> -/*
> * helper to insert a delayed data ref into the rbtree.
> */
> static noinline void
> @@ -795,12 +752,25 @@ int btrfs_add_delayed_tree_ref(struct btrfs_fs_info *fs_info,
> struct btrfs_qgroup_extent_record *record = NULL;
> int qrecord_inserted;
> int is_system = (ref_root == BTRFS_CHUNK_TREE_OBJECTID);
> + int ret;
> + u8 ref_type = parent ? BTRFS_SHARED_BLOCK_REF_KEY :
> + BTRFS_TREE_BLOCK_REF_KEY;
>
> BUG_ON(extent_op && extent_op->is_data);
> ref = kmem_cache_alloc(btrfs_delayed_tree_ref_cachep, GFP_NOFS);
> if (!ref)
> return -ENOMEM;
>
> + if (parent)
> + ref_type = BTRFS_SHARED_BLOCK_REF_KEY;
> + else
> + ref_type = BTRFS_TREE_BLOCK_REF_KEY;
Ooops, this if (parent) check is duplicated with the "parent ? " at the
beginning of the function. Would you care to delete either of them -
perhaps this one, or should I send a fixlet for you to squash ?
> + init_delayed_ref_common(fs_info, &ref->node, bytenr, num_bytes,
> + ref_root, action, ref_type);
> + ref->root = ref_root;
> + ref->parent = parent;
> + ref->level = level;
> +
> head_ref = kmem_cache_alloc(btrfs_delayed_ref_head_cachep, GFP_NOFS);
> if (!head_ref)
> goto free_ref;
> @@ -826,10 +796,16 @@ int btrfs_add_delayed_tree_ref(struct btrfs_fs_info *fs_info,
> is_system, &qrecord_inserted,
> old_ref_mod, new_ref_mod);
>
> - add_delayed_tree_ref(fs_info, trans, head_ref, &ref->node, bytenr,
> - num_bytes, parent, ref_root, level, action);
> +
> + ret = insert_delayed_ref(trans, delayed_refs, head_ref, &ref->node);
> spin_unlock(&delayed_refs->lock);
>
> + trace_add_delayed_tree_ref(fs_info, &ref->node, ref,
> + action == BTRFS_ADD_DELAYED_EXTENT ?
> + BTRFS_ADD_DELAYED_REF : action);
> + if (ret > 0)
> + kmem_cache_free(btrfs_delayed_tree_ref_cachep, ref);
> +
> if (qrecord_inserted)
> btrfs_qgroup_trace_extent_post(fs_info, record);
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
