On 2018/12/10 下午5:21, Nikolay Borisov wrote:
>
>
> On 6.12.18 г. 8:58 ч., Qu Wenruo wrote:
>> btrfs_add_delayed_tree_ref() has a longer and longer parameter list, and
>> some caller like btrfs_inc_extent_ref() are using @owner as level for
>> delayed tree ref.
>>
>> Instead of making the parameter list longer and longer, use btrfs_ref to
>> refactor it, so each parameter assignment should be self-explaining
>> without dirty level/owner trick, and provides the basis for later refactor.
>>
>> Signed-off-by: Qu Wenruo <wqu@xxxxxxxx>
>> ---
>> fs/btrfs/delayed-ref.c | 24 ++++++++++++++-------
>> fs/btrfs/delayed-ref.h | 4 +---
>> fs/btrfs/extent-tree.c | 48 ++++++++++++++++++++++++------------------
>> 3 files changed, 44 insertions(+), 32 deletions(-)
>>
>> diff --git a/fs/btrfs/delayed-ref.c b/fs/btrfs/delayed-ref.c
>> index 11dd46be4017..c42b8ade7b07 100644
>> --- a/fs/btrfs/delayed-ref.c
>> +++ b/fs/btrfs/delayed-ref.c
>> @@ -710,9 +710,7 @@ static void init_delayed_ref_common(struct btrfs_fs_info *fs_info,
>> * transaction commits.
>> */
>> int btrfs_add_delayed_tree_ref(struct btrfs_trans_handle *trans,
>> - u64 bytenr, u64 num_bytes, u64 parent,
>> - u64 ref_root, int level, bool for_reloc,
>> - int action,
>> + struct btrfs_ref *generic_ref,
>> struct btrfs_delayed_extent_op *extent_op,
>> int *old_ref_mod, int *new_ref_mod)
>> {
>> @@ -722,10 +720,17 @@ int btrfs_add_delayed_tree_ref(struct btrfs_trans_handle *trans,
>> struct btrfs_delayed_ref_root *delayed_refs;
>> struct btrfs_qgroup_extent_record *record = NULL;
>> int qrecord_inserted;
>> - bool is_system = (ref_root == BTRFS_CHUNK_TREE_OBJECTID);
>> + bool is_system = (generic_ref->real_root == BTRFS_CHUNK_TREE_OBJECTID);
>> + int action = generic_ref->action;
>> + int level = generic_ref->tree_ref.level;
>> int ret;
>> + u64 bytenr = generic_ref->bytenr;
>> + u64 num_bytes = generic_ref->len;
>> + u64 parent = generic_ref->parent;
>> u8 ref_type;
>>
>> + ASSERT(generic_ref && generic_ref->type == BTRFS_REF_METADATA &&
>> + generic_ref->action);
>
> You have already dereferenced generic_ref by the time you ASSERT and
> would have crashed. I guess the first condition in the assert can be
> removed.
>
>> BUG_ON(extent_op && extent_op->is_data);
>> ref = kmem_cache_alloc(btrfs_delayed_tree_ref_cachep, GFP_NOFS);
>> if (!ref)
>
> <snip>
>
>> --- a/fs/btrfs/extent-tree.c
>> +++ b/fs/btrfs/extent-tree.c
>> @@ -2031,6 +2031,7 @@ int btrfs_inc_extent_ref(struct btrfs_trans_handle *trans,
>> bool for_reloc)
>> {
>> struct btrfs_fs_info *fs_info = root->fs_info;
>> + struct btrfs_ref generic_ref = { 0 };
>> int old_ref_mod, new_ref_mod;
>> int ret;
>>
>> @@ -2040,13 +2041,13 @@ int btrfs_inc_extent_ref(struct btrfs_trans_handle *trans,
>> btrfs_ref_tree_mod(root, bytenr, num_bytes, parent, root_objectid,
>> owner, offset, BTRFS_ADD_DELAYED_REF);
>>
>> + btrfs_init_generic_ref(&generic_ref, BTRFS_ADD_DELAYED_REF, bytenr,
>> + num_bytes, root->root_key.objectid, parent);
>> + generic_ref.skip_qgroup = for_reloc;
>> if (owner < BTRFS_FIRST_FREE_OBJECTID) {
>> - ret = btrfs_add_delayed_tree_ref(trans, bytenr,
>> - num_bytes, parent,
>> - root_objectid, (int)owner,
>> - for_reloc,
>> - BTRFS_ADD_DELAYED_REF, NULL,
>> - &old_ref_mod, &new_ref_mod);
>> + btrfs_init_tree_ref(&generic_ref, (int)owner, root_objectid);
>
> The cast is now redundant since the parameter definition of
> btrfs_init_tree_ref is an int.
It will be redundant when btrfs_free_extent() is converted to use btrfs_ref.
But not now.
As @owner is still u64, while parameter of btrfs_init_tree_ref() is
still using int for @level.
Thanks,
Qu
>> + ret = btrfs_add_delayed_tree_ref(trans, &generic_ref,
>> + NULL, &old_ref_mod, &new_ref_mod);
>> } else {
>> ret = btrfs_add_delayed_data_ref(trans, root, bytenr,
>> num_bytes, parent,
>
> <snip>
>
>> @@ -7102,11 +7110,8 @@ int btrfs_free_extent(struct btrfs_trans_handle *trans,
>> old_ref_mod = new_ref_mod = 0;
>> ret = 0;
>> } else if (owner < BTRFS_FIRST_FREE_OBJECTID) {
>> - ret = btrfs_add_delayed_tree_ref(trans, bytenr,
>> - num_bytes, parent,
>> - root_objectid, (int)owner,
>> - for_reloc,
>> - BTRFS_DROP_DELAYED_REF, NULL,
>> + btrfs_init_tree_ref(&generic_ref, (int)owner, root_objectid);
>
> ditto about the cast
>> + ret = btrfs_add_delayed_tree_ref(trans, &generic_ref, NULL,
>> &old_ref_mod, &new_ref_mod);
>> } else {
>> ret = btrfs_add_delayed_data_ref(trans, root, bytenr,
>
> <sniop>
>
