Re: [PATCH 3/5] btrfs: only take normal tickets into account in may_commit_transaction

[Nikolay Borisov <nborisov@suse.com>]

On 9.03.20 г. 22:23 ч., Josef Bacik wrote:
> In debugging a generic/320 failure on ppc64, Nikolay noticed that
> sometimes we'd ENOSPC out with plenty of space to reclaim if we had
> committed the transaction.  He further discovered that this was because
> there was a priority ticket that was small enough to fit in the free
> space currently in the space_info.  While that is a problem by itself,
> it exposed another flaw, that we consider priority tickets in
> may_commit_transaction.
> 
> Priority tickets are not allowed to commit the transaction, thus we
> shouldn't even consider them in may_commit_transaction.  Instead we need
> to only consider current normal tickets.  With this fix in place, we
> will properly commit the transaction.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> ---
>  fs/btrfs/space-info.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c
> index 8d00a9ee9458..d198cfd45cf7 100644
> --- a/fs/btrfs/space-info.c
> +++ b/fs/btrfs/space-info.c
> @@ -592,10 +592,7 @@ static int may_commit_transaction(struct btrfs_fs_info *fs_info,
>  	else
>  		cur_free_bytes = 0;
>  
> -	if (!list_empty(&space_info->priority_tickets))
> -		ticket = list_first_entry(&space_info->priority_tickets,
> -					  struct reserve_ticket, list);
> -	else if (!list_empty(&space_info->tickets))
> +	if (!list_empty(&space_info->tickets))
>  		ticket = list_first_entry(&space_info->tickets,
>  					  struct reserve_ticket, list);
>  	bytes_needed = (ticket) ? ticket->bytes : 0;
> 


Thinking about this a bit more, if we have a ticket here that we have
enough free space to satisfy we just return, where is this ticket
supposed to be granted? Can we get into the same situation we had with
the priority tickets? I guess what I"m asking is "why don't we call try
granting ticket" in this case?