On 22.08.2017 23:00, josef@xxxxxxxxxxxxxx wrote:
> From: Josef Bacik <jbacik@xxxxxx>
>
> Nikolay reported that generic/273 was failing currently with ENOSPC.
> Turns out this is because we get to the point where the outstanding
> reservations are greater than the pinned space on the fs. This is a
> mistake, previously we used the current reservation amount in
> may_commit_transaction, not the entire outstanding reservation amount.
> Fix this to find the minimum byte size needed to make progress in
> flushing, and pass that into may_commit_transaction. From there we can
> make a smarter decision on whether to commit the transaction or not.
> This fixes the failure in generic/273.
>
> Reported-by: Nikolay Borisov <nborisov@xxxxxxxx>
> Signed-off-by: Josef Bacik <jbacik@xxxxxx>
Reviewed-and-tested-by: Nikolay Borisov <nborisov@xxxxxxxx>
> ---
> v1->v2:
> - check the ticket bytes in may_commit_transaction instead of copying bytes
> around.
> - clean up may_commit_transaction to remove unused arguments
>
> fs/btrfs/extent-tree.c | 42 ++++++++++++++++++++++++++++--------------
> 1 file changed, 28 insertions(+), 14 deletions(-)
>
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index a5d59dd..1464678 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -4836,6 +4836,13 @@ static void shrink_delalloc(struct btrfs_fs_info *fs_info, u64 to_reclaim,
> }
> }
>
> +struct reserve_ticket {
> + u64 bytes;
> + int error;
> + struct list_head list;
> + wait_queue_head_t wait;
> +};
> +
> /**
> * maybe_commit_transaction - possibly commit the transaction if its ok to
> * @root - the root we're allocating for
> @@ -4847,18 +4854,29 @@ static void shrink_delalloc(struct btrfs_fs_info *fs_info, u64 to_reclaim,
> * will return -ENOSPC.
> */
> static int may_commit_transaction(struct btrfs_fs_info *fs_info,
> - struct btrfs_space_info *space_info,
> - u64 bytes, int force)
> + struct btrfs_space_info *space_info)
> {
> + struct reserve_ticket *ticket = NULL;
> struct btrfs_block_rsv *delayed_rsv = &fs_info->delayed_block_rsv;
> struct btrfs_trans_handle *trans;
> + u64 bytes;
>
> trans = (struct btrfs_trans_handle *)current->journal_info;
> if (trans)
> return -EAGAIN;
>
> - if (force)
> - goto commit;
> + spin_lock(&space_info->lock);
> + 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))
> + ticket = list_first_entry(&space_info->tickets,
> + struct reserve_ticket, list);
> + bytes = (ticket) ? ticket->bytes : 0;
> + spin_unlock(&space_info->lock);
> +
> + if (!bytes)
> + return 0;
>
> /* See if there is enough pinned space to make this reservation */
> if (percpu_counter_compare(&space_info->total_bytes_pinned,
> @@ -4873,8 +4891,12 @@ 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;
> if (percpu_counter_compare(&space_info->total_bytes_pinned,
> - bytes - delayed_rsv->size) < 0) {
> + bytes) < 0) {
> spin_unlock(&delayed_rsv->lock);
> return -ENOSPC;
> }
> @@ -4888,13 +4910,6 @@ static int may_commit_transaction(struct btrfs_fs_info *fs_info,
> return btrfs_commit_transaction(trans);
> }
>
> -struct reserve_ticket {
> - u64 bytes;
> - int error;
> - struct list_head list;
> - wait_queue_head_t wait;
> -};
> -
> /*
> * Try to flush some data based on policy set by @state. This is only advisory
> * and may fail for various reasons. The caller is supposed to examine the
> @@ -4944,8 +4959,7 @@ static void flush_space(struct btrfs_fs_info *fs_info,
> ret = 0;
> break;
> case COMMIT_TRANS:
> - ret = may_commit_transaction(fs_info, space_info,
> - num_bytes, 0);
> + ret = may_commit_transaction(fs_info, space_info);
> break;
> default:
> ret = -ENOSPC;
>
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
