All of lore.kernel.org
 help / color / mirror / Atom feed
From: Josef Bacik <josef@toxicpanda.com>
To: linux-btrfs@vger.kernel.org, kernel-team@fb.com
Subject: [PATCH 4/5] btrfs: only check priority tickets for priority flushing
Date: Mon,  9 Mar 2020 16:23:21 -0400	[thread overview]
Message-ID: <20200309202322.12327-5-josef@toxicpanda.com> (raw)
In-Reply-To: <20200309202322.12327-1-josef@toxicpanda.com>

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.

This is problematic because we prioritize priority tickets, refilling
them first as new space becomes available.  However this leaves a corner
where we could fail to satisfy a priority ticket when we would have
otherwise succeeded.

Consider the case where there's no flushing left to happen other than
commit the transaction, and there are tickets on the normal flushing
list.  The priority flusher comes in, and assume there's enough space
left in the space_info to satisfy this request.  We will still be added
to the priority list and go through the flushing motions, and eventually
fail returning an ENOSPC.

Instead we should only add ourselves to the list if there's something on
the priority_list already.  This way we avoid the incorrect ENOSPC
scenario.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/btrfs/space-info.c | 28 +++++++++++++++++++++++-----
 1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c
index d198cfd45cf7..77ea204f0b6a 100644
--- a/fs/btrfs/space-info.c
+++ b/fs/btrfs/space-info.c
@@ -1276,6 +1276,17 @@ 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_DATA) ||
+		(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
@@ -1311,8 +1322,17 @@ static int __reserve_bytes(struct btrfs_fs_info *fs_info,
 	spin_lock(&space_info->lock);
 	ret = -ENOSPC;
 	used = btrfs_space_info_used(space_info, true);
-	pending_tickets = !list_empty(&space_info->tickets) ||
-		!list_empty(&space_info->priority_tickets);
+
+	/*
+	 * 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
@@ -1338,9 +1358,7 @@ static int __reserve_bytes(struct btrfs_fs_info *fs_info,
 		ticket.error = 0;
 		init_waitqueue_head(&ticket.wait);
 		ticket.steal = (flush == BTRFS_RESERVE_FLUSH_ALL_STEAL);
-		if (flush == BTRFS_RESERVE_FLUSH_ALL ||
-		    flush == BTRFS_RESERVE_FLUSH_DATA ||
-		    flush == BTRFS_RESERVE_FLUSH_ALL_STEAL) {
+		if (is_normal_flushing(flush)) {
 			list_add_tail(&ticket.list, &space_info->tickets);
 			if (!space_info->flush) {
 				space_info->flush = 1;
-- 
2.24.1


  parent reply	other threads:[~2020-03-09 20:23 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-09 20:23 [PATCH 0/5] Deal with a few ENOSPC corner cases Josef Bacik
2020-03-09 20:23 ` [PATCH 1/5] btrfs: Improve global reserve stealing logic Josef Bacik
2020-03-09 20:48   ` Nikolay Borisov
2020-03-10 14:27   ` Nikolay Borisov
2020-03-09 20:23 ` [PATCH 2/5] btrfs: Account for trans_block_rsv in may_commit_transaction Josef Bacik
2020-03-09 20:44   ` Nikolay Borisov
2020-03-09 20:23 ` [PATCH 3/5] btrfs: only take normal tickets into account " Josef Bacik
2020-03-09 20:51   ` Nikolay Borisov
2020-03-09 23:13   ` Nikolay Borisov
2020-03-10 10:27   ` Nikolay Borisov
2020-03-09 20:23 ` Josef Bacik [this message]
2020-03-10 10:30   ` [PATCH 4/5] btrfs: only check priority tickets for priority flushing Nikolay Borisov
2020-03-09 20:23 ` [PATCH 5/5] btrfs: run btrfs_try_granting_tickets if a priority ticket fails Josef Bacik
2020-03-10 10:32   ` Nikolay Borisov
2020-03-13 19:54     ` Josef Bacik
2020-03-10 17:28 ` [PATCH 0/5] Deal with a few ENOSPC corner cases Nikolay Borisov
2020-03-11  1:45   ` David Sterba
2020-03-13 12:37     ` Nikolay Borisov
2020-03-13 19:58 [PATCH 0/5][v2] " Josef Bacik
2020-03-13 19:58 ` [PATCH 4/5] btrfs: only check priority tickets for priority flushing Josef Bacik
2020-03-17 12:55   ` Nikolay Borisov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200309202322.12327-5-josef@toxicpanda.com \
    --to=josef@toxicpanda.com \
    --cc=kernel-team@fb.com \
    --cc=linux-btrfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.