Commit 666daa9f authored Mar 13, 2020 by Josef Bacik Committed by David Sterba May 25, 2020

btrfs: only check priority tickets for priority flushing



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.

Consider the following scenario.  There is no more space to reclaim in
the fs without committing the transaction.  Assume there's 1MiB of space
free in the space info, but there are pending normal tickets with 2MiB
reservations.

Now a priority ticket comes in with a .5MiB reservation.  Because we
have normal tickets pending we add ourselves to the priority list,
despite the fact that we could satisfy this reservation.

The flushing machinery now gets to the point where it wants to commit
the transaction, but because there's a .5MiB ticket on the priority list
and we have 1MiB of free space we assume the ticket will be granted
soon, so we bail without committing the transaction.

Meanwhile the priority flushing does not commit the transaction, and
eventually fails with an ENOSPC.  Then all other tickets are failed with
ENOSPC because we were never able to actually commit the transaction.

The fix for this is we should have simply granted the priority flusher
his reservation, because there was space to make the reservation.
Priority flushers by definition take priority, so they are allowed to
make their reservations before any previous normal tickets.  By not
adding this priority ticket to the list the normal flushing mechanisms
will then commit the transaction and everything will continue normally.

We still need to serialize ourselves with other priority tickets, so if
there are any tickets on the priority list then we need to add ourselves
to that list in order to maintain the serialization between priority
tickets.

Reviewed-by: Nikolay Borisov <nborisov@suse.com>
Tested-by: Nikolay Borisov <nborisov@suse.com>
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: David Sterba <dsterba@suse.com>

parent bb4f58a7

fs/btrfs/space-info.c

+21 −2

Original line number	Diff line number	Diff line
		@@ -1184,6 +1184,16 @@ static int handle_reserve_ticket(struct btrfs_fs_info *fs_info,
		return ret;
		}

		/*
		* This returns true if this flush state will go through the ordinary flushing
		* code.
		*/
		static inline bool is_normal_flushing(enum btrfs_reserve_flush_enum flush)
		{
		return (flush == BTRFS_RESERVE_FLUSH_ALL) \|\|
		(flush == BTRFS_RESERVE_FLUSH_ALL_STEAL);
		}

		/**
		* reserve_metadata_bytes - try to reserve bytes from the block_rsv's space
		* @root - the root we're allocating for
		@@ -1214,8 +1224,17 @@ static int __reserve_metadata_bytes(struct btrfs_fs_info *fs_info,
		spin_lock(&space_info->lock);
		ret = -ENOSPC;
		used = btrfs_space_info_used(space_info, true);

		/*
		* We don't want NO_FLUSH allocations to jump everybody, they can
		* generally handle ENOSPC in a different way, so treat them the same as
		* normal flushers when it comes to skipping pending tickets.
		*/
		if (is_normal_flushing(flush) \|\| (flush == BTRFS_RESERVE_NO_FLUSH))
		pending_tickets = !list_empty(&space_info->tickets) \|\|
		!list_empty(&space_info->priority_tickets);
		else
		pending_tickets = !list_empty(&space_info->priority_tickets);

		/*
		* Carry on if we have enough space (short-circuit) OR call

Admin message