Re: [PATCH v2 1/3] btrfs: qgroup: Refactor btrfs_qgroup_insert_dirty_extent()
At 08/09/2016 10:24 AM, Goldwyn Rodrigues wrote: On 08/08/2016 08:12 PM, Qu Wenruo wrote: At 08/09/2016 09:01 AM, Goldwyn Rodrigues wrote: On 08/08/2016 07:26 PM, Qu Wenruo wrote: At 08/08/2016 10:54 AM, Goldwyn Rodrigues wrote: On 08/07/2016 07:39 PM, Qu Wenruo wrote: At 08/07/2016 01:19 AM, Goldwyn Rodrigues wrote: Thanks Qu, This patch set fixes all the reported problems. However, I have some minor issues with coding style. Thanks for the test. On 08/04/2016 09:29 PM, 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 Signed-off-by: Qu Wenruo --- fs/btrfs/delayed-ref.c | 5 + fs/btrfs/extent-tree.c | 36 +--- fs/btrfs/qgroup.c | 39 ++- fs/btrfs/qgroup.h | 44 +--- 4 files changed, 81 insertions(+), 43 deletions(-) diff --git a/fs/btrfs/delayed-ref.c b/fs/btrfs/delayed-ref.c index 430b368..5eed597 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,9 +605,7 @@ 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(delayed_refs, - qrecord); -if (qexisting) +if(_btrfs_qgroup_insert_dirty_extent(delayed_refs, qrecord)) kfree(qrecord); } diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c index 9fcb8c9..47c85ff 100644 --- a/fs/btrfs/extent-tree.c +++ b/fs/btrfs/extent-tree.c @@ -8519,34 +8519,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(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) @@ -8580,7 +8552,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_record_dirty_extent(trans, root->fs_info, +bytenr, num_bytes, GFP_NOFS); if (ret) return ret; } @@ -8729,8 +8702,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_record_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 9d4c05b..76d4f67 100644 --- a/fs/btrfs/qgroup.c +++ b/fs/btrfs/qgroup.c @@ -1453,9 +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_delayed_ref_root *delay
Re: [PATCH v2 1/3] btrfs: qgroup: Refactor btrfs_qgroup_insert_dirty_extent()
On 08/08/2016 08:12 PM, Qu Wenruo wrote: > > > At 08/09/2016 09:01 AM, Goldwyn Rodrigues wrote: >> >> >> On 08/08/2016 07:26 PM, Qu Wenruo wrote: >>> >>> >>> At 08/08/2016 10:54 AM, Goldwyn Rodrigues wrote: On 08/07/2016 07:39 PM, Qu Wenruo wrote: > > > At 08/07/2016 01:19 AM, Goldwyn Rodrigues wrote: >> Thanks Qu, >> >> This patch set fixes all the reported problems. However, I have some >> minor issues with coding style. >> > > Thanks for the test. > >> >> On 08/04/2016 09:29 PM, 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 >>> Signed-off-by: Qu Wenruo >>> --- >>> fs/btrfs/delayed-ref.c | 5 + >>> fs/btrfs/extent-tree.c | 36 +--- >>> fs/btrfs/qgroup.c | 39 ++- >>> fs/btrfs/qgroup.h | 44 >>> +--- >>> 4 files changed, 81 insertions(+), 43 deletions(-) >>> >>> diff --git a/fs/btrfs/delayed-ref.c b/fs/btrfs/delayed-ref.c >>> index 430b368..5eed597 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,9 +605,7 @@ 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(delayed_refs, >>> - qrecord); >>> -if (qexisting) >>> +if(_btrfs_qgroup_insert_dirty_extent(delayed_refs, >>> qrecord)) >>> kfree(qrecord); >>> } >>> >>> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c >>> index 9fcb8c9..47c85ff 100644 >>> --- a/fs/btrfs/extent-tree.c >>> +++ b/fs/btrfs/extent-tree.c >>> @@ -8519,34 +8519,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(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) >>> @@ -8580,7 +8552,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_record_dirty_extent(trans, >>> root->fs_info
Re: [PATCH v2 1/3] btrfs: qgroup: Refactor btrfs_qgroup_insert_dirty_extent()
At 08/09/2016 09:01 AM, Goldwyn Rodrigues wrote: On 08/08/2016 07:26 PM, Qu Wenruo wrote: At 08/08/2016 10:54 AM, Goldwyn Rodrigues wrote: On 08/07/2016 07:39 PM, Qu Wenruo wrote: At 08/07/2016 01:19 AM, Goldwyn Rodrigues wrote: Thanks Qu, This patch set fixes all the reported problems. However, I have some minor issues with coding style. Thanks for the test. On 08/04/2016 09:29 PM, 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 Signed-off-by: Qu Wenruo --- fs/btrfs/delayed-ref.c | 5 + fs/btrfs/extent-tree.c | 36 +--- fs/btrfs/qgroup.c | 39 ++- fs/btrfs/qgroup.h | 44 +--- 4 files changed, 81 insertions(+), 43 deletions(-) diff --git a/fs/btrfs/delayed-ref.c b/fs/btrfs/delayed-ref.c index 430b368..5eed597 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,9 +605,7 @@ 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(delayed_refs, - qrecord); -if (qexisting) +if(_btrfs_qgroup_insert_dirty_extent(delayed_refs, qrecord)) kfree(qrecord); } diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c index 9fcb8c9..47c85ff 100644 --- a/fs/btrfs/extent-tree.c +++ b/fs/btrfs/extent-tree.c @@ -8519,34 +8519,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(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) @@ -8580,7 +8552,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_record_dirty_extent(trans, root->fs_info, +bytenr, num_bytes, GFP_NOFS); if (ret) return ret; } @@ -8729,8 +8702,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_record_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 9d4c05b..76d4f67 100644 --- a/fs/btrfs/qgroup.c +++ b/fs/btrfs/qgroup.c @@ -1453,9 +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_delayed_ref_root *delayed_refs, - struct btrfs_qgroup_extent_record *record) +int _btrfs_qgroup_insert
Re: [PATCH v2 1/3] btrfs: qgroup: Refactor btrfs_qgroup_insert_dirty_extent()
On 08/08/2016 07:26 PM, Qu Wenruo wrote: > > > At 08/08/2016 10:54 AM, Goldwyn Rodrigues wrote: >> >> >> On 08/07/2016 07:39 PM, Qu Wenruo wrote: >>> >>> >>> At 08/07/2016 01:19 AM, Goldwyn Rodrigues wrote: Thanks Qu, This patch set fixes all the reported problems. However, I have some minor issues with coding style. >>> >>> Thanks for the test. >>> On 08/04/2016 09:29 PM, 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 > Signed-off-by: Qu Wenruo > --- > fs/btrfs/delayed-ref.c | 5 + > fs/btrfs/extent-tree.c | 36 +--- > fs/btrfs/qgroup.c | 39 ++- > fs/btrfs/qgroup.h | 44 > +--- > 4 files changed, 81 insertions(+), 43 deletions(-) > > diff --git a/fs/btrfs/delayed-ref.c b/fs/btrfs/delayed-ref.c > index 430b368..5eed597 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,9 +605,7 @@ 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(delayed_refs, > - qrecord); > -if (qexisting) > +if(_btrfs_qgroup_insert_dirty_extent(delayed_refs, qrecord)) > kfree(qrecord); > } > > diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c > index 9fcb8c9..47c85ff 100644 > --- a/fs/btrfs/extent-tree.c > +++ b/fs/btrfs/extent-tree.c > @@ -8519,34 +8519,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(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) > @@ -8580,7 +8552,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_record_dirty_extent(trans, root->fs_info, > +bytenr, num_bytes, GFP_NOFS); > if (ret) > return ret; > } > @@ -8729,8 +8702,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, > -
Re: [PATCH v2 1/3] btrfs: qgroup: Refactor btrfs_qgroup_insert_dirty_extent()
At 08/08/2016 10:54 AM, Goldwyn Rodrigues wrote: On 08/07/2016 07:39 PM, Qu Wenruo wrote: At 08/07/2016 01:19 AM, Goldwyn Rodrigues wrote: Thanks Qu, This patch set fixes all the reported problems. However, I have some minor issues with coding style. Thanks for the test. On 08/04/2016 09:29 PM, 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 Signed-off-by: Qu Wenruo --- fs/btrfs/delayed-ref.c | 5 + fs/btrfs/extent-tree.c | 36 +--- fs/btrfs/qgroup.c | 39 ++- fs/btrfs/qgroup.h | 44 +--- 4 files changed, 81 insertions(+), 43 deletions(-) diff --git a/fs/btrfs/delayed-ref.c b/fs/btrfs/delayed-ref.c index 430b368..5eed597 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,9 +605,7 @@ 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(delayed_refs, - qrecord); -if (qexisting) +if(_btrfs_qgroup_insert_dirty_extent(delayed_refs, qrecord)) kfree(qrecord); } diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c index 9fcb8c9..47c85ff 100644 --- a/fs/btrfs/extent-tree.c +++ b/fs/btrfs/extent-tree.c @@ -8519,34 +8519,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(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) @@ -8580,7 +8552,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_record_dirty_extent(trans, root->fs_info, +bytenr, num_bytes, GFP_NOFS); if (ret) return ret; } @@ -8729,8 +8702,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_record_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 9d4c05b..76d4f67 100644 --- a/fs/btrfs/qgroup.c +++ b/fs/btrfs/qgroup.c @@ -1453,9 +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_delayed_ref_root *delayed_refs, - struct btrfs_qgroup_extent_record *record) +int _btrfs_qgroup_insert_dirty_extent( +struct btrfs_delayed_ref_root *delayed_refs, +struct btrfs_qgrou
Re: [PATCH v2 1/3] btrfs: qgroup: Refactor btrfs_qgroup_insert_dirty_extent()
On 08/07/2016 07:39 PM, Qu Wenruo wrote: > > > At 08/07/2016 01:19 AM, Goldwyn Rodrigues wrote: >> Thanks Qu, >> >> This patch set fixes all the reported problems. However, I have some >> minor issues with coding style. >> > > Thanks for the test. > >> >> On 08/04/2016 09:29 PM, 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 >>> Signed-off-by: Qu Wenruo >>> --- >>> fs/btrfs/delayed-ref.c | 5 + >>> fs/btrfs/extent-tree.c | 36 +--- >>> fs/btrfs/qgroup.c | 39 ++- >>> fs/btrfs/qgroup.h | 44 >>> +--- >>> 4 files changed, 81 insertions(+), 43 deletions(-) >>> >>> diff --git a/fs/btrfs/delayed-ref.c b/fs/btrfs/delayed-ref.c >>> index 430b368..5eed597 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,9 +605,7 @@ 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(delayed_refs, >>> - qrecord); >>> -if (qexisting) >>> +if(_btrfs_qgroup_insert_dirty_extent(delayed_refs, qrecord)) >>> kfree(qrecord); >>> } >>> >>> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c >>> index 9fcb8c9..47c85ff 100644 >>> --- a/fs/btrfs/extent-tree.c >>> +++ b/fs/btrfs/extent-tree.c >>> @@ -8519,34 +8519,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(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) >>> @@ -8580,7 +8552,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_record_dirty_extent(trans, root->fs_info, >>> +bytenr, num_bytes, GFP_NOFS); >>> if (ret) >>> return ret; >>> } >>> @@ -8729,8 +8702,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_record_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 9d4c05b..76d4f67 100644 >>> --- a/fs/btrfs/qgroup.
Re: [PATCH v2 1/3] btrfs: qgroup: Refactor btrfs_qgroup_insert_dirty_extent()
At 08/07/2016 01:19 AM, Goldwyn Rodrigues wrote: Thanks Qu, This patch set fixes all the reported problems. However, I have some minor issues with coding style. Thanks for the test. On 08/04/2016 09:29 PM, 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 Signed-off-by: Qu Wenruo --- fs/btrfs/delayed-ref.c | 5 + fs/btrfs/extent-tree.c | 36 +--- fs/btrfs/qgroup.c | 39 ++- fs/btrfs/qgroup.h | 44 +--- 4 files changed, 81 insertions(+), 43 deletions(-) diff --git a/fs/btrfs/delayed-ref.c b/fs/btrfs/delayed-ref.c index 430b368..5eed597 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,9 +605,7 @@ 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(delayed_refs, -qrecord); - if (qexisting) + if(_btrfs_qgroup_insert_dirty_extent(delayed_refs, qrecord)) kfree(qrecord); } diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c index 9fcb8c9..47c85ff 100644 --- a/fs/btrfs/extent-tree.c +++ b/fs/btrfs/extent-tree.c @@ -8519,34 +8519,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(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) @@ -8580,7 +8552,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_record_dirty_extent(trans, root->fs_info, + bytenr, num_bytes, GFP_NOFS); if (ret) return ret; } @@ -8729,8 +8702,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_record_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 9d4c05b..76d4f67 100644 --- a/fs/btrfs/qgroup.c +++ b/fs/btrfs/qgroup.c @@ -1453,9 +1453,9 @@ int btrfs_qgroup_prepare_account_extents(struct btrfs_trans_handle *trans, return
Re: [PATCH v2 1/3] btrfs: qgroup: Refactor btrfs_qgroup_insert_dirty_extent()
Thanks Qu, This patch set fixes all the reported problems. However, I have some minor issues with coding style. On 08/04/2016 09:29 PM, 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 > Signed-off-by: Qu Wenruo > --- > fs/btrfs/delayed-ref.c | 5 + > fs/btrfs/extent-tree.c | 36 +--- > fs/btrfs/qgroup.c | 39 ++- > fs/btrfs/qgroup.h | 44 +--- > 4 files changed, 81 insertions(+), 43 deletions(-) > > diff --git a/fs/btrfs/delayed-ref.c b/fs/btrfs/delayed-ref.c > index 430b368..5eed597 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,9 +605,7 @@ 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(delayed_refs, > - qrecord); > - if (qexisting) > + if(_btrfs_qgroup_insert_dirty_extent(delayed_refs, qrecord)) > kfree(qrecord); > } > > diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c > index 9fcb8c9..47c85ff 100644 > --- a/fs/btrfs/extent-tree.c > +++ b/fs/btrfs/extent-tree.c > @@ -8519,34 +8519,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(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) > @@ -8580,7 +8552,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_record_dirty_extent(trans, root->fs_info, > + bytenr, num_bytes, GFP_NOFS); > if (ret) > return ret; > } > @@ -8729,8 +8702,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_record_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 9d4c05b..76d4f67 100644 > --- a/fs/btrfs/qgroup.c > +++ b/fs/btrfs/qgroup.c > @@ -1453,9 +1453,9 @@ int btrfs_qgroup_p
Re: [PATCH v2 1/3] btrfs: qgroup: Refactor btrfs_qgroup_insert_dirty_extent()
Thanks Qu, This patch set fixes all the reported problems. However, I have some minor issues with coding style. On 08/04/2016 09:29 PM, 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 > Signed-off-by: Qu Wenruo > --- > fs/btrfs/delayed-ref.c | 5 + > fs/btrfs/extent-tree.c | 36 +--- > fs/btrfs/qgroup.c | 39 ++- > fs/btrfs/qgroup.h | 44 +--- > 4 files changed, 81 insertions(+), 43 deletions(-) > > diff --git a/fs/btrfs/delayed-ref.c b/fs/btrfs/delayed-ref.c > index 430b368..5eed597 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,9 +605,7 @@ 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(delayed_refs, > - qrecord); > - if (qexisting) > + if(_btrfs_qgroup_insert_dirty_extent(delayed_refs, qrecord)) > kfree(qrecord); > } > > diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c > index 9fcb8c9..47c85ff 100644 > --- a/fs/btrfs/extent-tree.c > +++ b/fs/btrfs/extent-tree.c > @@ -8519,34 +8519,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(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) > @@ -8580,7 +8552,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_record_dirty_extent(trans, root->fs_info, > + bytenr, num_bytes, GFP_NOFS); > if (ret) > return ret; > } > @@ -8729,8 +8702,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_record_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 9d4c05b..76d4f67 100644 > --- a/fs/btrfs/qgroup.c > +++ b/fs/btrfs/qgroup.c > @@ -1453,9 +1453,9 @@ int btrfs_qgroup_p