All of lore.kernel.org
 help / color / mirror / Atom feed
From: Qu Wenruo <wqu@suse.com>
To: linux-btrfs@vger.kernel.org
Cc: dsterba@suse.cz
Subject: [PATCH v2] btrfs-progs: free-space-cache: Enhance free space cache free space check
Date: Thu,  8 Mar 2018 15:02:31 +0800	[thread overview]
Message-ID: <20180308070231.28876-1-wqu@suse.com> (raw)

When we found free space difference between free space cache and block
group item, we just discard this free space cache.

Normally such difference is caused by btrfs_reserve_extent() called by
delalloc which is out of a transaction.
And since all btrfs_release_extent() is called with a transaction, under
heavy race free space cache can have less free space than block group
item.

Normally kernel will detect such difference and just discard that cache.

However we must be more careful if free space cache has more free space
cache, and if that happens, paried with above race one invalid free
space cache can be loaded into kernel.

So if we find any free space cache who has more free space then block
group item, we report it as an error other than ignoring it.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
v2:
  Fix the timming of free space output.
---
 check/main.c       |  4 +++-
 free-space-cache.c | 32 ++++++++++++++++++++++++--------
 2 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/check/main.c b/check/main.c
index 97baae583f04..bc31f7e32061 100644
--- a/check/main.c
+++ b/check/main.c
@@ -5339,7 +5339,9 @@ static int check_space_cache(struct btrfs_root *root)
 			error += ret;
 		} else {
 			ret = load_free_space_cache(root->fs_info, cache);
-			if (!ret)
+			if (ret < 0)
+				error++;
+			if (ret <= 0)
 				continue;
 		}
 
diff --git a/free-space-cache.c b/free-space-cache.c
index f933f9f1cf3f..9b83a71ca59a 100644
--- a/free-space-cache.c
+++ b/free-space-cache.c
@@ -438,7 +438,8 @@ int load_free_space_cache(struct btrfs_fs_info *fs_info,
 	struct btrfs_path *path;
 	u64 used = btrfs_block_group_used(&block_group->item);
 	int ret = 0;
-	int matched;
+	u64 bg_free;
+	s64 diff;
 
 	path = btrfs_alloc_path();
 	if (!path)
@@ -448,18 +449,33 @@ int load_free_space_cache(struct btrfs_fs_info *fs_info,
 				      block_group->key.objectid);
 	btrfs_free_path(path);
 
-	matched = (ctl->free_space == (block_group->key.offset - used -
-				       block_group->bytes_super));
-	if (ret == 1 && !matched) {
-		__btrfs_remove_free_space_cache(ctl);
+	bg_free = block_group->key.offset - used - block_group->bytes_super;
+	diff = ctl->free_space - bg_free;
+	if (ret == 1 && diff) {
 		fprintf(stderr,
-		       "block group %llu has wrong amount of free space\n",
-		       block_group->key.objectid);
+		       "block group %llu has wrong amount of free space, free space cache has %llu block group has %llu\n",
+		       block_group->key.objectid, ctl->free_space, bg_free);
+		__btrfs_remove_free_space_cache(ctl);
+		/*
+		 * Due to btrfs_reserve_extent() can happen out of a
+		 * transaction, but all btrfs_release_extent() happens inside
+		 * a transaction, so under heavy race it's possible that free
+		 * space cache has less free space, and both kernel just discard
+		 * such cache. But if we find some case where free space cache
+		 * has more free space, this means under certain case such
+		 * cache can be loaded and cause double allocate.
+		 *
+		 * Detect such possibility here.
+		 */
+		if (diff > 0)
+			error(
+"free space cache has more free space than block group item, this could leads to serious corruption, please contact btrfs developers");
 		ret = -1;
 	}
 
 	if (ret < 0) {
-		ret = 0;
+		if (diff <= 0)
+			ret = 0;
 
 		fprintf(stderr,
 		       "failed to load free space cache for block group %llu\n",
-- 
2.16.2


             reply	other threads:[~2018-03-08  7:02 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-08  7:02 Qu Wenruo [this message]
2018-03-08 14:05 ` [PATCH v2] btrfs-progs: free-space-cache: Enhance free space cache free space check Nikolay Borisov
2018-03-08 23:27   ` Qu Wenruo
2018-03-09  8:06     ` Nikolay Borisov
2018-03-09 15:48       ` David Sterba
2018-03-09 15:49 ` David Sterba

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=20180308070231.28876-1-wqu@suse.com \
    --to=wqu@suse.com \
    --cc=dsterba@suse.cz \
    --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.