| Message ID | 20181109104308.13123-1-fdmanana@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Btrfs: simpler and more efficient cleanup of a log tree's extent io tree | expand |
On 9.11.18 г. 12:43 ч., fdmanana@kernel.org wrote: > From: Filipe Manana <fdmanana@suse.com> > > We currently are in a loop finding each range (corresponding to a btree > node/leaf) in a log root's extent io tree and then clean it up. This is a > waste of time since we are traversing the extent io tree's rb_tree more > times then needed (one for a range lookup and another for cleaning it up) > without any good reason. > > We free the log trees when we are in the critical section of a transaction > commit (the transaction state is set to TRANS_STATE_COMMIT_DOING), so it's > of great convenience to do everything as fast as possible in order to > reduce the time we block other tasks from starting a new transaction. > > So fix this by traversing the extent io tree once and cleaning up all its > records in one go while traversing it. > > Signed-off-by: Filipe Manana <fdmanana@suse.com> Reviewed-by: Nikolay Borisov <nborisov@suse.com> > --- > fs/btrfs/tree-log.c | 16 ++-------------- > 1 file changed, 2 insertions(+), 14 deletions(-) > > diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c > index d49edd25f2e5..aac3749f697f 100644 > --- a/fs/btrfs/tree-log.c > +++ b/fs/btrfs/tree-log.c > @@ -3204,8 +3204,6 @@ static void free_log_tree(struct btrfs_trans_handle *trans, > struct btrfs_root *log) > { > int ret; > - u64 start; > - u64 end; > struct walk_control wc = { > .free = 1, > .process_func = process_one_buffer > @@ -3216,18 +3214,8 @@ static void free_log_tree(struct btrfs_trans_handle *trans, > if (ret) > btrfs_abort_transaction(trans, ret); > > - while (1) { > - ret = find_first_extent_bit(&log->dirty_log_pages, > - 0, &start, &end, > - EXTENT_DIRTY | EXTENT_NEW | EXTENT_NEED_WAIT, > - NULL); > - if (ret) > - break; > - > - clear_extent_bits(&log->dirty_log_pages, start, end, > - EXTENT_DIRTY | EXTENT_NEW | EXTENT_NEED_WAIT); > - } > - > + clear_extent_bits(&log->dirty_log_pages, 0, (u64)-1, > + EXTENT_DIRTY | EXTENT_NEW | EXTENT_NEED_WAIT); > free_extent_buffer(log->node); > kfree(log); > } >
On Fri, Nov 09, 2018 at 10:43:08AM +0000, fdmanana@kernel.org wrote: > From: Filipe Manana <fdmanana@suse.com> > > We currently are in a loop finding each range (corresponding to a btree > node/leaf) in a log root's extent io tree and then clean it up. This is a > waste of time since we are traversing the extent io tree's rb_tree more > times then needed (one for a range lookup and another for cleaning it up) > without any good reason. > > We free the log trees when we are in the critical section of a transaction > commit (the transaction state is set to TRANS_STATE_COMMIT_DOING), so it's > of great convenience to do everything as fast as possible in order to > reduce the time we block other tasks from starting a new transaction. > > So fix this by traversing the extent io tree once and cleaning up all its > records in one go while traversing it. > > Signed-off-by: Filipe Manana <fdmanana@suse.com> Sheesh Reviewed-by: Josef Bacik <josef@toxicpanda.com> Thanks, Josef
On Fri, Nov 09, 2018 at 10:43:08AM +0000, fdmanana@kernel.org wrote: > From: Filipe Manana <fdmanana@suse.com> > > We currently are in a loop finding each range (corresponding to a btree > node/leaf) in a log root's extent io tree and then clean it up. This is a > waste of time since we are traversing the extent io tree's rb_tree more > times then needed (one for a range lookup and another for cleaning it up) > without any good reason. > > We free the log trees when we are in the critical section of a transaction > commit (the transaction state is set to TRANS_STATE_COMMIT_DOING), so it's > of great convenience to do everything as fast as possible in order to > reduce the time we block other tasks from starting a new transaction. > > So fix this by traversing the extent io tree once and cleaning up all its > records in one go while traversing it. > > Signed-off-by: Filipe Manana <fdmanana@suse.com> Added to misc-next, thanks.
diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c index d49edd25f2e5..aac3749f697f 100644 --- a/fs/btrfs/tree-log.c +++ b/fs/btrfs/tree-log.c @@ -3204,8 +3204,6 @@ static void free_log_tree(struct btrfs_trans_handle *trans, struct btrfs_root *log) { int ret; - u64 start; - u64 end; struct walk_control wc = { .free = 1, .process_func = process_one_buffer @@ -3216,18 +3214,8 @@ static void free_log_tree(struct btrfs_trans_handle *trans, if (ret) btrfs_abort_transaction(trans, ret); - while (1) { - ret = find_first_extent_bit(&log->dirty_log_pages, - 0, &start, &end, - EXTENT_DIRTY | EXTENT_NEW | EXTENT_NEED_WAIT, - NULL); - if (ret) - break; - - clear_extent_bits(&log->dirty_log_pages, start, end, - EXTENT_DIRTY | EXTENT_NEW | EXTENT_NEED_WAIT); - } - + clear_extent_bits(&log->dirty_log_pages, 0, (u64)-1, + EXTENT_DIRTY | EXTENT_NEW | EXTENT_NEED_WAIT); free_extent_buffer(log->node); kfree(log); }