On 7.04.20 г. 13:38 ч., fdmanana@xxxxxxxxxx wrote:
> From: Filipe Manana <fdmanana@xxxxxxxx>
>
> Whenever we add a ticket to a space_info object we increment the object's
> reclaim_size counter witht the ticket's bytes, and we decrement it with
> the corresponding amount only when we are able to grant the requested
> space to the ticket. When we are not able to grant the space to a ticket,
> or when the ticket is removed due to a signal (e.g. an application has
> received sigterm from the terminal) we never decrement the counter with
> the corresponding bytes from the ticket. This leak can result in the
> space reclaim code to later do much more work than necessary. So fix it
> by decrementing the counter when those two cases happen as well.
>
> Fixes: db161806dc5615 ("btrfs: account ticket size at add/delete time")
> Signed-off-by: Filipe Manana <fdmanana@xxxxxxxx>
Doh, you are correct. I especially like it you've actually factored
ticket removal into a function.
Reviewed-by: Nikolay Borisov <nborisov@xxxxxxxx>
> ---
> fs/btrfs/block-group.c | 1 +
> fs/btrfs/space-info.c | 20 ++++++++++++++------
> 2 files changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
> index 786849fcc319..47f66c6a7d7f 100644
> --- a/fs/btrfs/block-group.c
> +++ b/fs/btrfs/block-group.c
> @@ -3370,6 +3370,7 @@ int btrfs_free_block_groups(struct btrfs_fs_info *info)
> space_info->bytes_reserved > 0 ||
> space_info->bytes_may_use > 0))
> btrfs_dump_space_info(info, space_info, 0, 0);
> + WARN_ON(space_info->reclaim_size > 0);
> list_del(&space_info->list);
> btrfs_sysfs_remove_space_info(space_info);
> }
> diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c
> index 8b0fe053a25d..ff17a4420358 100644
> --- a/fs/btrfs/space-info.c
> +++ b/fs/btrfs/space-info.c
> @@ -361,6 +361,16 @@ int btrfs_can_overcommit(struct btrfs_fs_info *fs_info,
> return 0;
> }
>
> +static void remove_ticket(struct btrfs_space_info *space_info,
> + struct reserve_ticket *ticket)
> +{
> + if (!list_empty(&ticket->list)) {
> + list_del_init(&ticket->list);
> + ASSERT(space_info->reclaim_size >= ticket->bytes);
> + space_info->reclaim_size -= ticket->bytes;
> + }
> +}
> +
> /*
> * This is for space we already have accounted in space_info->bytes_may_use, so
> * basically when we're returning space from block_rsv's.
> @@ -388,9 +398,7 @@ void btrfs_try_granting_tickets(struct btrfs_fs_info *fs_info,
> btrfs_space_info_update_bytes_may_use(fs_info,
> space_info,
> ticket->bytes);
> - list_del_init(&ticket->list);
> - ASSERT(space_info->reclaim_size >= ticket->bytes);
> - space_info->reclaim_size -= ticket->bytes;
> + remove_ticket(space_info, ticket);
> ticket->bytes = 0;
> space_info->tickets_id++;
> wake_up(&ticket->wait);
> @@ -899,7 +907,7 @@ static bool maybe_fail_all_tickets(struct btrfs_fs_info *fs_info,
> btrfs_info(fs_info, "failing ticket with %llu bytes",
> ticket->bytes);
>
> - list_del_init(&ticket->list);
> + remove_ticket(space_info, ticket);
> ticket->error = -ENOSPC;
> wake_up(&ticket->wait);
>
> @@ -1063,7 +1071,7 @@ static void wait_reserve_ticket(struct btrfs_fs_info *fs_info,
> * despite getting an error, resulting in a space leak
> * (bytes_may_use counter of our space_info).
> */
> - list_del_init(&ticket->list);
> + remove_ticket(space_info, ticket);
> ticket->error = -EINTR;
> break;
> }
> @@ -1121,7 +1129,7 @@ static int handle_reserve_ticket(struct btrfs_fs_info *fs_info,
> * either the async reclaim job deletes the ticket from the list
> * or we delete it ourselves at wait_reserve_ticket().
> */
> - list_del_init(&ticket->list);
> + remove_ticket(space_info, ticket);
> if (!ret)
> ret = -ENOSPC;
> }
>
