| Message ID | 20200313211220.148772-5-josef@toxicpanda.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fix up some stupid delayed ref flushing behaviors | expand |
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@toxicpanda.com> > --- > 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: >
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:
We love running delayed refs in commit_cowonly_roots, but it is a bit 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, 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@toxicpanda.com> --- fs/btrfs/transaction.c | 12 ------------ 1 file changed, 12 deletions(-)