Skip to content

Commit

Permalink
btrfs: fix reclaim counter leak of space_info objects
Browse files Browse the repository at this point in the history
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: db16180 ("btrfs: account ticket size at add/delete time")
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
  • Loading branch information
fdmanana authored and kdave committed Apr 8, 2020
1 parent 7af5974 commit d611add
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 6 deletions.
1 change: 1 addition & 0 deletions fs/btrfs/block-group.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
20 changes: 14 additions & 6 deletions fs/btrfs/space-info.c
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down

0 comments on commit d611add

Please sign in to comment.