| Message ID | 20200131223613.490779-23-josef@toxicpanda.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Convert data reservations to the ticketing infrastructure | expand |
On 1.02.20 г. 0:36 ч., Josef Bacik wrote: > We can end up with free'd extents in the delayed refs, and thus > may_commit_transaction() may not think we have enough pinned space to > commit the transaction and we'll ENOSPC early. Handle this by running > the delayed refs in order to make sure pinned is uptodate before we try > to commit the transaction. > > Signed-off-by: Josef Bacik <josef@toxicpanda.com> Actually wouldn't it be better if the stages are structured: FLUSH_DELALLOC which potentially creates a bunch of delayed refs, FLUSH_DELAYED_REFS which in turn could free some reservations. Then IPUT to process anything from delayed refs running and finally committing transaction to reclaim pinned space? Codewise this is ok. Reviewed-by: Nikolay Borisov <nborisov@suse.com> > --- > fs/btrfs/space-info.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c > index c86fad4174f1..5b0dc1046daa 100644 > --- a/fs/btrfs/space-info.c > +++ b/fs/btrfs/space-info.c > @@ -805,6 +805,7 @@ static const enum btrfs_flush_state evict_flush_states[] = { > static const enum btrfs_flush_state data_flush_states[] = { > FLUSH_DELALLOC_WAIT, > RUN_DELAYED_IPUTS, > + FLUSH_DELAYED_REFS, > COMMIT_TRANS, > }; > >
On 2/3/20 11:16 AM, Nikolay Borisov wrote: > > > On 1.02.20 г. 0:36 ч., Josef Bacik wrote: >> We can end up with free'd extents in the delayed refs, and thus >> may_commit_transaction() may not think we have enough pinned space to >> commit the transaction and we'll ENOSPC early. Handle this by running >> the delayed refs in order to make sure pinned is uptodate before we try >> to commit the transaction. >> >> Signed-off-by: Josef Bacik <josef@toxicpanda.com> > > Actually wouldn't it be better if the stages are structured: > > FLUSH_DELALLOC which potentially creates a bunch of delayed refs, > FLUSH_DELAYED_REFS which in turn could free some reservations. Then IPUT > to process anything from delayed refs running and finally committing > transaction to reclaim pinned space? > It's more about doing the least amount of work for the largest chance of success. Flushing delalloc will reclaim space in one of two ways, first the compression case where we allocate less disk space than we had reserved, second the free'ing of extents we drop when overwriting old space. In order to get that free'd space we'd have to wait on delayed refs to make sure pinned was uptodate before committing the transaction. Running delayed iputs only reclaims space in the form of delayed refs flushing and then committing the transaction to free that pinned area. So at this point we have to run delayed refs. Running delayed refs isn't free, so I'd rather just coalesce these two operations together and then run delayed refs so we're completely sure we need to commit the transaction if we get to that point. Thanks, Josef
diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c index c86fad4174f1..5b0dc1046daa 100644 --- a/fs/btrfs/space-info.c +++ b/fs/btrfs/space-info.c @@ -805,6 +805,7 @@ static const enum btrfs_flush_state evict_flush_states[] = { static const enum btrfs_flush_state data_flush_states[] = { FLUSH_DELALLOC_WAIT, RUN_DELAYED_IPUTS, + FLUSH_DELAYED_REFS, COMMIT_TRANS, };
We can end up with free'd extents in the delayed refs, and thus may_commit_transaction() may not think we have enough pinned space to commit the transaction and we'll ENOSPC early. Handle this by running the delayed refs in order to make sure pinned is uptodate before we try to commit the transaction. Signed-off-by: Josef Bacik <josef@toxicpanda.com> --- fs/btrfs/space-info.c | 1 + 1 file changed, 1 insertion(+)