| Message ID | 20181203152038.21388-7-josef@toxicpanda.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Delayed refs rsv | expand |
On 3.12.18 г. 17:20 ч., Josef Bacik wrote: > Any space used in the delayed_refs_rsv will be freed up by a transaction > commit, so instead of just counting the pinned space we also need to > account for any space in the delayed_refs_rsv when deciding if it will > make a different to commit the transaction to satisfy our space > reservation. If we have enough bytes to satisfy our reservation ticket > then we are good to go, otherwise subtract out what space we would gain > back by committing the transaction and compare that against the pinned > space to make our decision. > > Signed-off-by: Josef Bacik <josef@toxicpanda.com> Reviewed-by: Nikolay Borisov <nborisov@suse.com> However, look below for one suggestion: > --- > fs/btrfs/extent-tree.c | 24 +++++++++++++++--------- > 1 file changed, 15 insertions(+), 9 deletions(-) > > diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c > index aa0a638d0263..63ff9d832867 100644 > --- a/fs/btrfs/extent-tree.c > +++ b/fs/btrfs/extent-tree.c > @@ -4843,8 +4843,10 @@ static int may_commit_transaction(struct btrfs_fs_info *fs_info, > { > struct reserve_ticket *ticket = NULL; > struct btrfs_block_rsv *delayed_rsv = &fs_info->delayed_block_rsv; > + struct btrfs_block_rsv *delayed_refs_rsv = &fs_info->delayed_refs_rsv; > struct btrfs_trans_handle *trans; > - u64 bytes; > + u64 bytes_needed; > + u64 reclaim_bytes = 0; > > trans = (struct btrfs_trans_handle *)current->journal_info; > if (trans) > @@ -4857,15 +4859,15 @@ static int may_commit_transaction(struct btrfs_fs_info *fs_info, > else if (!list_empty(&space_info->tickets)) > ticket = list_first_entry(&space_info->tickets, > struct reserve_ticket, list); > - bytes = (ticket) ? ticket->bytes : 0; > + bytes_needed = (ticket) ? ticket->bytes : 0; > spin_unlock(&space_info->lock); > > - if (!bytes) > + if (!bytes_needed) > return 0; > > /* See if there is enough pinned space to make this reservation */ > if (__percpu_counter_compare(&space_info->total_bytes_pinned, > - bytes, > + bytes_needed, > BTRFS_TOTAL_BYTES_PINNED_BATCH) >= 0) > goto commit; > > @@ -4877,14 +4879,18 @@ static int may_commit_transaction(struct btrfs_fs_info *fs_info, > return -ENOSPC; If we remove this : if (space_info != delayed_rsv->space_info) return -ENOSPC; Check, can't we move the reclaim_bytes calc code above the __percpu_counter_compare and eventually be left with just a single invocation to percpu_compare. The diff should looke something along the lines of: @@ -4828,19 +4827,6 @@ static int may_commit_transaction(struct btrfs_fs_info *fs_info, if (!bytes) return 0; - /* See if there is enough pinned space to make this reservation */ - if (__percpu_counter_compare(&space_info->total_bytes_pinned, - bytes, - BTRFS_TOTAL_BYTES_PINNED_BATCH) >= 0) - goto commit; - - /* - * See if there is some space in the delayed insertion reservation for - * this reservation. - */ - if (space_info != delayed_rsv->space_info) - return -ENOSPC; - spin_lock(&delayed_rsv->lock); if (delayed_rsv->size > bytes) bytes = 0; @@ -4850,9 +4836,8 @@ static int may_commit_transaction(struct btrfs_fs_info *fs_info, if (__percpu_counter_compare(&space_info->total_bytes_pinned, bytes, - BTRFS_TOTAL_BYTES_PINNED_BATCH) < 0) { + BTRFS_TOTAL_BYTES_PINNED_BATCH) < 0) return -ENOSPC; - } commit: trans = btrfs_join_transaction(fs_info->extent_root); > > spin_lock(&delayed_rsv->lock); > - if (delayed_rsv->size > bytes) > - bytes = 0; > - else > - bytes -= delayed_rsv->size; > + reclaim_bytes += delayed_rsv->reserved; > spin_unlock(&delayed_rsv->lock); > > + spin_lock(&delayed_refs_rsv->lock); > + reclaim_bytes += delayed_refs_rsv->reserved; > + spin_unlock(&delayed_refs_rsv->lock); > + if (reclaim_bytes >= bytes_needed) > + goto commit; > + bytes_needed -= reclaim_bytes; > + > if (__percpu_counter_compare(&space_info->total_bytes_pinned, > - bytes, > + bytes_needed, > BTRFS_TOTAL_BYTES_PINNED_BATCH) < 0) { > return -ENOSPC; > } >
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c index aa0a638d0263..63ff9d832867 100644 --- a/fs/btrfs/extent-tree.c +++ b/fs/btrfs/extent-tree.c @@ -4843,8 +4843,10 @@ static int may_commit_transaction(struct btrfs_fs_info *fs_info, { struct reserve_ticket *ticket = NULL; struct btrfs_block_rsv *delayed_rsv = &fs_info->delayed_block_rsv; + struct btrfs_block_rsv *delayed_refs_rsv = &fs_info->delayed_refs_rsv; struct btrfs_trans_handle *trans; - u64 bytes; + u64 bytes_needed; + u64 reclaim_bytes = 0; trans = (struct btrfs_trans_handle *)current->journal_info; if (trans) @@ -4857,15 +4859,15 @@ static int may_commit_transaction(struct btrfs_fs_info *fs_info, else if (!list_empty(&space_info->tickets)) ticket = list_first_entry(&space_info->tickets, struct reserve_ticket, list); - bytes = (ticket) ? ticket->bytes : 0; + bytes_needed = (ticket) ? ticket->bytes : 0; spin_unlock(&space_info->lock); - if (!bytes) + if (!bytes_needed) return 0; /* See if there is enough pinned space to make this reservation */ if (__percpu_counter_compare(&space_info->total_bytes_pinned, - bytes, + bytes_needed, BTRFS_TOTAL_BYTES_PINNED_BATCH) >= 0) goto commit; @@ -4877,14 +4879,18 @@ static int may_commit_transaction(struct btrfs_fs_info *fs_info, return -ENOSPC; spin_lock(&delayed_rsv->lock); - if (delayed_rsv->size > bytes) - bytes = 0; - else - bytes -= delayed_rsv->size; + reclaim_bytes += delayed_rsv->reserved; spin_unlock(&delayed_rsv->lock); + spin_lock(&delayed_refs_rsv->lock); + reclaim_bytes += delayed_refs_rsv->reserved; + spin_unlock(&delayed_refs_rsv->lock); + if (reclaim_bytes >= bytes_needed) + goto commit; + bytes_needed -= reclaim_bytes; + if (__percpu_counter_compare(&space_info->total_bytes_pinned, - bytes, + bytes_needed, BTRFS_TOTAL_BYTES_PINNED_BATCH) < 0) { return -ENOSPC; }
Any space used in the delayed_refs_rsv will be freed up by a transaction commit, so instead of just counting the pinned space we also need to account for any space in the delayed_refs_rsv when deciding if it will make a different to commit the transaction to satisfy our space reservation. If we have enough bytes to satisfy our reservation ticket then we are good to go, otherwise subtract out what space we would gain back by committing the transaction and compare that against the pinned space to make our decision. Signed-off-by: Josef Bacik <josef@toxicpanda.com> --- fs/btrfs/extent-tree.c | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-)