On 13.03.20 г. 23:12 ч., Josef Bacik wrote:
> We love running delayed refs in commit_cowonly_roots, but it is a bit
You refer to commit_cowonly_roots but in fact are changing
create_pending_snapshot. Between calling
create_pending_snapshots->create_pending_snapshot and
commit_cowonly_roots we have 2 calls to btrfs_run_delayed_refs. My point
is by the time commit_cowonly_roots is called we've already long flushed
the delayed refs generated from create_pending_snapshot. IMO referring
to commit_cowonly_roots in this commit is wrong?
> excessive. I was seeing cases of running 3 or 4 refs a few times in a
> row during this time. Instead simply update all of the roots first,
By "simply update all of the roots" I assume you meant the call to
commit_fs_roots or the changes happening to the snapshoted roots in
create_pending_snapshot? If it's the latter I'd rather you made the text
more explicit by referring to the fact the refs are generated from
snapshots e.g.
Instead simply run all snapshot-related work, then drain delayed refs
resulting from this ...
> then run delayed refs, then handle the empty block groups case, and then
> if we have any more dirty roots do the whole thing again. This allows
> us to be much more efficient with our delayed ref running, as we can
> batch a few more operations at once.
>
> Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
> ---
> fs/btrfs/transaction.c | 12 ------------
> 1 file changed, 12 deletions(-)
>
> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
> index 3e7fd8a934c1..c3b3b524b8c3 100644
> --- a/fs/btrfs/transaction.c
> +++ b/fs/btrfs/transaction.c
> @@ -1646,12 +1646,6 @@ static noinline int create_pending_snapshot(struct btrfs_trans_handle *trans,
> goto fail;
> }
>
> - ret = btrfs_run_delayed_refs(trans, (unsigned long)-1);
> - if (ret) {
> - btrfs_abort_transaction(trans, ret);
> - goto fail;
> - }
> -
> /*
> * Do special qgroup accounting for snapshot, as we do some qgroup
> * snapshot hack to do fast snapshot.
> @@ -1698,12 +1692,6 @@ static noinline int create_pending_snapshot(struct btrfs_trans_handle *trans,
> }
> }
>
> - ret = btrfs_run_delayed_refs(trans, (unsigned long)-1);
> - if (ret) {
> - btrfs_abort_transaction(trans, ret);
> - goto fail;
> - }
> -
> fail:
> pending->error = ret;
> dir_item_existed:
>
