On 08/09/2016 03:30 AM, Qu Wenruo wrote:
> Refactor btrfs_qgroup_insert_dirty_extent() function, to two functions:
> 1. _btrfs_qgroup_insert_dirty_extent()
>    Almost the same with original code.
>    For delayed_ref usage, which has delayed refs locked.
> 
>    Change the return value type to int, since caller never needs the
>    pointer, but only needs to know if they need to free the allocated
>    memory.
> 
> 2. btrfs_qgroup_record_dirty_extent()
>    The more encapsulated version.
> 
>    Will do the delayed_refs lock, memory allocation, quota enabled check
>    and other misc things.
> 
> The original design is to keep exported functions to minimal, but since
> more btrfs hacks exposed, like replacing path in balance, needs us to
> record dirty extents manually, so we have to add such functions.
> 
> Also, add comment for both functions, to info developers how to keep
> qgroup correct when doing hacks.
> 
> Cc: Mark Fasheh <mfas...@suse.de>
> Cc: Goldwyn Rodrigues <rgold...@suse.de>
> Signed-off-by: Qu Wenruo <quwen...@cn.fujitsu.com>

Reviewed-and-Tested-by: Goldwyn Rodrigues <rgold...@suse.com>

> ---
>  fs/btrfs/delayed-ref.c |  7 ++-----
>  fs/btrfs/extent-tree.c | 37 +++++--------------------------------
>  fs/btrfs/qgroup.c      | 41 +++++++++++++++++++++++++++++++++++------
>  fs/btrfs/qgroup.h      | 33 +++++++++++++++++++++++++++++----
>  4 files changed, 71 insertions(+), 47 deletions(-)
> 
> diff --git a/fs/btrfs/delayed-ref.c b/fs/btrfs/delayed-ref.c
> index b6d210e..dd9b101 100644
> --- a/fs/btrfs/delayed-ref.c
> +++ b/fs/btrfs/delayed-ref.c
> @@ -541,7 +541,6 @@ add_delayed_ref_head(struct btrfs_fs_info *fs_info,
>       struct btrfs_delayed_ref_head *existing;
>       struct btrfs_delayed_ref_head *head_ref = NULL;
>       struct btrfs_delayed_ref_root *delayed_refs;
> -     struct btrfs_qgroup_extent_record *qexisting;
>       int count_mod = 1;
>       int must_insert_reserved = 0;
>  
> @@ -606,10 +605,8 @@ add_delayed_ref_head(struct btrfs_fs_info *fs_info,
>               qrecord->num_bytes = num_bytes;
>               qrecord->old_roots = NULL;
>  
> -             qexisting = btrfs_qgroup_insert_dirty_extent(fs_info,
> -                                                          delayed_refs,
> -                                                          qrecord);
> -             if (qexisting)
> +             if(btrfs_qgroup_insert_dirty_extent_nolock(fs_info,
> +                                     delayed_refs, qrecord))
>                       kfree(qrecord);
>       }
>  
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index c2b81b0..2a860c9 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -8521,35 +8521,6 @@ reada:
>       wc->reada_slot = slot;
>  }
>  
> -/*
> - * These may not be seen by the usual inc/dec ref code so we have to
> - * add them here.
> - */
> -static int record_one_subtree_extent(struct btrfs_trans_handle *trans,
> -                                  struct btrfs_root *root, u64 bytenr,
> -                                  u64 num_bytes)
> -{
> -     struct btrfs_qgroup_extent_record *qrecord;
> -     struct btrfs_delayed_ref_root *delayed_refs;
> -
> -     qrecord = kmalloc(sizeof(*qrecord), GFP_NOFS);
> -     if (!qrecord)
> -             return -ENOMEM;
> -
> -     qrecord->bytenr = bytenr;
> -     qrecord->num_bytes = num_bytes;
> -     qrecord->old_roots = NULL;
> -
> -     delayed_refs = &trans->transaction->delayed_refs;
> -     spin_lock(&delayed_refs->lock);
> -     if (btrfs_qgroup_insert_dirty_extent(trans->fs_info,
> -                                          delayed_refs, qrecord))
> -             kfree(qrecord);
> -     spin_unlock(&delayed_refs->lock);
> -
> -     return 0;
> -}
> -
>  static int account_leaf_items(struct btrfs_trans_handle *trans,
>                             struct btrfs_root *root,
>                             struct extent_buffer *eb)
> @@ -8583,7 +8554,8 @@ static int account_leaf_items(struct btrfs_trans_handle 
> *trans,
>  
>               num_bytes = btrfs_file_extent_disk_num_bytes(eb, fi);
>  
> -             ret = record_one_subtree_extent(trans, root, bytenr, num_bytes);
> +             ret = btrfs_qgroup_insert_dirty_extent(trans, root->fs_info,
> +                             bytenr, num_bytes, GFP_NOFS);
>               if (ret)
>                       return ret;
>       }
> @@ -8732,8 +8704,9 @@ walk_down:
>                       btrfs_set_lock_blocking_rw(eb, BTRFS_READ_LOCK);
>                       path->locks[level] = BTRFS_READ_LOCK_BLOCKING;
>  
> -                     ret = record_one_subtree_extent(trans, root, 
> child_bytenr,
> -                                                     root->nodesize);
> +                     ret = btrfs_qgroup_insert_dirty_extent(trans,
> +                                     root->fs_info, child_bytenr,
> +                                     root->nodesize, GFP_NOFS);
>                       if (ret)
>                               goto out;
>               }
> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
> index 93ee1c1..1f29693 100644
> --- a/fs/btrfs/qgroup.c
> +++ b/fs/btrfs/qgroup.c
> @@ -1453,10 +1453,9 @@ int btrfs_qgroup_prepare_account_extents(struct 
> btrfs_trans_handle *trans,
>       return ret;
>  }
>  
> -struct btrfs_qgroup_extent_record *
> -btrfs_qgroup_insert_dirty_extent(struct btrfs_fs_info *fs_info,
> -                              struct btrfs_delayed_ref_root *delayed_refs,
> -                              struct btrfs_qgroup_extent_record *record)
> +int btrfs_qgroup_insert_dirty_extent_nolock(struct btrfs_fs_info *fs_info,
> +                             struct btrfs_delayed_ref_root *delayed_refs,
> +                             struct btrfs_qgroup_extent_record *record)
>  {
>       struct rb_node **p = &delayed_refs->dirty_extent_root.rb_node;
>       struct rb_node *parent_node = NULL;
> @@ -1475,12 +1474,42 @@ btrfs_qgroup_insert_dirty_extent(struct btrfs_fs_info 
> *fs_info,
>               else if (bytenr > entry->bytenr)
>                       p = &(*p)->rb_right;
>               else
> -                     return entry;
> +                     return 1;
>       }
>  
>       rb_link_node(&record->node, parent_node, p);
>       rb_insert_color(&record->node, &delayed_refs->dirty_extent_root);
> -     return NULL;
> +     return 0;
> +}
> +
> +int btrfs_qgroup_insert_dirty_extent(struct btrfs_trans_handle *trans,
> +             struct btrfs_fs_info *fs_info, u64 bytenr, u64 num_bytes,
> +             gfp_t gfp_flag)
> +{
> +     struct btrfs_qgroup_extent_record *record;
> +     struct btrfs_delayed_ref_root *delayed_refs;
> +     int ret;
> +
> +     if (!fs_info->quota_enabled || bytenr == 0 || num_bytes == 0)
> +             return 0;
> +     if (WARN_ON(trans == NULL))
> +             return -EINVAL;
> +     record = kmalloc(sizeof(*record), gfp_flag);
> +     if (!record)
> +             return -ENOMEM;
> +
> +     delayed_refs = &trans->transaction->delayed_refs;
> +     record->bytenr = bytenr;
> +     record->num_bytes = num_bytes;
> +     record->old_roots = NULL;
> +
> +     spin_lock(&delayed_refs->lock);
> +     ret = btrfs_qgroup_insert_dirty_extent_nolock(fs_info, delayed_refs,
> +                                                   record);
> +     spin_unlock(&delayed_refs->lock);
> +     if (ret > 0)
> +             kfree(record);
> +     return 0;
>  }
>  
>  #define UPDATE_NEW   0
> diff --git a/fs/btrfs/qgroup.h b/fs/btrfs/qgroup.h
> index 710887c..f6a38e4 100644
> --- a/fs/btrfs/qgroup.h
> +++ b/fs/btrfs/qgroup.h
> @@ -63,10 +63,35 @@ void btrfs_free_qgroup_config(struct btrfs_fs_info 
> *fs_info);
>  struct btrfs_delayed_extent_op;
>  int btrfs_qgroup_prepare_account_extents(struct btrfs_trans_handle *trans,
>                                        struct btrfs_fs_info *fs_info);
> -struct btrfs_qgroup_extent_record *
> -btrfs_qgroup_insert_dirty_extent(struct btrfs_fs_info *fs_info,
> -                              struct btrfs_delayed_ref_root *delayed_refs,
> -                              struct btrfs_qgroup_extent_record *record);
> +/*
> + * Insert one dirty extent record into @delayed_refs, informing qgroup to
> + * account that extent at commit trans time.
> + *
> + * No lock version, caller must acquire delayed ref lock and allocate memory.
> + *
> + * Return 0 for success insert
> + * Return >0 for existing record, caller can free @record safely.
> + * Error is not possible
> + */
> +int btrfs_qgroup_insert_dirty_extent_nolock(
> +             struct btrfs_fs_info *fs_info,
> +             struct btrfs_delayed_ref_root *delayed_refs,
> +             struct btrfs_qgroup_extent_record *record);
> +
> +/*
> + * Insert one dirty extent record into @delayed_refs, informing qgroup to
> + * account that extent at commit trans time.
> + *
> + * Better encapsulated version.
> + *
> + * Return 0 if the operation is done.
> + * Return <0 for error, like memory allocation failure or invalid parameter
> + * (NULL trans)
> + */
> +int btrfs_qgroup_insert_dirty_extent(struct btrfs_trans_handle *trans,
> +             struct btrfs_fs_info *fs_info, u64 bytenr, u64 num_bytes,
> +             gfp_t gfp_flag);
> +
>  int
>  btrfs_qgroup_account_extent(struct btrfs_trans_handle *trans,
>                           struct btrfs_fs_info *fs_info,
> 

-- 
Goldwyn
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to