btrfs: fix possible infinite loop in data async reclaim

Dave reported an issue where generic/102 would sometimes hang.  This
turned out to be because we'd get into this spot where we were no longer
making progress on data reservations because our exit condition was not
met.  The log is basically

while (!space_info->full && !list_empty(&space_info->tickets))
	flush_space(space_info, flush_state);

where flush state is our various flush states, but doesn't include
ALLOC_CHUNK_FORCE.  This is because we actually lead with allocating
chunks, and so the assumption was that once you got to the actual
flushing states you could no longer allocate chunks.  This was a stupid
assumption, because you could have deleted block groups that would be
reclaimed by a transaction commit, thus unsetting space_info->full.
This is essentially what happens with generic/102, and so sometimes
you'd get stuck in the flushing loop because we weren't allocating
chunks, but flushing space wasn't giving us what we needed to make
progress.

Fix this by adding ALLOC_CHUNK_FORCE to the end of our flushing states,
that way we will eventually bail out because we did end up with
space_info->full if we free'd a chunk previously.  Otherwise, as is the
case for this test, we'll allocate our chunk and continue on our happy
merry way.

Reported-by: David Sterba <dsterba@suse.com>
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: David Sterba <dsterba@suse.com>
This commit is contained in:
Josef Bacik 2020-08-25 16:56:59 -04:00 committed by David Sterba
parent 1a7a92c8dd
commit c4923027bd
1 changed files with 6 additions and 0 deletions

View File

@ -1044,12 +1044,18 @@ static void btrfs_async_reclaim_metadata_space(struct work_struct *work)
* total_bytes_pinned < reservation we will not commit. This is why the * total_bytes_pinned < reservation we will not commit. This is why the
* previous states are actually important, to make sure we know for sure * previous states are actually important, to make sure we know for sure
* whether committing the transaction will allow us to make progress. * whether committing the transaction will allow us to make progress.
*
* ALLOC_CHUNK_FORCE
* For data we start with alloc chunk force, however we could have been full
* before, and then the transaction commit could have freed new block groups,
* so if we now have space to allocate do the force chunk allocation.
*/ */
static const enum btrfs_flush_state data_flush_states[] = { static const enum btrfs_flush_state data_flush_states[] = {
FLUSH_DELALLOC_WAIT, FLUSH_DELALLOC_WAIT,
RUN_DELAYED_IPUTS, RUN_DELAYED_IPUTS,
FLUSH_DELAYED_REFS, FLUSH_DELAYED_REFS,
COMMIT_TRANS, COMMIT_TRANS,
ALLOC_CHUNK_FORCE,
}; };
static void btrfs_async_reclaim_data_space(struct work_struct *work) static void btrfs_async_reclaim_data_space(struct work_struct *work)