Re: [PATCH 06/35] btrfs: check if free bgs for commit

Nikolay Borisov
2018-09-03T17:39:37Z

On 3.09.2018 12:06, Nikolay Borisov wrote:
>
>
> On 30.08.2018 20:41, Josef Bacik wrote:
>> may_commit_transaction will skip committing the transaction if we don't
>> have enough pinned space or if we're trying to find space for a SYSTEM
>> chunk. However if we have pending free block groups in this transaction
>> we still want to commit as we may be able to allocate a chunk to make
>> our reservation. So instead of just returning ENOSPC, check if we have
>> free block groups pending, and if so commit the transaction to allow us
>> to use that free space.
>>
>> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
>> ---
>> fs/btrfs/extent-tree.c | 18 ++++++++++++------
>> 1 file changed, 12 insertions(+), 6 deletions(-)
>>
>> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
>> index 6e7f350754d2..80615a579b18 100644
>> --- a/fs/btrfs/extent-tree.c
>> +++ b/fs/btrfs/extent-tree.c
>> @@ -4804,6 +4804,7 @@ static int may_commit_transaction(struct btrfs_fs_info *fs_info,
>> struct btrfs_trans_handle *trans;
>> u64 bytes;
>> u64 reclaim_bytes = 0;
>> + bool do_commit = true;
>>
>> trans = (struct btrfs_trans_handle *)current->journal_info;
>> if (trans)
>
> While you are at it, does this check even make sense, since the
> transaction handle is acquired proper later I think this can be removed?
>
>> @@ -4832,8 +4833,10 @@ static int may_commit_transaction(struct btrfs_fs_info *fs_info,
>> * See if there is some space in the delayed insertion reservation for
>> * this reservation.
>> */
>> - if (space_info != delayed_rsv->space_info)
>> - return -ENOSPC;
>> + if (space_info != delayed_rsv->space_info) {
>> + do_commit = false;
>> + goto commit;
>> + }
>>
>> spin_lock(&delayed_rsv->lock);
>> reclaim_bytes += delayed_rsv->reserved;
>> @@ -4848,15 +4851,18 @@ 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) {
>> - return -ENOSPC;
>> - }
>> -
>> + BTRFS_TOTAL_BYTES_PINNED_BATCH) < 0)
>> + do_commit = false;
>> commit:
>> trans = btrfs_join_transaction(fs_info->extent_root);
>> if (IS_ERR(trans))
>> return -ENOSPC;
>>
>> + if (!do_commit &&> + !test_bit(BTRFS_TRANS_HAVE_FREE_BGS, &trans->transaction->flags)) {

offtopic: And yet on a more different note, do we really need the
BTRFS_TRANS_HAVE_FREE_BFS flag when we can just check
fs_info->free_chunk_space ?

>> + btrfs_end_transaction(trans);
>> + return -ENOSPC;
>> + }
>> return btrfs_commit_transaction(trans);
>> }
>>
>>
>