linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] Add support to clear v1 free space cache for btrfs check
@ 2016-05-17  3:47 Qu Wenruo
  2016-05-17  3:47 ` [PATCH 1/4] btrfs-progs: corrupt-block: Add ability to corrupt free space cache file Qu Wenruo
                   ` (4 more replies)
  0 siblings, 5 replies; 19+ messages in thread
From: Qu Wenruo @ 2016-05-17  3:47 UTC (permalink / raw)
  To: linux-btrfs; +Cc: dsterba

The patchset can be also fetched from github, just in case mail list
blocks the last patch, which contains binary file.
https://github.com/adam900710/btrfs-progs.git fsck_clear_cache

Just as describe in the first patch, btrfs kernel "clear_cache" mount
option can't rebuild all free space cache, due to the design of free
space cache.
(Although I pretty like the idea to delay the load of free space cache,
as it hugely reduce the mount time of large fs)

So this makes users to report error in mail list, complaining
"clear_cache" doesn't wipe the annoying kernel warning on corrupted free
space cache.

Since kernel can't handle it, and user consider it more like an error,
we'd better handle it like an error, to fix it in btrfs check.

So this patchset adds the ability to clear free space cache, and add
test case for it.

The clear procedure is different from kernel, it will remove all free
space cache inodes and its contents(with backrefs), and set
cache_generation of super block to -1.
So there will be no free space cache at all and kernel will be happy
with that.

This patch also enhances btrfs_previous_item() to use min_objectid to
return as early as possible.

Lu Fengqi (1):
  btrfs-progs: tests: add 020-bad-free-space-cache

Qu Wenruo (3):
  btrfs-progs: corrupt-block: Add ability to corrupt free space cache
    file
  btrfs-progs: ctree: return earlier for btrfs_previous_item
  btrfs-progs: fsck: Add support to clear free space cache

 Documentation/btrfs-check.asciidoc                 |   8 ++
 btrfs-corrupt-block.c                              | 124 ++++++++++++++++++++-
 cmds-check.c                                       |  58 +++++++++-
 ctree.c                                            |   2 +
 free-space-cache.c                                 | 124 +++++++++++++++++++++
 free-space-cache.h                                 |   4 +
 .../020-bad-free-space-cache/default_case.raw.xz   | Bin 0 -> 164068 bytes
 tests/fsck-tests/020-bad-free-space-cache/test.sh  |  16 +++
 8 files changed, 334 insertions(+), 2 deletions(-)
 create mode 100644 tests/fsck-tests/020-bad-free-space-cache/default_case.raw.xz
 create mode 100755 tests/fsck-tests/020-bad-free-space-cache/test.sh

-- 
2.8.2




^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 1/4] btrfs-progs: corrupt-block: Add ability to corrupt free space cache file
  2016-05-17  3:47 [PATCH 0/4] Add support to clear v1 free space cache for btrfs check Qu Wenruo
@ 2016-05-17  3:47 ` Qu Wenruo
  2016-05-17 13:00   ` David Sterba
  2016-05-17  3:47 ` [PATCH 2/4] btrfs-progs: ctree: return earlier for btrfs_previous_item Qu Wenruo
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 19+ messages in thread
From: Qu Wenruo @ 2016-05-17  3:47 UTC (permalink / raw)
  To: linux-btrfs; +Cc: dsterba

One user in mail list reported free space cache error where clear_cache
mount option failes to fix.

To reproduce such problem, add support to corrupt free space cache
file(old free space cache implement, or space_cache=v1).

With this patch, we can reproduce the problem quite easily:
Not suitable for a btrfs-progs or xfstest test case, as btrfsck won't
treat it as an error.

======
$ fallocate  test.img -l 2G
$ mkfs.btrfs test.img
$ sudo mount test.img  /mnt/btrfs/
$ sudo xfs_io  -f -c "pwrite 0 16M" /mnt/btrfs/tmp
$ sudo umount /mnt/btrfs
$ ./btrfs-corrupt-block -s content -l 136708096 ../test.img
  Here 136708096 is the second data chunk bytenr.
  (The first data chunk is the 8M one from mkfs, which doesn't have free
   space cache)

$ ./btrfsck test.img
Checking filesystem on ../test.img
UUID: 9542051b-8150-42e5-a9e6-95829656485c
checking extents
checking free space cache
btrfs: csum mismatch on free space cache  <<< Found space cache problem
failed to load free space cache for block group 136708096
checking fs roots
checking csums
checking root refs
found 16941058 bytes used err is 0
total csum bytes: 16384
total tree bytes: 163840
total fs tree bytes: 32768
total extent tree bytes: 16384
btree space waste bytes: 139567
file data blocks allocated: 16908288
 referenced 16908288

$ sudo mount -o clear_cache test.img /mnt/btrfs
$ sudo umount /mnt/btrfs
$ ./btrfsck test.img
Checking filesystem on ../test.img
UUID: 9542051b-8150-42e5-a9e6-95829656485c
checking extents
checking free space cache
btrfs: csum mismatch on free space cache <<< Problem not fixed by kernel
failed to load free space cache for block group 136708096
checking fs roots
checking csums
checking root refs
found 16941058 bytes used err is 0
total csum bytes: 16384
total tree bytes: 163840
total fs tree bytes: 32768
total extent tree bytes: 16384
btree space waste bytes: 139567
file data blocks allocated: 16908288
 referenced 16908288
======

Btrfs-progs fix will follow soon.

Reported-by: Ivan P <chrnosphered@gmail.com>
Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 btrfs-corrupt-block.c | 124 +++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 123 insertions(+), 1 deletion(-)

diff --git a/btrfs-corrupt-block.c b/btrfs-corrupt-block.c
index d331f96..eb27265 100644
--- a/btrfs-corrupt-block.c
+++ b/btrfs-corrupt-block.c
@@ -115,6 +115,9 @@ static void print_usage(int ret)
 	fprintf(stderr, "\t-C Delete a csum for the specified bytenr.  When "
 		"used with -b it'll delete that many bytes, otherwise it's "
 		"just sectorsize\n");
+	fprintf(stderr, "\t-s <method>\n");
+	fprintf(stderr, "\t   Corrupt free space cache file(space_cache v1), must also specify -l for blockgroup bytenr\n");
+	fprintf(stderr, "\t   <method> can be 'zero_gen','rand_gen' or 'content'\n");
 	exit(ret);
 }
 
@@ -1012,6 +1015,100 @@ out:
 	return ret;
 
 }
+
+u64 rand_u64(void)
+{
+	u64 ret = 0;
+	int n;
+
+	for (n = 0; n < sizeof(ret) / sizeof(RAND_MAX); n++)
+		ret += rand() << (n * sizeof(RAND_MAX) * 8);
+	return ret;
+}
+
+#define CORRUPT_SC_FILE_ZERO_GEN	1
+#define CORRUPT_SC_FILE_RAND_GEN	2
+#define CORRUPT_SC_FILE_CONTENT		3
+int corrupt_space_cache_file(struct btrfs_fs_info *fs_info, u64 block_group,
+			     int method)
+{
+	struct btrfs_root *tree_root = fs_info->tree_root;
+	struct btrfs_path path;
+	struct btrfs_key key;
+	struct btrfs_disk_key location;
+	struct btrfs_free_space_header *sc_header;
+	struct btrfs_file_extent_item *fi;
+	struct extent_buffer *node;
+	struct extent_buffer *corrupted_node;
+	u64 disk_bytenr;
+	int slot;
+	int ret;
+
+	key.objectid = BTRFS_FREE_SPACE_OBJECTID;
+	key.type = 0;
+	key.offset = block_group;
+
+	btrfs_init_path(&path);
+
+	/* Don't start trans, as this will cause generation different */
+	ret = btrfs_search_slot(NULL, tree_root, &key, &path, 0, 0);
+	if (ret) {
+		error("failed to find free space cahce file for block group %llu, ret: %d\n",
+		      block_group, ret);
+		goto out;
+	}
+	slot = path.slots[0];
+	node = path.nodes[0];
+	sc_header = btrfs_item_ptr(node, slot, struct btrfs_free_space_header);
+	if (method == CORRUPT_SC_FILE_ZERO_GEN ||
+	    method == CORRUPT_SC_FILE_RAND_GEN) {
+		u64 dst_gen;
+
+		if (method == CORRUPT_SC_FILE_ZERO_GEN)
+			dst_gen = 0;
+		else
+			dst_gen = rand_u64();
+
+		btrfs_set_free_space_generation(node, sc_header, dst_gen);
+		/* Manually re-calc csum and write to disk */
+		ret = write_tree_block(NULL, tree_root, node);
+		goto out;
+	}
+
+	btrfs_free_space_key(node, sc_header, &location);
+	btrfs_disk_key_to_cpu(&key, &location);
+	btrfs_release_path(&path);
+
+	/* Change to type and offset to search file extent */
+	key.type = BTRFS_EXTENT_DATA_KEY;
+	key.offset = 0;
+
+	ret = btrfs_search_slot(NULL, tree_root, &key, &path, 0, 0);
+	if (ret) {
+		error("failed to find free space cache extent data for blockgroup %llu, ret: %d\n",
+		      block_group, ret);
+		goto out;
+	}
+
+	slot = path.slots[0];
+	node = path.nodes[0];
+	fi = btrfs_item_ptr(node, slot, struct btrfs_file_extent_item);
+	disk_bytenr = btrfs_file_extent_disk_bytenr(node, fi);
+
+	/*
+	 * Directly write random data into
+	 * [disk_bytenr, disk_bytenr + sectorsize) as free space cache inode
+	 * doesn't have csum
+	 */
+	corrupted_node = debug_corrupt_block(tree_root, disk_bytenr,
+					     tree_root->sectorsize, 0);
+	free_extent_buffer(corrupted_node);
+
+out:
+	btrfs_release_path(&path);
+	return ret;
+}
+
 int main(int argc, char **argv)
 {
 	struct cache_tree root_cache;
@@ -1032,6 +1129,7 @@ int main(int argc, char **argv)
 	int corrupt_item = 0;
 	int corrupt_di = 0;
 	int delete = 0;
+	int corrupt_sc_file = 0;
 	u64 metadata_block = 0;
 	u64 inode = 0;
 	u64 file_extent = (u64)-1;
@@ -1065,11 +1163,12 @@ int main(int argc, char **argv)
 			{ "delete", no_argument, NULL, 'd'},
 			{ "root", no_argument, NULL, 'r'},
 			{ "csum", required_argument, NULL, 'C'},
+			{ "space-cache-file", required_argument, NULL, 's'},
 			{ "help", no_argument, NULL, GETOPT_VAL_HELP},
 			{ NULL, 0, NULL, 0 }
 		};
 
-		c = getopt_long(argc, argv, "l:c:b:eEkuUi:f:x:m:K:IDdr:C:",
+		c = getopt_long(argc, argv, "l:c:b:eEkuUi:f:x:m:K:IDdr:C:s:",
 				long_options, NULL);
 		if (c < 0)
 			break;
@@ -1136,6 +1235,22 @@ int main(int argc, char **argv)
 			case 'C':
 				csum_bytenr = arg_strtou64(optarg);
 				break;
+			case 's':
+				if (!strncmp(optarg, "zero_gen",
+					     sizeof("zero_gen")))
+					corrupt_sc_file =
+						CORRUPT_SC_FILE_ZERO_GEN;
+				else if (!strncmp(optarg, "rand_gen",
+						  sizeof("rand_gen")))
+					corrupt_sc_file =
+						CORRUPT_SC_FILE_RAND_GEN;
+				else if (!strncmp(optarg, "content",
+						  sizeof("content")))
+					corrupt_sc_file =
+						CORRUPT_SC_FILE_CONTENT;
+				else
+					print_usage(1);
+				break;
 			case GETOPT_VAL_HELP:
 			default:
 				print_usage(c != GETOPT_VAL_HELP);
@@ -1154,6 +1269,13 @@ int main(int argc, char **argv)
 		fprintf(stderr, "Open ctree failed\n");
 		exit(1);
 	}
+	if (corrupt_sc_file) {
+		if (logical == (u64)-1)
+			print_usage(1);
+		ret = corrupt_space_cache_file(root->fs_info, logical,
+					       corrupt_sc_file);
+		goto out_close;
+	}
 	if (extent_rec) {
 		struct btrfs_trans_handle *trans;
 
-- 
2.8.2




^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 2/4] btrfs-progs: ctree: return earlier for btrfs_previous_item
  2016-05-17  3:47 [PATCH 0/4] Add support to clear v1 free space cache for btrfs check Qu Wenruo
  2016-05-17  3:47 ` [PATCH 1/4] btrfs-progs: corrupt-block: Add ability to corrupt free space cache file Qu Wenruo
@ 2016-05-17  3:47 ` Qu Wenruo
  2016-05-17 12:50   ` David Sterba
  2016-05-17  3:47 ` [PATCH 3/4] btrfs-progs: fsck: Add support to clear free space cache Qu Wenruo
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 19+ messages in thread
From: Qu Wenruo @ 2016-05-17  3:47 UTC (permalink / raw)
  To: linux-btrfs; +Cc: dsterba

Btrfs_previous_item() has a parameter to specify minimal objectid to
return.

But surprisingly it doesn't use it at all.
Although that's OK, but it would take years long for large tree, so
return it earlier.

Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 ctree.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/ctree.c b/ctree.c
index 079696e..3a9f417 100644
--- a/ctree.c
+++ b/ctree.c
@@ -2894,6 +2894,8 @@ int btrfs_previous_item(struct btrfs_root *root,
 		btrfs_item_key_to_cpu(leaf, &found_key, path->slots[0]);
 		if (found_key.type == type)
 			return 0;
+		if (found_key.objectid < min_objectid)
+			break;
 	}
 	return 1;
 }
-- 
2.8.2




^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 3/4] btrfs-progs: fsck: Add support to clear free space cache
  2016-05-17  3:47 [PATCH 0/4] Add support to clear v1 free space cache for btrfs check Qu Wenruo
  2016-05-17  3:47 ` [PATCH 1/4] btrfs-progs: corrupt-block: Add ability to corrupt free space cache file Qu Wenruo
  2016-05-17  3:47 ` [PATCH 2/4] btrfs-progs: ctree: return earlier for btrfs_previous_item Qu Wenruo
@ 2016-05-17  3:47 ` Qu Wenruo
  2016-05-17  3:53   ` [PATCH v1.1 " Qu Wenruo
  2016-05-17  3:47 ` [PATCH 4/4] btrfs-progs: tests: add 020-bad-free-space-cache Qu Wenruo
  2016-05-17  3:56 ` [PATCH 0/4] Add support to clear v1 free space cache for btrfs check Qu Wenruo
  4 siblings, 1 reply; 19+ messages in thread
From: Qu Wenruo @ 2016-05-17  3:47 UTC (permalink / raw)
  To: linux-btrfs; +Cc: dsterba

Add a new option "--clear-space-cache" for btrfs check.

Unlike many may assume, kernel "clear_cache" will only rebuild *SOME* of
the free space cache, not *ALL*.

Or more specifically, it will only rebuild free space cache for block
groups that has read out the cache during "clear_cache" mount time.

And since kernel will not read out free space cache at mount, but only
when kernel needs to allocate space from the block group, so bad
free space cache will stay untouched for a long time.

Such kernel design is quite good, as it dramatically reduce the mount
time for large fs, so I prefer not to change that.

Instead, since a lot of user consider free space warning from kernel as
an error, we should handle them like an error, which means we should use
btrfsck to fix it, even it's harmless most of time.

This patch will use "--clear-space-cache" to clear all free space
cache, and modify cache_generation to -1.

Reported-by: Ivan P <chrnosphered@gmail.com>
Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 Documentation/btrfs-check.asciidoc |   8 +++
 cmds-check.c                       |  58 ++++++++++++++++-
 free-space-cache.c                 | 124 +++++++++++++++++++++++++++++++++++++
 free-space-cache.h                 |   4 ++
 4 files changed, 193 insertions(+), 1 deletion(-)

diff --git a/Documentation/btrfs-check.asciidoc b/Documentation/btrfs-check.asciidoc
index 74a2ad2..ea25582 100644
--- a/Documentation/btrfs-check.asciidoc
+++ b/Documentation/btrfs-check.asciidoc
@@ -78,6 +78,14 @@ respective superblock offset is within the device size
 This can be used to use a different starting point if some of the primary
 superblock is damaged.
 
+--clear-space-cache::
+clear all free space cache
++
+NOTE:
+Kernel mount option 'clear_cache' is only designed to rebuild free space cache
+which is modified during the lifetime of that mount option.
+It doesn't rebuild all free space cache, nor clear them out.
+
 DANGEROUS OPTIONS
 -----------------
 
diff --git a/cmds-check.c b/cmds-check.c
index ec0bbfd..1f6aefb 100644
--- a/cmds-check.c
+++ b/cmds-check.c
@@ -9524,10 +9524,41 @@ const char * const cmd_check_usage[] = {
 	"                            print subvolume extents and sharing state",
 	"-r|--tree-root <bytenr>     use the given bytenr for the tree root",
 	"--chunk-root <bytenr>       use the given bytenr for the chunk tree root",
+	"--clear-space-cache         clear all free space cache(v1)",
 	"-p|--progress               indicate progress",
 	NULL
 };
 
+static int clear_free_space_cache(struct btrfs_fs_info *fs_info)
+{
+	struct btrfs_trans_handle *trans;
+	struct btrfs_block_group_cache *bg_cache;
+	u64 current = 0;
+	int ret = 0;
+
+	/* Clear all free space cache inodes and its extent data */
+	while (1) {
+		bg_cache = btrfs_lookup_first_block_group(fs_info, current);
+		if (!bg_cache)
+			break;
+		ret = btrfs_clear_free_space_cache(fs_info, bg_cache);
+		if (ret < 0)
+			return ret;
+		current = bg_cache->key.objectid + bg_cache->key.offset;
+	}
+
+	/* Don't forget to set cache_generation to -1 */
+	trans = btrfs_start_transaction(fs_info->tree_root, 0);
+	if (IS_ERR(trans)) {
+		error("failed to update super block cache generation");
+		return PTR_ERR(trans);
+	}
+	btrfs_set_super_cache_generation(fs_info->super_copy, (u64)-1);
+	btrfs_commit_transaction(trans, fs_info->tree_root);
+	
+	return ret;
+}
+
 int cmd_check(int argc, char **argv)
 {
 	struct cache_tree root_cache;
@@ -9543,13 +9574,15 @@ int cmd_check(int argc, char **argv)
 	int init_csum_tree = 0;
 	int readonly = 0;
 	int qgroup_report = 0;
+	int clear_space_cache = 0;
 	enum btrfs_open_ctree_flags ctree_flags = OPEN_CTREE_EXCLUSIVE;
 
 	while(1) {
 		int c;
 		enum { GETOPT_VAL_REPAIR = 257, GETOPT_VAL_INIT_CSUM,
 			GETOPT_VAL_INIT_EXTENT, GETOPT_VAL_CHECK_CSUM,
-			GETOPT_VAL_READONLY, GETOPT_VAL_CHUNK_TREE };
+			GETOPT_VAL_READONLY, GETOPT_VAL_CHUNK_TREE, 
+			GETOPT_VAL_CLEAR_SPACE_CACHE};
 		static const struct option long_options[] = {
 			{ "super", required_argument, NULL, 's' },
 			{ "repair", no_argument, NULL, GETOPT_VAL_REPAIR },
@@ -9566,6 +9599,8 @@ int cmd_check(int argc, char **argv)
 			{ "tree-root", required_argument, NULL, 'r' },
 			{ "chunk-root", required_argument, NULL,
 				GETOPT_VAL_CHUNK_TREE },
+			{ "clear-space-cache", no_argument, NULL,
+				GETOPT_VAL_CLEAR_SPACE_CACHE},
 			{ "progress", no_argument, NULL, 'p' },
 			{ NULL, 0, NULL, 0}
 		};
@@ -9631,6 +9666,11 @@ int cmd_check(int argc, char **argv)
 			case GETOPT_VAL_CHECK_CSUM:
 				check_data_csum = 1;
 				break;
+			case GETOPT_VAL_CLEAR_SPACE_CACHE:
+				clear_space_cache = 1;
+				repair = 1;
+				ctree_flags |= OPEN_CTREE_WRITES;
+				break;
 		}
 	}
 
@@ -9693,6 +9733,22 @@ int cmd_check(int argc, char **argv)
 	}
 
 	uuid_unparse(info->super_copy->fsid, uuidbuf);
+	if (clear_space_cache) {
+		/* Basic check, don't support v2 free space cache yet */
+		if (btrfs_fs_compat_ro(info,
+				BTRFS_FEATURE_COMPAT_RO_FREE_SPACE_TREE)) {
+			error("doesn't support space cache tree(v2) yet");
+			ret = -ENOTTY;
+			goto close_out;
+		}
+		printf("Clearing all free space cache\n");
+		ret = clear_free_space_cache(info);
+		if (ret)
+			error("failed to clear free space cache");
+		else
+			printf("Free space cache cleared\n");
+		goto close_out;
+	}
 	if (qgroup_report) {
 		printf("Print quota groups for %s\nUUID: %s\n", argv[optind],
 		       uuidbuf);
diff --git a/free-space-cache.c b/free-space-cache.c
index 357d69e..971702a 100644
--- a/free-space-cache.c
+++ b/free-space-cache.c
@@ -25,6 +25,7 @@
 #include "crc32c.h"
 #include "bitops.h"
 #include "internal.h"
+#include "utils.h"
 
 /*
  * Kernel always uses PAGE_CACHE_SIZE for sectorsize, but we don't have
@@ -877,3 +878,126 @@ next:
 		prev = e;
 	}
 }
+
+int btrfs_clear_free_space_cache(struct btrfs_fs_info *fs_info,
+				 struct btrfs_block_group_cache *bg)
+{
+	struct btrfs_trans_handle *trans;
+	struct btrfs_root *tree_root = fs_info->tree_root;
+	struct btrfs_path path;
+	struct btrfs_key key;
+	struct btrfs_disk_key location;
+	struct btrfs_free_space_header *sc_header;
+	struct extent_buffer *node;
+	u64 ino;
+	int slot;
+	int ret;
+
+	trans = btrfs_start_transaction(tree_root, 1);
+	if (IS_ERR(trans))
+		return PTR_ERR(trans);
+
+	btrfs_init_path(&path);
+
+	key.objectid = BTRFS_FREE_SPACE_OBJECTID;
+	key.type = 0;
+	key.offset = bg->key.objectid;
+
+	ret = btrfs_search_slot(trans, tree_root, &key, &path, -1, 1);
+	if (ret > 0) {
+		ret = 0;
+		goto out;
+	}
+	if (ret < 0)
+		goto out;
+
+	node = path.nodes[0];
+	slot = path.slots[0];
+	sc_header = btrfs_item_ptr(node, slot, struct btrfs_free_space_header);
+	btrfs_free_space_key(node, sc_header, &location);
+	ino = location.objectid;
+
+	/* Delete the free space header, as we have the ino to continue */
+	ret = btrfs_del_item(trans, tree_root, &path);
+	if (ret < 0) {
+		error("failed to remove free space header for block group %llu",
+		      bg->key.objectid);
+		goto out;
+	}
+	btrfs_release_path(&path);
+
+	/* Iterate from the end of the free space cache inode */
+	key.objectid = ino;
+	key.type = BTRFS_EXTENT_DATA_KEY;
+	key.offset = (u64)-1;
+	ret = btrfs_search_slot(trans, tree_root, &key, &path, -1, 1);
+	if (ret < 0) {
+		error("failed to locate free space cache extent for block group %llu",
+		      bg->key.objectid);
+		goto out;
+	}
+	while (1) {
+		struct btrfs_file_extent_item *fi;
+		u64 disk_bytenr;
+		u64 disk_num_bytes;
+
+
+		ret = btrfs_previous_item(tree_root, &path, ino,
+					  BTRFS_EXTENT_DATA_KEY);
+		if (ret > 0) {
+			ret = 0;
+			break;
+		}
+		if (ret < 0) {
+			error("failed to locate free space cache extent for block group %llu",
+			      bg->key.objectid);
+			goto out;
+		}
+		node = path.nodes[0];
+		slot = path.slots[0];
+		btrfs_item_key_to_cpu(node, &key, slot);
+		fi = btrfs_item_ptr(node, slot, struct btrfs_file_extent_item);
+		disk_bytenr = btrfs_file_extent_disk_bytenr(node, fi);
+		disk_num_bytes = btrfs_file_extent_disk_num_bytes(node, fi);
+
+		ret = btrfs_free_extent(trans, tree_root, disk_bytenr,
+					disk_num_bytes, 0, tree_root->objectid,
+					ino, key.offset);
+		if (ret < 0) {
+			error("failed to remove backref for disk bytenr %llu",
+			      disk_bytenr);
+			goto out;
+		}
+		ret = btrfs_del_item(trans, tree_root, &path);
+		if (ret < 0) {
+			error("failed to remove free space extent data for ino %llu offset %llu",
+			      ino, key.offset);
+			goto out;
+		}
+	}
+	btrfs_release_path(&path);
+
+	/* Now delete free space cache inode item */
+	key.objectid = ino;
+	key.type = BTRFS_INODE_ITEM_KEY;
+	key.offset = 0;
+
+	ret = btrfs_search_slot(trans, tree_root, &key, &path, -1, 1);
+	if (ret > 0)
+		warning("free space inode %llu not found, ignore", ino);
+	if (ret < 0) {
+		error("failed to locate free space cache inode %llu for block group %llu",
+		      ino, bg->key.objectid);
+		goto out;
+	}
+	ret = btrfs_del_item(trans, tree_root, &path);
+	if (ret < 0) {
+		error("failed to delete free space cache inode %llu for block group %llu",
+		      ino, bg->key.objectid);
+	}
+out:
+	btrfs_release_path(&path);
+	if (!ret)
+		btrfs_commit_transaction(trans, tree_root);
+	return ret;
+}
diff --git a/free-space-cache.h b/free-space-cache.h
index 9214077..90302ac 100644
--- a/free-space-cache.h
+++ b/free-space-cache.h
@@ -59,4 +59,8 @@ void unlink_free_space(struct btrfs_free_space_ctl *ctl,
 		       struct btrfs_free_space *info);
 int btrfs_add_free_space(struct btrfs_free_space_ctl *ctl, u64 offset,
 			 u64 bytes);
+
+/* Used for clearing one free space cache for given block group */
+int btrfs_clear_free_space_cache(struct btrfs_fs_info *fs_info,
+				 struct btrfs_block_group_cache *bg);
 #endif
-- 
2.8.2




^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 4/4] btrfs-progs: tests: add 020-bad-free-space-cache
  2016-05-17  3:47 [PATCH 0/4] Add support to clear v1 free space cache for btrfs check Qu Wenruo
                   ` (2 preceding siblings ...)
  2016-05-17  3:47 ` [PATCH 3/4] btrfs-progs: fsck: Add support to clear free space cache Qu Wenruo
@ 2016-05-17  3:47 ` Qu Wenruo
  2016-05-17  3:56 ` [PATCH 0/4] Add support to clear v1 free space cache for btrfs check Qu Wenruo
  4 siblings, 0 replies; 19+ messages in thread
From: Qu Wenruo @ 2016-05-17  3:47 UTC (permalink / raw)
  To: linux-btrfs; +Cc: dsterba, Lu Fengqi

From: Lu Fengqi <lufq.fnst@cn.fujitsu.com>

To test if fsck can detect and clear free space cache error

Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
---
 .../020-bad-free-space-cache/default_case.raw.xz       | Bin 0 -> 164068 bytes
 tests/fsck-tests/020-bad-free-space-cache/test.sh      |  16 ++++++++++++++++
 2 files changed, 16 insertions(+)
 create mode 100644 tests/fsck-tests/020-bad-free-space-cache/default_case.raw.xz
 create mode 100755 tests/fsck-tests/020-bad-free-space-cache/test.sh

diff --git a/tests/fsck-tests/020-bad-free-space-cache/default_case.raw.xz b/tests/fsck-tests/020-bad-free-space-cache/default_case.raw.xz
new file mode 100644
index 0000000000000000000000000000000000000000..5e08de426c5cc5faa9ccd430432b9c37f4e072ff
GIT binary patch
literal 164068
zcmeI*^;eeNx-amDR-{2nC8RqA>6GpeDQQIM6j137K{}*Cq@`OL1t~!~M5R-@W$!We
zIOF`V&KY~XYi!^3uDve)0mhimoY%ZQ-#PF5zTo$Kpoc)9E{#>mAS0-eh!F^c3&&#4
z<)trbl_3IQe{p%4Q+p{ZDtFyUz(l<+hH)t;2;*dHfx>Tuqf1a2NwM6kCoD{vBL{oZ
z`^}FC8hp)pUD7Vsu<uWgYvR1Ip0v)n$eSCaSBJdBN_`NcP7sOhaA5g3U!?SMuA9Bf
z=J>r{jDvPxtx%!Tkbo_rhU;xZv758eDW_e=+Pb&X`q?vkT5awf>N;|gam?{2x9MLE
zeUp;lr*CIb@fnr#*(qs|p<(vVd;4f8MEO5zWo@Q=@)kZ7RJb_c+Q(T_uQqFn<0z&{
zRGX_fa*CJuia9>py^$1!^-5Lr7m~=)Rm^x<fu!nL?P(fiqPF8Oma6swvWTp#OAgh?
zbtBKky9&QG)$5xZa`=zue|KB!GZ~$4Td53JLf<0|xFBg1T(KkWP-lPesqci83g_y&
zcWBIWKCRmUQHiW2n^{kEMQ)AI*;Berlu{Fp%OYl;3KMTEVKt>-c#fC)a>@ofXcoM0
zxOX}<R5BNxfF_GuTQzg*pojT^_Z#!viLeyD!Lz!-qc*u8b#q+!+Lfet*8Fz7iqB9O
zHy@~tq)*IVJ0Uq>xG}RQ{v_w-^E9b7B9u7oM!FE)!@e7I*Y)gzdMnHKJ!d^!dxqD?
zY2G@P9<y0jQPNDJU&$P`oA$m(7L=7G({imX+3CtkoVJwF%av-oI|3xpqZX7Rje3`A
zn?vFBnv<qiHEat4o734AVu~IuMN3*bmC|}%d&e&D+A2S1F_zJlE>USQeg{*~z$6l7
zu5~GUeZO{^{KJ(!R`NO3(c!lgRb>`oax}897FehonYY`Q>I&+FbF?Xzbo#ze6J=~?
zHjwFRv9grUuhEJON{u<^wpN;6xqhmv5c?BHU1F^K6AyX9b=1LR919|%4ee80FVUKm
zM4=m`>b?{vR6Pe%mfuv?M3Uu+S>jJ*PgRyRzQ>^7Crg)3|317RbEx$*#FdqEx?N4{
z)$K%d6r&yoA);1p{-nI?NO>zcSD7~oFZJk)ov?^w^4|+Y(SCDC&!;ikVEh#LIX)@O
zi}}?qgX<AlD|(W0(#1sCXf$poo#yeuD?To?yf=jIzKxqEd?v_6oQZTPCJkcV8QURR
z@1J%j{HSnvoS)ybCy+}UaU!ZLI7c@6l}R<waXauGc8&C{Q8ig-nHf~dHLA)RyiJ<o
zt5}%Fa(5PeyIY4BJAx@2v_Idr^&u=dIP~%DjC*%Xy3y(SJU%hrGxo|V8(vlHfl&k<
zgMZdd1o1}A3?fc<SCsDVh~#JwgAcdCow{h02w4m7E%%#J`0`&Z{BwlRdKq#2Jf;j7
zABoi{XI)*2+%k}*IV|EmY#^T)-&IWM(=3bck_+h0nO<6w?$=jV>{;9uH|ITiO~*JN
zsOJ$WMfUR%ipq4-I{&wJGHj0i0?w4_KGXQ)5J5(PpNb!p3Fy`JvNJ~(&}bN3MCl?h
z@8it(<>d}t@ft`>Oo?nMPvb3?TWtRkmbMw2HEJ94<>l}G!4x}Qq-9*<A+aDfoHg?=
z-j_b-BG0Ie<g)wwf7^Fk_H7QzvRI#A;TM(|-)a~rXCO7u4O+F-c<_GJtyp(qQ6KHf
z$%m<u^D_+=mwpO;D#?>sO_5_J^Q%HWpO|gFRtx>?Y-Tqq#1&Pvl#C|Cxx|Y-c68T0
zzSe&k&nSaiaQ=MmEQOQ(err_Vp1*Ia#s8gF|Ac?7jw@D?ap{AcjSwwlEwGua6Aym7
zM$?jcd$sEUzP<k!u8F({pS2xdD%7mBuzVx5J9((1AelGz{Os!?Uhj^KL}i+Poveh!
zu8Z42KyeF8BGt}!cUSwwT_JnxXk}3%mmuRyyGDz8Od}qY{#o~hvRrYcr;B0(0eOks
zo3k&)#Tp$iu5)VKKlSXlr8OC>Ai^wt=h4j&qjTJ&LW?hC@gtx7<{iGD)gfz-U+`=2
zEfnoERAZh?3<s0*M!RgHwMo6p+qOY&k>-EGJTrP-GN$t=hFyat8Hb;|JH6`U9C1v<
zG*aj1v?9_>D<pJ|m2|oAs84x#`~{7VlQ#;l${X5>WR>BIEAa{1tD2|p_}P3)nrcUm
z`WDo3xD{m|t`Eo}S?#(<v#U4ahYQ$?Ew`hWJQe#+gW(pAk#w^oEZ0MfuN3E3S@Jmk
zW9exuv6>GP+X{o@>U&0`rZU%(&g)IbHq!+@*hQL;&*C4cFL@US-ukF_v(;LmtUTYE
z*~MKipi<J&?ugGmD!y_xS!J5)EoVbx<1?S2^m>{D?=kEfVlSQ+`4dsweIE7P6D86#
z{a-i#{!c~y@1}m6`?v!v>_1Rb{%c`FCH%XKFbvoq+_hl9V8DKFUj#D@X4s!G!_bk*
z5bwQ5Hw+>_^W!Y-CQ{ZRRV|5kRfl#lR5WUCzAkAo(RRwE5uA$h_ob+=qLNArc_b0G
zX}Cq*Zme>pFje-UHNP8@V13P`vzJoeuh;t})YCZQWfi(l+3miK%Mbn7bbff(;xeZ%
z#pfkD(ynr+2-aim_RZnA*|4`cAs?*597S0QyBYBoRZ_5{*pTu9A8s?`S*6fdCgjz8
zVtTyiJ0-;trDbehwAw}}z|~bb&2rb=2J_?br+mJQw`HHHQv7XgO5;5PYF%ie*ohBx
z)58vXrnadRu<Y<OUi{QYV?Q;gQ{GrAQd`fS+X)qvrh4(}#Xaeg8qKwhK_gt&;CO~?
z#uUsXExYmX-ViY(uCtG-D}*AoKYSW!-X5cePyXAsqQ381ouTW5uJiZNAGWxEXp4(R
z;z5ibQ&*KRdxt!gOf35G+atgK4yb?iSg7f=NumyF8PqbUWq+~obrU-q@vUIIKq_`5
z!|Y=wr<3iS&CH&S59*#08M$E<b3)86b`D?5ks3dFmnAHV3)?V-|I~csZ~Hj3;>Ro5
z>@P-^o~sEX-_l>~BFp4v$rf;tTM6-V)Gy#|iNtWr$=rLYH8tD3)~51~D6EZMaD`Rx
zk|>T+DaPAW`a{>x-1(hXn??`$_G2wGq)ZvA$MP|pekEM$92(_tMf;`)G+g50N6lbS
zJ)f!mxh!0dk+HSiKiO<BX)K!}`VhI!-GB@gMafIod+S^`&K8Zz=J1jJrfztq?2slq
zb%va~pqm*s1M&I3t2=)pX7Ac~)1oA`E9O<TyNx`UrQN-E>wNq9g{>k&<MDHhRIn9=
z7z)i<#TxLepOWn&ESR5c5n!!w3J98S=L%gfJh5+Xp7FAG-A~DSbbb#}Z|p8fEWhL`
zjWN1|Aug(J!txzigEoBrb5QrVD$POE#f8<JAwg$Y;<31U-U?d8SzO`+2R|vt0%?Y+
zpY~sB7E6QH-xf(74CPSXS&~m&5^m#&VaK^iN>8y;bzA#X@Tz9R43U~Nb#)!zMJ_KR
zPC~aMVHw(WmbsxZjgq@}&d3c4j+&Czc%;L~!t(Se%=sov+<Zh<-@EXI$p-51w4SvJ
zma!MAvZ0lye;MyL&Ao2ba--Ruoyw?h&ZHahN>nMUP^ry1;yw2upObSGo$1ueRoW{(
zUuZRi1hw1JA`lu)n!93n7Y84Fs!}i~(-sqm-(fE^&_ul;_%!iT$>8pXy3+xDR_9-o
zbn=x*vIS41bnyt-pP8Pm8CV!A{Gz{+eLK6ZA9v#Dqo<(|X={<IXJo{s`J%7xbO1&;
zro^bukn6y)m+sBy5nB7j8%#W!E=)W8N+`wJL)vn>vcY57Sq;2g$s=PNGzoq0eWmn6
zR~HIZeK;<qmaG^Rc56!Z40zt)1r!~9_oq$HEPtk-Pk}A)=7N|1W|Mixt2<9Ee~Gh=
z(aMI&?GyOZRtkL@)-7bYT=J?BLuHB3>%RM`Z~vqLZSAsSU-Zb1Bn&ex`#e=SV~;D3
z(%M|1V&8SY6X(qJs@tl%s}oP4b9+t#Y0WYVnabO4R%zS(cGOq#ew`c#cACjCxia*%
zS;JB+a>J=VJTHr474RzQI`iI5dLZ+p&C)9?Q7pfr7ck)@yi+V=+?+5|bQfh%G><dC
z5A|mLdc-dxXFYzK>1%SmYMx;-L(AS?_F+xLm0V{dI1(WX9aY~qdB%CvB%E2$?uUum
z+9+h#^$_9|%bTCeZX3MMOTJ%4(}BiO=2$R;Dlp~8uBh-ZO-QHzNcw_DO|SK!dhaQI
z-3*`iA+bpRC-*Ym4N_{~op^&A25H$m*;8VEi~P-zgN*cf=DPQe_j_d`sF&+MNn&wj
zE?0P_Ur*<~=~qmsr1zp`?kJvoE#sv78B_KN(_)??`ryNh#gAVOvO|prL*|Rf)f1zS
zI?$eFOxl{`;g!X&Bzp+YDipKr>KxZ8NZ9DxU>~y>ZSoEZ%*(Gzp|)}$FJ&NYYHdq1
zdNPp54B@F>@GgF0(sK?`E40$$rtoZ8`FPcoIPxNnB;a-JMp}?N1!E&pDQ-l7)n=-t
z3gM^xJ|ltn2c`(|#K=<lg$=P1;W%Dz^$o7AmYI<z&wE{u#2bBmm?v+BO}>#M<r0&O
z3%J36&!L7hCENPuBu4k^PddZ(fma>Q9^IQ>H)<ytNow%~@!KldQsjqJV<W~$@yPTM
zjdxO(@13&~J>*$_qDQgyoN12jG0jyZVUz6WGnLy5Ppcvy;~cB6<X}~FrK<UiS0B-0
zS=l-@#E3N`zC8QdA$Y@-+ccyOt>!^I-D&vs>$?_ch#wl&MD=Slop%K`v4V^@-qLM5
zN{5}>l<i}0gh|}3)+f229h==RR~;(xBbUu@y~<JB>zq^R(wV-JAi>A>bQvwYF?UFk
z&Xy7Pq*Z?kW72b^(B~Pe%CJwZ|8{%i#UVFcnAj0%VtP*T*AJqC!CGBzPZA8@GU?tg
zCh>6alxQe0emH3NLd_7@!Hj*G|A=~RzcueeGN(V5z%~8KNqJYQ*e(phXICqbaZ330
zIPSOH3aFez94pniv=HeLASuNg`*GaxSaf%aYF3o36@D-JByz{%6wOehnf94|s&cUC
zWcuWa2&dp3{4kd8mPfwi4h+ql=UwNFLT^c|N@5j?DQ)bBPlj5*tHqj03S*=@#Wu9B
zSLS>#3m-HHzr(FoqZX>GORO{4@RsSN>_MQkXR9~z1v@(R@oo%ze|BTo1lQL|f2^^X
zg-&JUVPdVFN4Eo2ITtCzTD=5_8(ZWptxwNQH_efRckiBH7-e6Tv;9??sHfsedV}ds
z^svM;^d2?u36lT;TjEW1{<}r|#!YPp@>Al(UXdl{K5whn;@xL5O50__az(J;r14_Q
zl2~X?PcAu_FDg682Wn@s@@x$EuC0l0g(A<DhA(0ys0)R!^O>u~$!ns$x|T7Mr<MES
z{)6S+;GS<ct<Y2DT#jaO56F2!amX0wlN<CJo=ztWWcm|e5c+O<-M=$g%DRdbO|aT(
zcvcgFA|C9BgGnW=k0{`CJDwcjiXRO1im%|WVCu=wV8*Pu#hf6jz>A|K%AZctd&;4v
zUnEnlb%Axij`Wy~=63o>yaK62l=HXXZ49SF|8z!GB*G~&)I_DXG=W-r!5FzKA5gmN
zs>d;nlr%)2#jainwY!SWwoFYelSb9k>wDF!iO$QFC1;>tyxy;Pz>q_A|EK6@>W}M|
z!eX^u`#B#*2b~qaqg7?qM`|#v5eQ1Uw2Qu7r!w$RmGaX*Ssu!5VGeqTY@0;&Hr~*>
zCqq=2xkzZ6ma}|FRy%@c1!w$m#MNl>&jI7FK1L2oYCK+0bMnjzyse#1G{B{DW>Pfg
zbK{{y!Pds!tRJB$SNbO@u4MW3!a24u?Qcvvx{TKflOuI581$;c>r5gJ<fQWR0u_(6
z(HBPyhEv-%O<g=&UM8Zy=(fYRZDD(I95G{I7CvAqbDJNf=C;-98$?*VNcZW{qST>n
zs3skX$XFb+CQB?*1z()1l7!LMXK}W-kO<G)(Qa{}^z|KtuP|~9ZQjA>ds#HIixDPl
z&ooi5>UbejiTUxlJ4x#3o#u{({RRuVr9sPbap!dM?B$7XWPKULn5LG~AKNT!rW9W$
zbX^Pn<<a)s&Nk+|K<rl!l+#em)st_=dZZybdncCejD3Aliws#CS`24K0{ZVsc8HwF
zCnFW_lWzTtOEe#UFhKu>%xkOlaTpqLL2PNXW|3^@VM~FlIa+gLs=>GMuPBOhUkd_j
zS9ONnP8mX6+|+4tl<cG&%g>|D6VZCTulq=6rt;4g<uZuyiTV{SMT=ulNvb}@*fmLe
z7Fa~|luFUKUnc{5_I^r$NG?6!q5n2lo`q}m{9vk*HQp`CLfi$fhx^RpRLtJ;rjK^|
zRc;Z7e(}|CHn&fH(5LWy%=auK#Ds8?e!6LTrZVc}!5d@tHtv$=KJpePlnO&ya>0oq
zG~chctG_<p)-Vy^PApm$uT;NFcfABlnB+$S{Z1;I)oX)WB<?KtW+zMqytLm0_1YL{
zJ*??Bk)lXwJIH0h_3M@&N6Ih2H!prRAb)UZ7}g@F{hn~Zg#P~HZ*LCwUCRv93L>mX
zR%BxpHjMMgX5-HZI;2EjVj&Nhb!7TqYozSWAvQCvK>tP;U!s;Qpefh5M{`HzmTLQ(
z*L)fWhcyZeIX|pFPx#d94Oi3Pl?lC|Y7Dc*S7!EKHDGQaz*0KW(Jos?kW35W%DWsF
zk6$63If-xE=cKZq7wKsGdO`JmCibO}aFod_$1Nnqi8=cT1SdyU-S>XQ)v}cc<0H-~
z-|mVS#zfR<5AB+idA^pM@?P|eP5eWd+)z)xCIhJ-gN2RRPld!5tk$_t#~%byl%Y)(
z<cVSs|0om9BT>pP%)aY!s5}2+>mk)+xpx?6q1Cl{r%I+5sRY$aW{U54CipK(n?~>C
zlNsCjSJHf0^FZ<oj?E>>7^OYouEN4rAfaG=!-RZktDQZcdvr{jg(2=KYtD7J*f6A0
zWQcrv`(eindb(g4^+ACwwNG!#aKuV7W><aZI1NsAP(ZQL@iGr1*Y@YjJHfF7_k`Ys
zkeXalZZ9_bKg=VGHcK2evFGaVPijBsZ0kuoA=1&FYO;S>-uT(zf}Zos(PF2|@L?di
z8}CwGp~0KaGDhVn_&a7U>7)YWzWE#1CcojSQ>QAhAJXpTg~~L4vwv@uo9(E5k8VCI
z`ou-&rcWX@Iictd62?LO-4DGhi3*`<W)a&FEnj35<1zdFcGcC!qDiSGcL}jNE2AG5
zIu|fT)lQzsOdh&eHjuhtnFhSjQ#XxqkvHhHt%zC9{5XJtx|K1PC;2Hv%m5KJq*6xD
z80SbS@`vtM{l&r-YE2Fsh)y6nf#~Ef7M;NL7P#K>yDnwI$;>}=G6PEzmLx36ztrt+
zSdy?LVM)T0{8xTLg%BAPvF?FZzATY4y)g02W5x0(YD-jHolCIpbspXHTWRwWW%H$-
z&L4AU@fqaqlF{-YDnBG2reO1)lW_Eso1ja48NAi1-{U$jJ>r{XC7f1NbKv1xSHa~u
zEFu};vG+q7Bk$cpA#oTVf(O?n8sp9C!KbI34po)&=qvm#EqyxZb8Z6OThX=F*Q9@r
zEYfBh#D#wSP}m_Xo%H-!z-F?Ij%P^Y+=|e{qtKLfiZ(0-Pwx(MZ<f1e`Dub|H}HC1
zNd@b6ZQdFXqLJqLg1PHBKf1+G`^j553!9SPwFJ3BzBXgevZYt!wIwAV8I4cc|MRi4
zzgR5tCqET6R8+WG2{8=BFc8B)4D+x3yd`wx(2+w&4juWwaz_p|3~Jc#tzp<$mj80E
zak!(-E__(}C^x3!<R;0Db%&GdOPl%pJHcKb$Bwv^KA^jYGPsW=71uLusF`4fzp1L`
zzfURDBt~`kHt#sUCu+`bFHwobslj$A&E&SI-uuz<VT`yCjxmP=UGbH~LT-=62n6p-
z#U_r0H#ZbKE{Dh>UtYW2Qye?Qm974!hnObaQi|1MKjjqXs|~U7mOJzGY~Vv;GF~3b
zMv{H@*9O>mp;9>CyGy0bhV8%e(2=pH_TGEAN<mzGI(2^M=DUc70&#ZXPun6_ML1Ze
zPnbtsU;D965MoQ*<hxfrCS-rJl8exBHiHMZRXrf)Gwqpy#ZNsOC%G#jf~0Fs)Q$&4
z2R}#OJqRb@Q<1rj)nO*H|DiI?MM_BdG)$2jN8ZY}cqKBV7UM&_B^z}JuDwGpU++cb
z4YR_^rkV2E2d)8<Qe<=1ZzN@3$PEuVcBD^*1geY_mIyN!l=hC(cIuK3FqMAP2~iK~
zvb3Au<s;nl{#37XbyN`T)9N)}1wW;YtL#J#S49|z9M*5^Y~S6d|Ajku`z4mgId5?2
zYi`+VEN%nO4pw#~Gs-&a8djtjg8ZEC&%P;k+PR`mcBLOV?C4R_ro8R<2VT5d!xAq0
zYH`dj2wNSG)NiJQlTs8F5fq}SG$U?1EibHhOGIs;t*7fyhse>LJ<TPKZX#&?nbCql
zF|F$)8GX`<#m@KC@#}VTw17@~<LiOy3aMb7{3+6yAKlO0rFFf+<8YAM67T}5%6(g<
zNV9R?Moy;^h^j~gdC-ybzD@i*_Nj&Kq0Vb+mV4D$ymlmkv4}+9ua0D&GG}S(XH^|h
zr(U7Om6!T|_m?G`nOm9~GCF;^!S>!>;@gsGdRpcLM;=fAoHpXTW-kc;oZ&y;VD#gi
zKv&O-+O;3Q^l*!Vo=oWAap&lU|By}d^)YcVj`!9#(~*dKl;)0>cOekNesBD+ZNz`H
zUAH}Dm_0#Ks8{*%3qqCg%E(&O#H@otdijrjLc|5e%7v|L<U-}OUvt{<1y1w#lMM;z
zGV0kMjNTttHFz*&C#tNH9c>;=fA!sveP{JVB-1;^2ve!}iL^t?b9})8&(D!Ks`E9$
z<?#hmji!l)#KOvjY8PxvWrA`i(^yiTHKB+f9}ATkWvgB(aC@TJ*(VA`B4VT7N`C%%
zeGnPR=&5A56%vQmcJjLwR8H~0>867tWFvbbLXBzb;rfWhJ5DEZ+CJs|^=_9VrV+M-
z!bZ{xAK%ey(=nn3pzgSoPcCHo;w@d{Zcv~3nyYX*v5NCifhEQ537*-~gY^(`ccOcD
zIY%D7j}j>i#!@!S>>%6XCiT*Mlt1vkAR+hbI=iN+(p$m9l`HbBCY(uER((u1-8^n~
zUNd>DeZ_eB7J<#Q?%s8*&cH>5po+e^)?+?H+e17hrl;j3OQvV0?Pk(9295X}k0lwa
z>wjT<(8wd78Qi=k6?w^RtE6z*w}~C`{vd*Ycu6e4G;JfD?d6NGOtj;9bwifM8IJ3i
zs*Y4;mn50}$$7IKap=yi{BDjYMMsf#!3|E26;R8iY?wdWRFgHvZx5%tr<EGUZvP^(
zLLj|86uyfo;4Fm9iNl2d%`T~clH0eDw1mvcHa#djd5=XY{-EWUxLKp}$Zx}|?R{ea
zmMP!8n+qgat#6(bx48$|A_j`E{2A9@BRwKO_RLTs_V#~eEws{^<&gU`BC4C{Zf#bS
zXHnvEAcHlTdwx7ovGrO?kr-Bm>#^w4kNZjrw;KvR+_1>)*#0WFdWO~IDs#wWkAgaq
zEA{|8MmEDX#iUplo9@AB(h?!wP;|9P3Iby=CTZ}p!^dmbU?p1YL})3qPiNB4<QkT5
z<4f}S7N^Evp6}_sIA*DT4jbr^iFNK{;0n-w{Ss=;wSJCqBPCzZFxpMSn)>*2c+T7!
zRULs_MA;)&rmWc%65)KjCbCS0w?^L@bq*`<$bJZ7!~7W0gflA<wUs;e{OIGu*K|g%
z7GodBcek@@qy=m^#rc{fg9-SpPiCCkQ@EX8YwTEF?OwGPqt(BE!C@|U5R&Z@85wvh
zH>67nZH-Q(tosU@PxLQKTR~R^`<&slkoqPxtgWhCDsL+fe{CC?d?(_{Gz4+;Y4ddS
z<*f1na((}kZMQXR#Z+p{^w2%@=+w_&odrXT(e&Qfb>2;jFnX{Rb@jN4VnsISj1gUa
zNPbT{Je)hDXnlE@m$R~6SVufuY$4$6E;e6;SD>D?kQS1V8_8Ctf>#5LrF#%noG!X=
zqsQsE(^^h6K`$c8B->DuQ%^oVd=Et^P?Kjg$XUnFu$$F!|81SMj=_~_YZ0oHhdnii
zvs>5csq(@(Oq^ss#gsVwSZN@8_+Hg*PmSq{kQR?5*#?=D0DlLQn~-v7t?*;t^Ck9#
zPeS$5+GZ0BL3{@4%oI1t>q%VAZuwI4JQJEF$e<h7tqCQ&RbmyXM?FqUSK&-PzSwe^
zirIW(cBrSsm`eS9@Y&EZ_wWNHLslj#jU(IiD^COT8~k3{-ubRtNLt-i!r+Na!J11$
zn9TUgXx3@;3pWOd)tf+!<XiOi(~jE4)v;B5f}EQ<=M8O3RHxZa_(={Huk8*M$ZBpa
zMn|5zt83Z)dL)f%$DO%Ea%!es%8KVPDR8Gf^kg#<N#x<G=z=vbLk8FN&<@i8OxNA9
zfO5m~xrAk|yjrmuE;EPjajCVhX`L(VD4gy_ri0>n_c6K^UM}dHifnI|r+@7VV&H$#
zp?oEnqn}qgIo#F$25BO?W4IX?RoRwR-=>xo18=#|H;cjROGU1mO2SvzGWb(1_pyIj
z2Tf})CLal&eH2_VaV*b4N+s`LksYTRqt|7|ptAq@W}sn5?^$~?&LfjprLThqW2e(~
zj91xSBtIn{Nj<z%`8;F#@$0;YJkomiM-y&yc=ZnmiOsm>p4mi6<l?vDBJtg?9vPw^
z$P~S_M|2Ywu)39OqHNU66<vB1v)d%A=1|PNOm1zC?iHRl+rVt+yKAB6LqtF&NuPNd
zb3Is4ah=uKL3sIg<ar8ZN!}buB@*G1M^oO-3x2kzPmT|;R&ji7>(zGWx0E8xMRLDa
z20gcx%wyZ)dD-P!|H!R^5{0~6`F*A98R^|4R(q3*B22A!gNegGchI*83;Zq^4qqXQ
zADpw;^0^PmJR{)9dbxKVXuaEh=D-?`zRxO)S=k!r>tH}3qdC}CG*FI35rS`ZbB9u>
z=L^4PbZ$^8U)Hy}I<zat+uE{HZr=LV-HSe$oFkJdyicYSmO052Ree51FOrk=)6Nqs
zDyG?^2(6!C4{>*Q(5qLYIS$H4m0ggl#4Dc^I#e6r>MAq%&rHeeky&vGn<Y*sQQtQb
zEeWjOxwdbr9Xjy}M^u5L<jD-1vq&7BImz+!AT(75lttFA><h|QOxN}#xQ6Bg9EIj7
z$$K6@?#_&Rnoem{jrG)Sbd8On1DD)J?MRDsjjO58;(o=(h5VUFZpw>WO*(?DYZCQV
zWL$YyzXXu-F<((D3ct7Wp#devc3*x==B>w6`PJxXhtR2OS5GkL^evb3EavH_`MK`3
zy7gN>dU6|29Wj{fpw%mTdi3O!+yw*kZF*~jqF#Zmt{q{GJw~Lh@r(0gbH3y!t@BrU
zX)9BW#@2i^P8DAU^!lD8Bxn#=?aef~KVA(TEiaQxIL}HKV|Ht?=07W8qFM@WDp83K
zxb84_%R)`+h#TjQ$Ro2C-8x7=u2?poA0%$mDuil%)Zy4b;#s(zZz`RQT<4p_Vy_~6
zDM7uf7!!KBSi`!6Yd*NY{8Ks=Z!9edx1?AV3BkR2Glh<!aC4-AAhEwDk-{#MtjgtL
zr+pt=nK$^Q=NH?t-7ikLF8h1V+wm=^s^9mXGG`EU+X{VA=j+MxVoNwQ>Q<{<ujM&a
zdgL%Y|A`21H1OuPbp-92X^hzj+b@%U<Sl>A92nJbssN`7aH;^O3UI3MZ}r+sh%h0-
z{Jjw-%rKZ?FvH-W4-WeNjozjWMIVYj6n!ZAQ1qeb|9#&R`S-fm4Nq3Wla+tQYn`DF
zgFX!UFzCae5BoRj!v>Hz|M5$s#iBRH<Y}YszNjO)J*3swxML;qDw)>?<sKQK|DX2}
zv;W^$=YB_*c$AGN5(vlNaQqF&-*Eg5$KP=L{r7$X^>=JWfA!B~0x16=DDjcl|M6z5
ztX(;&iJ*NrKCCpi+=xQ`#vIo;f7BzF`~I&{?fRN0SHemX*)G}FQ~tMnqqUJk7+SGE
z^4(kr5+O*0An`BwJ!#k||KLUm0|o;IQ8h%>5LN#hMb%LBq3A==hoTQfABsNQ6Z>1n
zLc9a<4#YcuyDl`34_@(cq2>J(j$mL(!jgm~2}@G)WL6Vy3&3pwxGjKN@VC_fc1hSJ
zVVC^dbpeg*A6yqOU@&06w~)XLgBb=hY?tBBeC7hKxt@HODmg#XU~%cE(5C`RwmB%v
zf+fL{V9Ec1Y5@5DgB=YF7!26&Z2@71!3={L26-5C`q1ff**T;`rw^SzboyKNe|9y1
zJ`DOW=)<57gFfufuDo!}_6I933>Xa9@7=0khQSPj83rfc{=TgiopeB>KUfW5z+k|B
zZ~2571~Uw17^D@DRzO+-r$(3Yj52U)1gA!DY9tv=h;xY-3%97@7Ii1xKQYDoC;n@o
z5zq)|1T+E~0gZr0K%@Vm2kD@#fVKkK3TP{!t$?-y+6rhZpsj$nY~udi?`VfS><@O2
zFkmoXzqj3n83r>9W*FRYg*&dyHeaiSe*VE~0C^Y;*zYZ$FvDPm!3=}M6%toSTp@9V
z#1#@(NL(Rteek=!iV1nxA8fZ_z+k|BuTNlx!3={L1`#zx)DTfO43smF8vNdB0Ez1#
z+;3pOV8CEWLPQM_HAK`9QA0!x5j8~A5K%)!-HM*1oOCf!HX4l!uVL75*37?nU;3Pj
zJfk)ec1hSJVV8tm5_U=0C1IC@S1ZG{bI9}yAV7ow5duV@5zq)|1T+E~0gZr0KqH_L
z&<JQm1vCO00gZr0KqH_L&<JP*Gy)m{jetf#BcPEz&<JP*Gy)m{jetf#BcKt`2xtT}
z0vZ90fJX4FQ9sZKXaqC@8Uc-fMnEH=5zq)|1T+E~0gZr08bBkU5zq)|1T+E~0gZr0
zKqH_L&<JP*Gy)p&0gZr0KqH_L&<JP*Gy)m{jetf#BcKt`2xwFYGy)m{jetf#BcKt`
z2xtT}0vZ90fJQ(gppgd92xtT}0vZ90fJQ(gpb^jrXaqC@8Uc-fMr1%Epb^jrXaqC@
z8Uc-fMnEH=5zq)|1T+E~bpnlmMnEH=5zq)|1T+E~0gZr0KqH_L&<JQG1T+E~0gZr0
zKqH_L&<JP*Gy)m{jetf#BcKr}&<JP*Gy)m{jetf#BcKt`2xtT}0vZ90fJV4LBcKt`
z2xtT}0vZ90fJQ(gpb^jrXaqC@8tDU#fJQ(gpb^jrXaqC@8Uc-fMnEH=5zq)|v;{N*
z8Uc-fMnEH=5zq)|1T+E~0gZr0KqH`0Ezk&P1T+E~0gZr0KqH_L&<JP*Gy)m{jethi
zfJQ(gpb^jrXaqC@8Uc-fMnEH=5zq)|1T;DW8Uc-fMnEH=5zq)|1T+E~0gZr0KqH_L
z(C8J=2xtT}0vZ90fJQ(gpb^jrXaqC@8Uc-fMn*s*pb^jrXaqC@8Uc-fMnEH=5zq)|
z1T+E~SptoKMnEH=5zq)|1T+E~0gZr0KqH_L&<JRB1T+E~0gZr0KqH_L&<JP*Gy)m{
zjetf#BcPE5&<JP*Gy)m{jetf#BcKt`2xtT}0vZ90fJW6oBcKt`2xtT}0vZ90fJQ(g
zpb^jrXaqC@8ZiKkfJQ(gpb^jrXaqC@8Uc-fMnEH=5zy#Qp^@)27J>w^;jEc|@xJsq
z7kS2i^<k1#z7jtWh~6*HJ5mhg$`D9RzL-A$`G<-?_%qw15RnLRq9FeB4^>z1KLA`Q
B&L#i=

literal 0
HcmV?d00001

diff --git a/tests/fsck-tests/020-bad-free-space-cache/test.sh b/tests/fsck-tests/020-bad-free-space-cache/test.sh
new file mode 100755
index 0000000..8524d2a
--- /dev/null
+++ b/tests/fsck-tests/020-bad-free-space-cache/test.sh
@@ -0,0 +1,16 @@
+#!/bin/bash
+
+source $TOP/tests/common
+
+check_prereq btrfs
+
+image=$(extract_image "./default_case.raw.xz")
+run_check_stdout $TOP/btrfs check "$image" |\
+	grep -q "csum mismatch on free space cache" ||\
+	_fail "btrfs check didn't detect free space cache error"
+run_check $TOP/btrfs check --clear-space-cache "$image"
+run_check_stdout $TOP/btrfs check "$image" 2>&1 |
+	grep -q "csum mismatch on free space cache" &&
+	_fail "unexpected error message in the output"
+
+rm -f "$image"
-- 
2.8.2




^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v1.1 3/4] btrfs-progs: fsck: Add support to clear free space cache
  2016-05-17  3:47 ` [PATCH 3/4] btrfs-progs: fsck: Add support to clear free space cache Qu Wenruo
@ 2016-05-17  3:53   ` Qu Wenruo
  0 siblings, 0 replies; 19+ messages in thread
From: Qu Wenruo @ 2016-05-17  3:53 UTC (permalink / raw)
  To: linux-btrfs; +Cc: dsterba

Add a new option "--clear-space-cache" for btrfs check.

Unlike many may assume, kernel "clear_cache" will only rebuild *SOME* of
the free space cache, not *ALL*.

Or more specifically, it will only rebuild free space cache for block
groups that has read out the cache during "clear_cache" mount time.

And since kernel will not read out free space cache at mount, but only
when kernel needs to allocate space from the block group, so bad
free space cache will stay untouched for a long time.

Such kernel design is quite good, as it dramatically reduce the mount
time for large fs, so I prefer not to change that.

Instead, since a lot of user consider free space warning from kernel as
an error, we should handle them like an error, which means we should use
btrfsck to fix it, even it's harmless most of time.

This patch will use "--clear-space-cache" to clear all free space
cache, and modify cache_generation to -1.

Reported-by: Ivan P <chrnosphered@gmail.com>
Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
v1.1:
   Fix error that when free space cache inode is not found, we still
   remove one item from tree root.
---
 Documentation/btrfs-check.asciidoc |   8 +++
 cmds-check.c                       |  58 ++++++++++++++++-
 free-space-cache.c                 | 126 +++++++++++++++++++++++++++++++++++++
 free-space-cache.h                 |   4 ++
 4 files changed, 195 insertions(+), 1 deletion(-)

diff --git a/Documentation/btrfs-check.asciidoc b/Documentation/btrfs-check.asciidoc
index 74a2ad2..ea25582 100644
--- a/Documentation/btrfs-check.asciidoc
+++ b/Documentation/btrfs-check.asciidoc
@@ -78,6 +78,14 @@ respective superblock offset is within the device size
 This can be used to use a different starting point if some of the primary
 superblock is damaged.
 
+--clear-space-cache::
+clear all free space cache
++
+NOTE:
+Kernel mount option 'clear_cache' is only designed to rebuild free space cache
+which is modified during the lifetime of that mount option.
+It doesn't rebuild all free space cache, nor clear them out.
+
 DANGEROUS OPTIONS
 -----------------
 
diff --git a/cmds-check.c b/cmds-check.c
index ec0bbfd..1f6aefb 100644
--- a/cmds-check.c
+++ b/cmds-check.c
@@ -9524,10 +9524,41 @@ const char * const cmd_check_usage[] = {
 	"                            print subvolume extents and sharing state",
 	"-r|--tree-root <bytenr>     use the given bytenr for the tree root",
 	"--chunk-root <bytenr>       use the given bytenr for the chunk tree root",
+	"--clear-space-cache         clear all free space cache(v1)",
 	"-p|--progress               indicate progress",
 	NULL
 };
 
+static int clear_free_space_cache(struct btrfs_fs_info *fs_info)
+{
+	struct btrfs_trans_handle *trans;
+	struct btrfs_block_group_cache *bg_cache;
+	u64 current = 0;
+	int ret = 0;
+
+	/* Clear all free space cache inodes and its extent data */
+	while (1) {
+		bg_cache = btrfs_lookup_first_block_group(fs_info, current);
+		if (!bg_cache)
+			break;
+		ret = btrfs_clear_free_space_cache(fs_info, bg_cache);
+		if (ret < 0)
+			return ret;
+		current = bg_cache->key.objectid + bg_cache->key.offset;
+	}
+
+	/* Don't forget to set cache_generation to -1 */
+	trans = btrfs_start_transaction(fs_info->tree_root, 0);
+	if (IS_ERR(trans)) {
+		error("failed to update super block cache generation");
+		return PTR_ERR(trans);
+	}
+	btrfs_set_super_cache_generation(fs_info->super_copy, (u64)-1);
+	btrfs_commit_transaction(trans, fs_info->tree_root);
+	
+	return ret;
+}
+
 int cmd_check(int argc, char **argv)
 {
 	struct cache_tree root_cache;
@@ -9543,13 +9574,15 @@ int cmd_check(int argc, char **argv)
 	int init_csum_tree = 0;
 	int readonly = 0;
 	int qgroup_report = 0;
+	int clear_space_cache = 0;
 	enum btrfs_open_ctree_flags ctree_flags = OPEN_CTREE_EXCLUSIVE;
 
 	while(1) {
 		int c;
 		enum { GETOPT_VAL_REPAIR = 257, GETOPT_VAL_INIT_CSUM,
 			GETOPT_VAL_INIT_EXTENT, GETOPT_VAL_CHECK_CSUM,
-			GETOPT_VAL_READONLY, GETOPT_VAL_CHUNK_TREE };
+			GETOPT_VAL_READONLY, GETOPT_VAL_CHUNK_TREE, 
+			GETOPT_VAL_CLEAR_SPACE_CACHE};
 		static const struct option long_options[] = {
 			{ "super", required_argument, NULL, 's' },
 			{ "repair", no_argument, NULL, GETOPT_VAL_REPAIR },
@@ -9566,6 +9599,8 @@ int cmd_check(int argc, char **argv)
 			{ "tree-root", required_argument, NULL, 'r' },
 			{ "chunk-root", required_argument, NULL,
 				GETOPT_VAL_CHUNK_TREE },
+			{ "clear-space-cache", no_argument, NULL,
+				GETOPT_VAL_CLEAR_SPACE_CACHE},
 			{ "progress", no_argument, NULL, 'p' },
 			{ NULL, 0, NULL, 0}
 		};
@@ -9631,6 +9666,11 @@ int cmd_check(int argc, char **argv)
 			case GETOPT_VAL_CHECK_CSUM:
 				check_data_csum = 1;
 				break;
+			case GETOPT_VAL_CLEAR_SPACE_CACHE:
+				clear_space_cache = 1;
+				repair = 1;
+				ctree_flags |= OPEN_CTREE_WRITES;
+				break;
 		}
 	}
 
@@ -9693,6 +9733,22 @@ int cmd_check(int argc, char **argv)
 	}
 
 	uuid_unparse(info->super_copy->fsid, uuidbuf);
+	if (clear_space_cache) {
+		/* Basic check, don't support v2 free space cache yet */
+		if (btrfs_fs_compat_ro(info,
+				BTRFS_FEATURE_COMPAT_RO_FREE_SPACE_TREE)) {
+			error("doesn't support space cache tree(v2) yet");
+			ret = -ENOTTY;
+			goto close_out;
+		}
+		printf("Clearing all free space cache\n");
+		ret = clear_free_space_cache(info);
+		if (ret)
+			error("failed to clear free space cache");
+		else
+			printf("Free space cache cleared\n");
+		goto close_out;
+	}
 	if (qgroup_report) {
 		printf("Print quota groups for %s\nUUID: %s\n", argv[optind],
 		       uuidbuf);
diff --git a/free-space-cache.c b/free-space-cache.c
index 357d69e..8cff23b 100644
--- a/free-space-cache.c
+++ b/free-space-cache.c
@@ -25,6 +25,7 @@
 #include "crc32c.h"
 #include "bitops.h"
 #include "internal.h"
+#include "utils.h"
 
 /*
  * Kernel always uses PAGE_CACHE_SIZE for sectorsize, but we don't have
@@ -877,3 +878,128 @@ next:
 		prev = e;
 	}
 }
+
+int btrfs_clear_free_space_cache(struct btrfs_fs_info *fs_info,
+				 struct btrfs_block_group_cache *bg)
+{
+	struct btrfs_trans_handle *trans;
+	struct btrfs_root *tree_root = fs_info->tree_root;
+	struct btrfs_path path;
+	struct btrfs_key key;
+	struct btrfs_disk_key location;
+	struct btrfs_free_space_header *sc_header;
+	struct extent_buffer *node;
+	u64 ino;
+	int slot;
+	int ret;
+
+	trans = btrfs_start_transaction(tree_root, 1);
+	if (IS_ERR(trans))
+		return PTR_ERR(trans);
+
+	btrfs_init_path(&path);
+
+	key.objectid = BTRFS_FREE_SPACE_OBJECTID;
+	key.type = 0;
+	key.offset = bg->key.objectid;
+
+	ret = btrfs_search_slot(trans, tree_root, &key, &path, -1, 1);
+	if (ret > 0) {
+		ret = 0;
+		goto out;
+	}
+	if (ret < 0)
+		goto out;
+
+	node = path.nodes[0];
+	slot = path.slots[0];
+	sc_header = btrfs_item_ptr(node, slot, struct btrfs_free_space_header);
+	btrfs_free_space_key(node, sc_header, &location);
+	ino = location.objectid;
+
+	/* Delete the free space header, as we have the ino to continue */
+	ret = btrfs_del_item(trans, tree_root, &path);
+	if (ret < 0) {
+		error("failed to remove free space header for block group %llu",
+		      bg->key.objectid);
+		goto out;
+	}
+	btrfs_release_path(&path);
+
+	/* Iterate from the end of the free space cache inode */
+	key.objectid = ino;
+	key.type = BTRFS_EXTENT_DATA_KEY;
+	key.offset = (u64)-1;
+	ret = btrfs_search_slot(trans, tree_root, &key, &path, -1, 1);
+	if (ret < 0) {
+		error("failed to locate free space cache extent for block group %llu",
+		      bg->key.objectid);
+		goto out;
+	}
+	while (1) {
+		struct btrfs_file_extent_item *fi;
+		u64 disk_bytenr;
+		u64 disk_num_bytes;
+
+
+		ret = btrfs_previous_item(tree_root, &path, ino,
+					  BTRFS_EXTENT_DATA_KEY);
+		if (ret > 0) {
+			ret = 0;
+			break;
+		}
+		if (ret < 0) {
+			error("failed to locate free space cache extent for block group %llu",
+			      bg->key.objectid);
+			goto out;
+		}
+		node = path.nodes[0];
+		slot = path.slots[0];
+		btrfs_item_key_to_cpu(node, &key, slot);
+		fi = btrfs_item_ptr(node, slot, struct btrfs_file_extent_item);
+		disk_bytenr = btrfs_file_extent_disk_bytenr(node, fi);
+		disk_num_bytes = btrfs_file_extent_disk_num_bytes(node, fi);
+
+		ret = btrfs_free_extent(trans, tree_root, disk_bytenr,
+					disk_num_bytes, 0, tree_root->objectid,
+					ino, key.offset);
+		if (ret < 0) {
+			error("failed to remove backref for disk bytenr %llu",
+			      disk_bytenr);
+			goto out;
+		}
+		ret = btrfs_del_item(trans, tree_root, &path);
+		if (ret < 0) {
+			error("failed to remove free space extent data for ino %llu offset %llu",
+			      ino, key.offset);
+			goto out;
+		}
+	}
+	btrfs_release_path(&path);
+
+	/* Now delete free space cache inode item */
+	key.objectid = ino;
+	key.type = BTRFS_INODE_ITEM_KEY;
+	key.offset = 0;
+
+	ret = btrfs_search_slot(trans, tree_root, &key, &path, -1, 1);
+	if (ret > 0) {
+		warning("free space inode %llu not found, ignore", ino);
+		goto out;
+	}
+	if (ret < 0) {
+		error("failed to locate free space cache inode %llu for block group %llu",
+		      ino, bg->key.objectid);
+		goto out;
+	}
+	ret = btrfs_del_item(trans, tree_root, &path);
+	if (ret < 0) {
+		error("failed to delete free space cache inode %llu for block group %llu",
+		      ino, bg->key.objectid);
+	}
+out:
+	btrfs_release_path(&path);
+	if (!ret)
+		btrfs_commit_transaction(trans, tree_root);
+	return ret;
+}
diff --git a/free-space-cache.h b/free-space-cache.h
index 9214077..90302ac 100644
--- a/free-space-cache.h
+++ b/free-space-cache.h
@@ -59,4 +59,8 @@ void unlink_free_space(struct btrfs_free_space_ctl *ctl,
 		       struct btrfs_free_space *info);
 int btrfs_add_free_space(struct btrfs_free_space_ctl *ctl, u64 offset,
 			 u64 bytes);
+
+/* Used for clearing one free space cache for given block group */
+int btrfs_clear_free_space_cache(struct btrfs_fs_info *fs_info,
+				 struct btrfs_block_group_cache *bg);
 #endif
-- 
2.8.2




^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/4] Add support to clear v1 free space cache for btrfs check
  2016-05-17  3:47 [PATCH 0/4] Add support to clear v1 free space cache for btrfs check Qu Wenruo
                   ` (3 preceding siblings ...)
  2016-05-17  3:47 ` [PATCH 4/4] btrfs-progs: tests: add 020-bad-free-space-cache Qu Wenruo
@ 2016-05-17  3:56 ` Qu Wenruo
  2016-05-17 18:12   ` Ivan P
  4 siblings, 1 reply; 19+ messages in thread
From: Qu Wenruo @ 2016-05-17  3:56 UTC (permalink / raw)
  To: linux-btrfs, Ivan P; +Cc: dsterba

Also to Ivan P, the initial reporter of the problem.

Now "btrfsck --clear-cache" should help you to prevent kernel warning on 
free space cache problem.

Thanks,
Qu

Qu Wenruo wrote on 2016/05/17 11:47 +0800:
> The patchset can be also fetched from github, just in case mail list
> blocks the last patch, which contains binary file.
> https://github.com/adam900710/btrfs-progs.git fsck_clear_cache
>
> Just as describe in the first patch, btrfs kernel "clear_cache" mount
> option can't rebuild all free space cache, due to the design of free
> space cache.
> (Although I pretty like the idea to delay the load of free space cache,
> as it hugely reduce the mount time of large fs)
>
> So this makes users to report error in mail list, complaining
> "clear_cache" doesn't wipe the annoying kernel warning on corrupted free
> space cache.
>
> Since kernel can't handle it, and user consider it more like an error,
> we'd better handle it like an error, to fix it in btrfs check.
>
> So this patchset adds the ability to clear free space cache, and add
> test case for it.
>
> The clear procedure is different from kernel, it will remove all free
> space cache inodes and its contents(with backrefs), and set
> cache_generation of super block to -1.
> So there will be no free space cache at all and kernel will be happy
> with that.
>
> This patch also enhances btrfs_previous_item() to use min_objectid to
> return as early as possible.
>
> Lu Fengqi (1):
>   btrfs-progs: tests: add 020-bad-free-space-cache
>
> Qu Wenruo (3):
>   btrfs-progs: corrupt-block: Add ability to corrupt free space cache
>     file
>   btrfs-progs: ctree: return earlier for btrfs_previous_item
>   btrfs-progs: fsck: Add support to clear free space cache
>
>  Documentation/btrfs-check.asciidoc                 |   8 ++
>  btrfs-corrupt-block.c                              | 124 ++++++++++++++++++++-
>  cmds-check.c                                       |  58 +++++++++-
>  ctree.c                                            |   2 +
>  free-space-cache.c                                 | 124 +++++++++++++++++++++
>  free-space-cache.h                                 |   4 +
>  .../020-bad-free-space-cache/default_case.raw.xz   | Bin 0 -> 164068 bytes
>  tests/fsck-tests/020-bad-free-space-cache/test.sh  |  16 +++
>  8 files changed, 334 insertions(+), 2 deletions(-)
>  create mode 100644 tests/fsck-tests/020-bad-free-space-cache/default_case.raw.xz
>  create mode 100755 tests/fsck-tests/020-bad-free-space-cache/test.sh
>



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/4] btrfs-progs: ctree: return earlier for btrfs_previous_item
  2016-05-17  3:47 ` [PATCH 2/4] btrfs-progs: ctree: return earlier for btrfs_previous_item Qu Wenruo
@ 2016-05-17 12:50   ` David Sterba
  2016-05-18  0:40     ` Qu Wenruo
  0 siblings, 1 reply; 19+ messages in thread
From: David Sterba @ 2016-05-17 12:50 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: linux-btrfs, dsterba

On Tue, May 17, 2016 at 11:47:56AM +0800, Qu Wenruo wrote:
> Btrfs_previous_item() has a parameter to specify minimal objectid to
> return.
> 
> But surprisingly it doesn't use it at all.
> Although that's OK, but it would take years long for large tree, so
> return it earlier.

This makes it closer to the kernel implementation, but still not equal,
there are some more shortcuts. Can you please port them as well? This
patch is independent of the rest so I'd take it separatelly.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/4] btrfs-progs: corrupt-block: Add ability to corrupt free space cache file
  2016-05-17  3:47 ` [PATCH 1/4] btrfs-progs: corrupt-block: Add ability to corrupt free space cache file Qu Wenruo
@ 2016-05-17 13:00   ` David Sterba
  2016-05-17 13:02     ` David Sterba
  2016-05-18  0:49     ` Qu Wenruo
  0 siblings, 2 replies; 19+ messages in thread
From: David Sterba @ 2016-05-17 13:00 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: linux-btrfs, dsterba

On Tue, May 17, 2016 at 11:47:55AM +0800, Qu Wenruo wrote:
> 
> Btrfs-progs fix will follow soon.
> 
> Reported-by: Ivan P <chrnosphered@gmail.com>
> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
> ---
>  btrfs-corrupt-block.c | 124 +++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 123 insertions(+), 1 deletion(-)
> 
> diff --git a/btrfs-corrupt-block.c b/btrfs-corrupt-block.c
> index d331f96..eb27265 100644
> --- a/btrfs-corrupt-block.c
> +++ b/btrfs-corrupt-block.c
> @@ -115,6 +115,9 @@ static void print_usage(int ret)
>  	fprintf(stderr, "\t-C Delete a csum for the specified bytenr.  When "
>  		"used with -b it'll delete that many bytes, otherwise it's "
>  		"just sectorsize\n");
> +	fprintf(stderr, "\t-s <method>\n");

No single letter option, this is too specialized. Also we'd need to
specify the free space cache version to clear and support only v1 for
now.

> +	fprintf(stderr, "\t   Corrupt free space cache file(space_cache v1), must also specify -l for blockgroup bytenr\n");
> +	fprintf(stderr, "\t   <method> can be 'zero_gen','rand_gen' or 'content'\n");
>  	exit(ret);
>  }
>  
> @@ -1012,6 +1015,100 @@ out:
>  	return ret;
>  
>  }
> +
> +u64 rand_u64(void)
> +{
> +	u64 ret = 0;
> +	int n;
> +
> +	for (n = 0; n < sizeof(ret) / sizeof(RAND_MAX); n++)
> +		ret += rand() << (n * sizeof(RAND_MAX) * 8);
> +	return ret;
> +}

Please introduce the function in a separate patch and put it to utils.
The use of rand() in the code is quite random and coverity complains
about the wrong use, so I'd like to use this to a broader cleanup.

> +
> +#define CORRUPT_SC_FILE_ZERO_GEN	1
> +#define CORRUPT_SC_FILE_RAND_GEN	2
> +#define CORRUPT_SC_FILE_CONTENT		3
> +int corrupt_space_cache_file(struct btrfs_fs_info *fs_info, u64 block_group,
> +			     int method)
> +{
> +	struct btrfs_root *tree_root = fs_info->tree_root;
> +	struct btrfs_path path;
> +	struct btrfs_key key;
> +	struct btrfs_disk_key location;
> +	struct btrfs_free_space_header *sc_header;
> +	struct btrfs_file_extent_item *fi;
> +	struct extent_buffer *node;
> +	struct extent_buffer *corrupted_node;
> +	u64 disk_bytenr;
> +	int slot;
> +	int ret;
> +
> +	key.objectid = BTRFS_FREE_SPACE_OBJECTID;
> +	key.type = 0;
> +	key.offset = block_group;
> +
> +	btrfs_init_path(&path);
> +
> +	/* Don't start trans, as this will cause generation different */
> +	ret = btrfs_search_slot(NULL, tree_root, &key, &path, 0, 0);
> +	if (ret) {
> +		error("failed to find free space cahce file for block group %llu, ret: %d\n",
> +		      block_group, ret);
> +		goto out;
> +	}
> +	slot = path.slots[0];
> +	node = path.nodes[0];
> +	sc_header = btrfs_item_ptr(node, slot, struct btrfs_free_space_header);
> +	if (method == CORRUPT_SC_FILE_ZERO_GEN ||
> +	    method == CORRUPT_SC_FILE_RAND_GEN) {
> +		u64 dst_gen;
> +
> +		if (method == CORRUPT_SC_FILE_ZERO_GEN)
> +			dst_gen = 0;
> +		else
> +			dst_gen = rand_u64();
> +
> +		btrfs_set_free_space_generation(node, sc_header, dst_gen);
> +		/* Manually re-calc csum and write to disk */
> +		ret = write_tree_block(NULL, tree_root, node);
> +		goto out;
> +	}
> +
> +	btrfs_free_space_key(node, sc_header, &location);
> +	btrfs_disk_key_to_cpu(&key, &location);
> +	btrfs_release_path(&path);
> +
> +	/* Change to type and offset to search file extent */
> +	key.type = BTRFS_EXTENT_DATA_KEY;
> +	key.offset = 0;
> +
> +	ret = btrfs_search_slot(NULL, tree_root, &key, &path, 0, 0);
> +	if (ret) {
> +		error("failed to find free space cache extent data for blockgroup %llu, ret: %d\n",
> +		      block_group, ret);
> +		goto out;
> +	}
> +
> +	slot = path.slots[0];
> +	node = path.nodes[0];
> +	fi = btrfs_item_ptr(node, slot, struct btrfs_file_extent_item);
> +	disk_bytenr = btrfs_file_extent_disk_bytenr(node, fi);
> +
> +	/*
> +	 * Directly write random data into
> +	 * [disk_bytenr, disk_bytenr + sectorsize) as free space cache inode
> +	 * doesn't have csum
> +	 */
> +	corrupted_node = debug_corrupt_block(tree_root, disk_bytenr,
> +					     tree_root->sectorsize, 0);
> +	free_extent_buffer(corrupted_node);
> +
> +out:
> +	btrfs_release_path(&path);
> +	return ret;
> +}
> +
>  int main(int argc, char **argv)
>  {
>  	struct cache_tree root_cache;
> @@ -1032,6 +1129,7 @@ int main(int argc, char **argv)
>  	int corrupt_item = 0;
>  	int corrupt_di = 0;
>  	int delete = 0;
> +	int corrupt_sc_file = 0;

Why is it named _file?

>  	u64 metadata_block = 0;
>  	u64 inode = 0;
>  	u64 file_extent = (u64)-1;
> @@ -1065,11 +1163,12 @@ int main(int argc, char **argv)
>  			{ "delete", no_argument, NULL, 'd'},
>  			{ "root", no_argument, NULL, 'r'},
>  			{ "csum", required_argument, NULL, 'C'},
> +			{ "space-cache-file", required_argument, NULL, 's'},

"clear-free-space-cache" is quite long but more understandable IMHO

>  			{ "help", no_argument, NULL, GETOPT_VAL_HELP},
>  			{ NULL, 0, NULL, 0 }
>  		};
>  
> -		c = getopt_long(argc, argv, "l:c:b:eEkuUi:f:x:m:K:IDdr:C:",
> +		c = getopt_long(argc, argv, "l:c:b:eEkuUi:f:x:m:K:IDdr:C:s:",
>  				long_options, NULL);
>  		if (c < 0)
>  			break;
> @@ -1136,6 +1235,22 @@ int main(int argc, char **argv)
>  			case 'C':
>  				csum_bytenr = arg_strtou64(optarg);
>  				break;
> +			case 's':
> +				if (!strncmp(optarg, "zero_gen",
> +					     sizeof("zero_gen")))
> +					corrupt_sc_file =
> +						CORRUPT_SC_FILE_ZERO_GEN;
> +				else if (!strncmp(optarg, "rand_gen",
> +						  sizeof("rand_gen")))
> +					corrupt_sc_file =
> +						CORRUPT_SC_FILE_RAND_GEN;
> +				else if (!strncmp(optarg, "content",
> +						  sizeof("content")))
> +					corrupt_sc_file =
> +						CORRUPT_SC_FILE_CONTENT;
> +				else
> +					print_usage(1);
> +				break;
>  			case GETOPT_VAL_HELP:
>  			default:
>  				print_usage(c != GETOPT_VAL_HELP);
> @@ -1154,6 +1269,13 @@ int main(int argc, char **argv)
>  		fprintf(stderr, "Open ctree failed\n");
>  		exit(1);
>  	}
> +	if (corrupt_sc_file) {
> +		if (logical == (u64)-1)
> +			print_usage(1);
> +		ret = corrupt_space_cache_file(root->fs_info, logical,
> +					       corrupt_sc_file);
> +		goto out_close;
> +	}
>  	if (extent_rec) {
>  		struct btrfs_trans_handle *trans;
>  
> -- 
> 2.8.2
> 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/4] btrfs-progs: corrupt-block: Add ability to corrupt free space cache file
  2016-05-17 13:00   ` David Sterba
@ 2016-05-17 13:02     ` David Sterba
  2016-05-18  0:49     ` Qu Wenruo
  1 sibling, 0 replies; 19+ messages in thread
From: David Sterba @ 2016-05-17 13:02 UTC (permalink / raw)
  To: dsterba, Qu Wenruo, linux-btrfs

On Tue, May 17, 2016 at 03:00:04PM +0200, David Sterba wrote:
> > @@ -1065,11 +1163,12 @@ int main(int argc, char **argv)
> >  			{ "delete", no_argument, NULL, 'd'},
> >  			{ "root", no_argument, NULL, 'r'},
> >  			{ "csum", required_argument, NULL, 'C'},
> > +			{ "space-cache-file", required_argument, NULL, 's'},
> 
> "clear-free-space-cache" is quite long but more understandable IMHO

This is not the checker, so this would be ok as "space-cache" as it's
in the context of 'corrupt-block'.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/4] Add support to clear v1 free space cache for btrfs check
  2016-05-17  3:56 ` [PATCH 0/4] Add support to clear v1 free space cache for btrfs check Qu Wenruo
@ 2016-05-17 18:12   ` Ivan P
  2016-05-18  1:00     ` Qu Wenruo
  2016-05-18  9:14     ` David Sterba
  0 siblings, 2 replies; 19+ messages in thread
From: Ivan P @ 2016-05-17 18:12 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: btrfs, dsterba

Thank you, however I can't seem to be able to compile that snapshot, I'm getting

===========================
/usr/bin/install -c -m644 -d 64-btrfs-dm.rules
/home/myuser/aur/btrfs-progs-git/pkg/btrfs-progs-git/usr/lib/udev/rules.d
/usr/bin/install: cannot create directory ‘64-btrfs-dm.rules’: File exists
Makefile:400: recipe for target 'install' failed
make: *** [install] Error 1
==> ERROR: A failure occurred in package()
===========================

Just to make sure I wasn't screwing up somewhere, I tried the
btrfs-progs-git AUR package and I'm getting the same thing.
It's not only me, however: according to [1] it could be that this
commit has introduced it: [2]

Regards,
Ivan.

[1] https://aur.archlinux.org/packages/btrfs-progs-git/
[2] http://repo.or.cz/btrfs-progs-unstable/devel.git?a=commit;h=ebe5b1cc7885027521db3c1d16d84bd54cc1321b

On Tue, May 17, 2016 at 5:56 AM, Qu Wenruo <quwenruo@cn.fujitsu.com> wrote:
> Also to Ivan P, the initial reporter of the problem.
>
> Now "btrfsck --clear-cache" should help you to prevent kernel warning on
> free space cache problem.
>
> Thanks,
> Qu
>
> Qu Wenruo wrote on 2016/05/17 11:47 +0800:
>>
>> The patchset can be also fetched from github, just in case mail list
>> blocks the last patch, which contains binary file.
>> https://github.com/adam900710/btrfs-progs.git fsck_clear_cache
>>
>> Just as describe in the first patch, btrfs kernel "clear_cache" mount
>> option can't rebuild all free space cache, due to the design of free
>> space cache.
>> (Although I pretty like the idea to delay the load of free space cache,
>> as it hugely reduce the mount time of large fs)
>>
>> So this makes users to report error in mail list, complaining
>> "clear_cache" doesn't wipe the annoying kernel warning on corrupted free
>> space cache.
>>
>> Since kernel can't handle it, and user consider it more like an error,
>> we'd better handle it like an error, to fix it in btrfs check.
>>
>> So this patchset adds the ability to clear free space cache, and add
>> test case for it.
>>
>> The clear procedure is different from kernel, it will remove all free
>> space cache inodes and its contents(with backrefs), and set
>> cache_generation of super block to -1.
>> So there will be no free space cache at all and kernel will be happy
>> with that.
>>
>> This patch also enhances btrfs_previous_item() to use min_objectid to
>> return as early as possible.
>>
>> Lu Fengqi (1):
>>   btrfs-progs: tests: add 020-bad-free-space-cache
>>
>> Qu Wenruo (3):
>>   btrfs-progs: corrupt-block: Add ability to corrupt free space cache
>>     file
>>   btrfs-progs: ctree: return earlier for btrfs_previous_item
>>   btrfs-progs: fsck: Add support to clear free space cache
>>
>>  Documentation/btrfs-check.asciidoc                 |   8 ++
>>  btrfs-corrupt-block.c                              | 124
>> ++++++++++++++++++++-
>>  cmds-check.c                                       |  58 +++++++++-
>>  ctree.c                                            |   2 +
>>  free-space-cache.c                                 | 124
>> +++++++++++++++++++++
>>  free-space-cache.h                                 |   4 +
>>  .../020-bad-free-space-cache/default_case.raw.xz   | Bin 0 -> 164068
>> bytes
>>  tests/fsck-tests/020-bad-free-space-cache/test.sh  |  16 +++
>>  8 files changed, 334 insertions(+), 2 deletions(-)
>>  create mode 100644
>> tests/fsck-tests/020-bad-free-space-cache/default_case.raw.xz
>>  create mode 100755 tests/fsck-tests/020-bad-free-space-cache/test.sh
>>
>
>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/4] btrfs-progs: ctree: return earlier for btrfs_previous_item
  2016-05-17 12:50   ` David Sterba
@ 2016-05-18  0:40     ` Qu Wenruo
  0 siblings, 0 replies; 19+ messages in thread
From: Qu Wenruo @ 2016-05-18  0:40 UTC (permalink / raw)
  To: dsterba, linux-btrfs



David Sterba wrote on 2016/05/17 14:50 +0200:
> On Tue, May 17, 2016 at 11:47:56AM +0800, Qu Wenruo wrote:
>> Btrfs_previous_item() has a parameter to specify minimal objectid to
>> return.
>>
>> But surprisingly it doesn't use it at all.
>> Although that's OK, but it would take years long for large tree, so
>> return it earlier.
>
> This makes it closer to the kernel implementation, but still not equal,
> there are some more shortcuts. Can you please port them as well? This
> patch is independent of the rest so I'd take it separatelly.
>
>
Thanks, I just forgot that we can refer to kernel implement.

I'll submit it as a separate patch soon.

Thanks,
Qu



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/4] btrfs-progs: corrupt-block: Add ability to corrupt free space cache file
  2016-05-17 13:00   ` David Sterba
  2016-05-17 13:02     ` David Sterba
@ 2016-05-18  0:49     ` Qu Wenruo
  1 sibling, 0 replies; 19+ messages in thread
From: Qu Wenruo @ 2016-05-18  0:49 UTC (permalink / raw)
  To: dsterba, linux-btrfs



David Sterba wrote on 2016/05/17 15:00 +0200:
> On Tue, May 17, 2016 at 11:47:55AM +0800, Qu Wenruo wrote:
>>
>> Btrfs-progs fix will follow soon.
>>
>> Reported-by: Ivan P <chrnosphered@gmail.com>
>> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
>> ---
>>  btrfs-corrupt-block.c | 124 +++++++++++++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 123 insertions(+), 1 deletion(-)
>>
>> diff --git a/btrfs-corrupt-block.c b/btrfs-corrupt-block.c
>> index d331f96..eb27265 100644
>> --- a/btrfs-corrupt-block.c
>> +++ b/btrfs-corrupt-block.c
>> @@ -115,6 +115,9 @@ static void print_usage(int ret)
>>  	fprintf(stderr, "\t-C Delete a csum for the specified bytenr.  When "
>>  		"used with -b it'll delete that many bytes, otherwise it's "
>>  		"just sectorsize\n");
>> +	fprintf(stderr, "\t-s <method>\n");
>
> No single letter option, this is too specialized. Also we'd need to
> specify the free space cache version to clear and support only v1 for
> now.

Originally planned as 2 different options, clear-space-cache-file for v1 
and clear-space-cache-tree for v2.

But your advice is right, --space-cache v1|v2 is much better.
I'll use that method and move generation/content specification to -f 
options.

>
>> +	fprintf(stderr, "\t   Corrupt free space cache file(space_cache v1), must also specify -l for blockgroup bytenr\n");
>> +	fprintf(stderr, "\t   <method> can be 'zero_gen','rand_gen' or 'content'\n");
>>  	exit(ret);
>>  }
>>
>> @@ -1012,6 +1015,100 @@ out:
>>  	return ret;
>>
>>  }
>> +
>> +u64 rand_u64(void)
>> +{
>> +	u64 ret = 0;
>> +	int n;
>> +
>> +	for (n = 0; n < sizeof(ret) / sizeof(RAND_MAX); n++)
>> +		ret += rand() << (n * sizeof(RAND_MAX) * 8);
>> +	return ret;
>> +}
>
> Please introduce the function in a separate patch and put it to utils.
> The use of rand() in the code is quite random and coverity complains
> about the wrong use, so I'd like to use this to a broader cleanup.
>

OK, as I just forgot the static prefix and I think that's the reason 
compiler complains. (the only caller in main() has initialized the seed).

>> +
>> +#define CORRUPT_SC_FILE_ZERO_GEN	1
>> +#define CORRUPT_SC_FILE_RAND_GEN	2
>> +#define CORRUPT_SC_FILE_CONTENT		3
>> +int corrupt_space_cache_file(struct btrfs_fs_info *fs_info, u64 block_group,
>> +			     int method)
>> +{
>> +	struct btrfs_root *tree_root = fs_info->tree_root;
>> +	struct btrfs_path path;
>> +	struct btrfs_key key;
>> +	struct btrfs_disk_key location;
>> +	struct btrfs_free_space_header *sc_header;
>> +	struct btrfs_file_extent_item *fi;
>> +	struct extent_buffer *node;
>> +	struct extent_buffer *corrupted_node;
>> +	u64 disk_bytenr;
>> +	int slot;
>> +	int ret;
>> +
>> +	key.objectid = BTRFS_FREE_SPACE_OBJECTID;
>> +	key.type = 0;
>> +	key.offset = block_group;
>> +
>> +	btrfs_init_path(&path);
>> +
>> +	/* Don't start trans, as this will cause generation different */
>> +	ret = btrfs_search_slot(NULL, tree_root, &key, &path, 0, 0);
>> +	if (ret) {
>> +		error("failed to find free space cahce file for block group %llu, ret: %d\n",
>> +		      block_group, ret);
>> +		goto out;
>> +	}
>> +	slot = path.slots[0];
>> +	node = path.nodes[0];
>> +	sc_header = btrfs_item_ptr(node, slot, struct btrfs_free_space_header);
>> +	if (method == CORRUPT_SC_FILE_ZERO_GEN ||
>> +	    method == CORRUPT_SC_FILE_RAND_GEN) {
>> +		u64 dst_gen;
>> +
>> +		if (method == CORRUPT_SC_FILE_ZERO_GEN)
>> +			dst_gen = 0;
>> +		else
>> +			dst_gen = rand_u64();
>> +
>> +		btrfs_set_free_space_generation(node, sc_header, dst_gen);
>> +		/* Manually re-calc csum and write to disk */
>> +		ret = write_tree_block(NULL, tree_root, node);
>> +		goto out;
>> +	}
>> +
>> +	btrfs_free_space_key(node, sc_header, &location);
>> +	btrfs_disk_key_to_cpu(&key, &location);
>> +	btrfs_release_path(&path);
>> +
>> +	/* Change to type and offset to search file extent */
>> +	key.type = BTRFS_EXTENT_DATA_KEY;
>> +	key.offset = 0;
>> +
>> +	ret = btrfs_search_slot(NULL, tree_root, &key, &path, 0, 0);
>> +	if (ret) {
>> +		error("failed to find free space cache extent data for blockgroup %llu, ret: %d\n",
>> +		      block_group, ret);
>> +		goto out;
>> +	}
>> +
>> +	slot = path.slots[0];
>> +	node = path.nodes[0];
>> +	fi = btrfs_item_ptr(node, slot, struct btrfs_file_extent_item);
>> +	disk_bytenr = btrfs_file_extent_disk_bytenr(node, fi);
>> +
>> +	/*
>> +	 * Directly write random data into
>> +	 * [disk_bytenr, disk_bytenr + sectorsize) as free space cache inode
>> +	 * doesn't have csum
>> +	 */
>> +	corrupted_node = debug_corrupt_block(tree_root, disk_bytenr,
>> +					     tree_root->sectorsize, 0);
>> +	free_extent_buffer(corrupted_node);
>> +
>> +out:
>> +	btrfs_release_path(&path);
>> +	return ret;
>> +}
>> +
>>  int main(int argc, char **argv)
>>  {
>>  	struct cache_tree root_cache;
>> @@ -1032,6 +1129,7 @@ int main(int argc, char **argv)
>>  	int corrupt_item = 0;
>>  	int corrupt_di = 0;
>>  	int delete = 0;
>> +	int corrupt_sc_file = 0;
>
> Why is it named _file?

Free space cache file (v1).
As v1 free space cache is implemented as a normal file with nodatacow.

Anyway, since I'll follow your suggestion to use "space-cache", there is 
no "_file" then.

Thanks,
Qu

>
>>  	u64 metadata_block = 0;
>>  	u64 inode = 0;
>>  	u64 file_extent = (u64)-1;
>> @@ -1065,11 +1163,12 @@ int main(int argc, char **argv)
>>  			{ "delete", no_argument, NULL, 'd'},
>>  			{ "root", no_argument, NULL, 'r'},
>>  			{ "csum", required_argument, NULL, 'C'},
>> +			{ "space-cache-file", required_argument, NULL, 's'},
>
> "clear-free-space-cache" is quite long but more understandable IMHO
>
>>  			{ "help", no_argument, NULL, GETOPT_VAL_HELP},
>>  			{ NULL, 0, NULL, 0 }
>>  		};
>>
>> -		c = getopt_long(argc, argv, "l:c:b:eEkuUi:f:x:m:K:IDdr:C:",
>> +		c = getopt_long(argc, argv, "l:c:b:eEkuUi:f:x:m:K:IDdr:C:s:",
>>  				long_options, NULL);
>>  		if (c < 0)
>>  			break;
>> @@ -1136,6 +1235,22 @@ int main(int argc, char **argv)
>>  			case 'C':
>>  				csum_bytenr = arg_strtou64(optarg);
>>  				break;
>> +			case 's':
>> +				if (!strncmp(optarg, "zero_gen",
>> +					     sizeof("zero_gen")))
>> +					corrupt_sc_file =
>> +						CORRUPT_SC_FILE_ZERO_GEN;
>> +				else if (!strncmp(optarg, "rand_gen",
>> +						  sizeof("rand_gen")))
>> +					corrupt_sc_file =
>> +						CORRUPT_SC_FILE_RAND_GEN;
>> +				else if (!strncmp(optarg, "content",
>> +						  sizeof("content")))
>> +					corrupt_sc_file =
>> +						CORRUPT_SC_FILE_CONTENT;
>> +				else
>> +					print_usage(1);
>> +				break;
>>  			case GETOPT_VAL_HELP:
>>  			default:
>>  				print_usage(c != GETOPT_VAL_HELP);
>> @@ -1154,6 +1269,13 @@ int main(int argc, char **argv)
>>  		fprintf(stderr, "Open ctree failed\n");
>>  		exit(1);
>>  	}
>> +	if (corrupt_sc_file) {
>> +		if (logical == (u64)-1)
>> +			print_usage(1);
>> +		ret = corrupt_space_cache_file(root->fs_info, logical,
>> +					       corrupt_sc_file);
>> +		goto out_close;
>> +	}
>>  	if (extent_rec) {
>>  		struct btrfs_trans_handle *trans;
>>
>> --
>> 2.8.2
>>
>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/4] Add support to clear v1 free space cache for btrfs check
  2016-05-17 18:12   ` Ivan P
@ 2016-05-18  1:00     ` Qu Wenruo
  2016-05-19 16:54       ` Ivan P
  2016-05-18  9:14     ` David Sterba
  1 sibling, 1 reply; 19+ messages in thread
From: Qu Wenruo @ 2016-05-18  1:00 UTC (permalink / raw)
  To: Ivan P; +Cc: btrfs, dsterba

Not familiar about the problem, but you don't really need to install the 
patch.

You can just execute '<your btrfs-progs source dir>/btrfsck' directly to 
call the new fsck.

And there is still some time before the patch is merged, there is really 
no need to install.

Thanks,
Qu

Ivan P wrote on 2016/05/17 20:12 +0200:
> Thank you, however I can't seem to be able to compile that snapshot, I'm getting
>
> ===========================
> /usr/bin/install -c -m644 -d 64-btrfs-dm.rules
> /home/myuser/aur/btrfs-progs-git/pkg/btrfs-progs-git/usr/lib/udev/rules.d
> /usr/bin/install: cannot create directory ‘64-btrfs-dm.rules’: File exists
> Makefile:400: recipe for target 'install' failed
> make: *** [install] Error 1
> ==> ERROR: A failure occurred in package()
> ===========================
>
> Just to make sure I wasn't screwing up somewhere, I tried the
> btrfs-progs-git AUR package and I'm getting the same thing.
> It's not only me, however: according to [1] it could be that this
> commit has introduced it: [2]
>
> Regards,
> Ivan.
>
> [1] https://aur.archlinux.org/packages/btrfs-progs-git/
> [2] http://repo.or.cz/btrfs-progs-unstable/devel.git?a=commit;h=ebe5b1cc7885027521db3c1d16d84bd54cc1321b
>
> On Tue, May 17, 2016 at 5:56 AM, Qu Wenruo <quwenruo@cn.fujitsu.com> wrote:
>> Also to Ivan P, the initial reporter of the problem.
>>
>> Now "btrfsck --clear-cache" should help you to prevent kernel warning on
>> free space cache problem.
>>
>> Thanks,
>> Qu
>>
>> Qu Wenruo wrote on 2016/05/17 11:47 +0800:
>>>
>>> The patchset can be also fetched from github, just in case mail list
>>> blocks the last patch, which contains binary file.
>>> https://github.com/adam900710/btrfs-progs.git fsck_clear_cache
>>>
>>> Just as describe in the first patch, btrfs kernel "clear_cache" mount
>>> option can't rebuild all free space cache, due to the design of free
>>> space cache.
>>> (Although I pretty like the idea to delay the load of free space cache,
>>> as it hugely reduce the mount time of large fs)
>>>
>>> So this makes users to report error in mail list, complaining
>>> "clear_cache" doesn't wipe the annoying kernel warning on corrupted free
>>> space cache.
>>>
>>> Since kernel can't handle it, and user consider it more like an error,
>>> we'd better handle it like an error, to fix it in btrfs check.
>>>
>>> So this patchset adds the ability to clear free space cache, and add
>>> test case for it.
>>>
>>> The clear procedure is different from kernel, it will remove all free
>>> space cache inodes and its contents(with backrefs), and set
>>> cache_generation of super block to -1.
>>> So there will be no free space cache at all and kernel will be happy
>>> with that.
>>>
>>> This patch also enhances btrfs_previous_item() to use min_objectid to
>>> return as early as possible.
>>>
>>> Lu Fengqi (1):
>>>   btrfs-progs: tests: add 020-bad-free-space-cache
>>>
>>> Qu Wenruo (3):
>>>   btrfs-progs: corrupt-block: Add ability to corrupt free space cache
>>>     file
>>>   btrfs-progs: ctree: return earlier for btrfs_previous_item
>>>   btrfs-progs: fsck: Add support to clear free space cache
>>>
>>>  Documentation/btrfs-check.asciidoc                 |   8 ++
>>>  btrfs-corrupt-block.c                              | 124
>>> ++++++++++++++++++++-
>>>  cmds-check.c                                       |  58 +++++++++-
>>>  ctree.c                                            |   2 +
>>>  free-space-cache.c                                 | 124
>>> +++++++++++++++++++++
>>>  free-space-cache.h                                 |   4 +
>>>  .../020-bad-free-space-cache/default_case.raw.xz   | Bin 0 -> 164068
>>> bytes
>>>  tests/fsck-tests/020-bad-free-space-cache/test.sh  |  16 +++
>>>  8 files changed, 334 insertions(+), 2 deletions(-)
>>>  create mode 100644
>>> tests/fsck-tests/020-bad-free-space-cache/default_case.raw.xz
>>>  create mode 100755 tests/fsck-tests/020-bad-free-space-cache/test.sh
>>>
>>
>>
>
>



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/4] Add support to clear v1 free space cache for btrfs check
  2016-05-17 18:12   ` Ivan P
  2016-05-18  1:00     ` Qu Wenruo
@ 2016-05-18  9:14     ` David Sterba
  1 sibling, 0 replies; 19+ messages in thread
From: David Sterba @ 2016-05-18  9:14 UTC (permalink / raw)
  To: Ivan P; +Cc: Qu Wenruo, btrfs

On Tue, May 17, 2016 at 08:12:20PM +0200, Ivan P wrote:
> Thank you, however I can't seem to be able to compile that snapshot, I'm getting
> 
> ===========================
> /usr/bin/install -c -m644 -d 64-btrfs-dm.rules
> /home/myuser/aur/btrfs-progs-git/pkg/btrfs-progs-git/usr/lib/udev/rules.d
> /usr/bin/install: cannot create directory ‘64-btrfs-dm.rules’: File exists
> Makefile:400: recipe for target 'install' failed
> make: *** [install] Error 1
> ==> ERROR: A failure occurred in package()
> ===========================
> 
> Just to make sure I wasn't screwing up somewhere, I tried the
> btrfs-progs-git AUR package and I'm getting the same thing.
> It's not only me, however: according to [1] it could be that this
> commit has introduced it: [2]
> 
> Regards,
> Ivan.
> 
> [1] https://aur.archlinux.org/packages/btrfs-progs-git/
> [2] http://repo.or.cz/btrfs-progs-unstable/devel.git?a=commit;h=ebe5b1cc7885027521db3c1d16d84bd54cc1321b

I'll look into it, thanks for the report.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/4] Add support to clear v1 free space cache for btrfs check
  2016-05-18  1:00     ` Qu Wenruo
@ 2016-05-19 16:54       ` Ivan P
  2016-05-20  0:44         ` Qu Wenruo
  0 siblings, 1 reply; 19+ messages in thread
From: Ivan P @ 2016-05-19 16:54 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: btrfs, dsterba

Not sure why I was so fixated on installing it - thanks for reminding.

I was able to run it, but it fails:
===========================
# ./btrfsck --repair --clear-space-cache -p /dev/sda
enabling repair mode
Clearing all free space cache
btrfs unable to find ref byte nr 812846673920 parent 0 root 1  owner
1050 offset 0
ERROR: failed to remove backref for disk bytenr 812846673920
ERROR: failed to clear free space cache
transaction.h:41: btrfs_start_transaction: Assertion `root->commit_root` failed.
btrfs check[0x4437ee]
btrfs check(close_ctree_fs_info+0x213)[0x445ab3]
btrfs check(cmd_check+0x638)[0x42b718]
btrfs check(main+0x7b)[0x40fbdb]
/usr/lib/libc.so.6(__libc_start_main+0xf1)[0x7fe4ebe1a741]
btrfs check(_start+0x29)[0x40fcd9]
===========================

I also tried running it WITHOUT --repair first, as well as ONLY with
--repair, followed by --clearcache and same thing happened.
I must add that I feel cheated by the "-p" option, it's more of a
"print heartbeat" than "print progress" IMHO :p

@David Sterba: you're welcome.

Regards,
Ivan.

On Wed, May 18, 2016 at 3:00 AM, Qu Wenruo <quwenruo@cn.fujitsu.com> wrote:
> Not familiar about the problem, but you don't really need to install the
> patch.
>
> You can just execute '<your btrfs-progs source dir>/btrfsck' directly to
> call the new fsck.
>
> And there is still some time before the patch is merged, there is really no
> need to install.
>
> Thanks,
> Qu
>
>
> Ivan P wrote on 2016/05/17 20:12 +0200:
>>
>> Thank you, however I can't seem to be able to compile that snapshot, I'm
>> getting
>>
>> ===========================
>> /usr/bin/install -c -m644 -d 64-btrfs-dm.rules
>> /home/myuser/aur/btrfs-progs-git/pkg/btrfs-progs-git/usr/lib/udev/rules.d
>> /usr/bin/install: cannot create directory ‘64-btrfs-dm.rules’: File exists
>> Makefile:400: recipe for target 'install' failed
>> make: *** [install] Error 1
>> ==> ERROR: A failure occurred in package()
>> ===========================
>>
>> Just to make sure I wasn't screwing up somewhere, I tried the
>> btrfs-progs-git AUR package and I'm getting the same thing.
>> It's not only me, however: according to [1] it could be that this
>> commit has introduced it: [2]
>>
>> Regards,
>> Ivan.
>>
>> [1] https://aur.archlinux.org/packages/btrfs-progs-git/
>> [2]
>> http://repo.or.cz/btrfs-progs-unstable/devel.git?a=commit;h=ebe5b1cc7885027521db3c1d16d84bd54cc1321b
>>
>> On Tue, May 17, 2016 at 5:56 AM, Qu Wenruo <quwenruo@cn.fujitsu.com>
>> wrote:
>>>
>>> Also to Ivan P, the initial reporter of the problem.
>>>
>>> Now "btrfsck --clear-cache" should help you to prevent kernel warning on
>>> free space cache problem.
>>>
>>> Thanks,
>>> Qu
>>>
>>> Qu Wenruo wrote on 2016/05/17 11:47 +0800:
>>>>
>>>>
>>>> The patchset can be also fetched from github, just in case mail list
>>>> blocks the last patch, which contains binary file.
>>>> https://github.com/adam900710/btrfs-progs.git fsck_clear_cache
>>>>
>>>> Just as describe in the first patch, btrfs kernel "clear_cache" mount
>>>> option can't rebuild all free space cache, due to the design of free
>>>> space cache.
>>>> (Although I pretty like the idea to delay the load of free space cache,
>>>> as it hugely reduce the mount time of large fs)
>>>>
>>>> So this makes users to report error in mail list, complaining
>>>> "clear_cache" doesn't wipe the annoying kernel warning on corrupted free
>>>> space cache.
>>>>
>>>> Since kernel can't handle it, and user consider it more like an error,
>>>> we'd better handle it like an error, to fix it in btrfs check.
>>>>
>>>> So this patchset adds the ability to clear free space cache, and add
>>>> test case for it.
>>>>
>>>> The clear procedure is different from kernel, it will remove all free
>>>> space cache inodes and its contents(with backrefs), and set
>>>> cache_generation of super block to -1.
>>>> So there will be no free space cache at all and kernel will be happy
>>>> with that.
>>>>
>>>> This patch also enhances btrfs_previous_item() to use min_objectid to
>>>> return as early as possible.
>>>>
>>>> Lu Fengqi (1):
>>>>   btrfs-progs: tests: add 020-bad-free-space-cache
>>>>
>>>> Qu Wenruo (3):
>>>>   btrfs-progs: corrupt-block: Add ability to corrupt free space cache
>>>>     file
>>>>   btrfs-progs: ctree: return earlier for btrfs_previous_item
>>>>   btrfs-progs: fsck: Add support to clear free space cache
>>>>
>>>>  Documentation/btrfs-check.asciidoc                 |   8 ++
>>>>  btrfs-corrupt-block.c                              | 124
>>>> ++++++++++++++++++++-
>>>>  cmds-check.c                                       |  58 +++++++++-
>>>>  ctree.c                                            |   2 +
>>>>  free-space-cache.c                                 | 124
>>>> +++++++++++++++++++++
>>>>  free-space-cache.h                                 |   4 +
>>>>  .../020-bad-free-space-cache/default_case.raw.xz   | Bin 0 -> 164068
>>>> bytes
>>>>  tests/fsck-tests/020-bad-free-space-cache/test.sh  |  16 +++
>>>>  8 files changed, 334 insertions(+), 2 deletions(-)
>>>>  create mode 100644
>>>> tests/fsck-tests/020-bad-free-space-cache/default_case.raw.xz
>>>>  create mode 100755 tests/fsck-tests/020-bad-free-space-cache/test.sh
>>>>
>>>
>>>
>>
>>
>
>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/4] Add support to clear v1 free space cache for btrfs check
  2016-05-19 16:54       ` Ivan P
@ 2016-05-20  0:44         ` Qu Wenruo
  2016-05-22 17:44           ` Ivan P
  0 siblings, 1 reply; 19+ messages in thread
From: Qu Wenruo @ 2016-05-20  0:44 UTC (permalink / raw)
  To: Ivan P; +Cc: btrfs, dsterba



Ivan P wrote on 2016/05/19 18:54 +0200:
> Not sure why I was so fixated on installing it - thanks for reminding.
>
> I was able to run it, but it fails:
> ===========================
> # ./btrfsck --repair --clear-space-cache -p /dev/sda
> enabling repair mode
> Clearing all free space cache
> btrfs unable to find ref byte nr 812846673920 parent 0 root 1  owner

Error output here means btrfsck find the free space cache inode and its 
data extent, but failed to delete the backref.

In that case, the only successfully cleaned space cache will be commited 
to disk, and the deletion failure won't be commited to disk.

Would please run "btrfs check --readonly" on the image after clear space 
cache and paste the output?

I'm a little interested in what the root problem is.
If it's just a normal backref mismatch problem, it's my patch to blame 
as it's too restrict to handle such error. (but for a good safety reason)
If that's the case, I can add some option to allow clear cache to be a 
little more aggressive.


And it's better to paste "btrfs-debug-tree -t 1" and "btrfs-debug-tree 
-t 2" on your btrfs image for better debugging.

Such output won't leak much personal info, except the subvolume name.
And feel free to fuzz the subvolume name.

> 1050 offset 0
> ERROR: failed to remove backref for disk bytenr 812846673920
> ERROR: failed to clear free space cache
> transaction.h:41: btrfs_start_transaction: Assertion `root->commit_root` failed.
> btrfs check[0x4437ee]
> btrfs check(close_ctree_fs_info+0x213)[0x445ab3]
> btrfs check(cmd_check+0x638)[0x42b718]
> btrfs check(main+0x7b)[0x40fbdb]
> /usr/lib/libc.so.6(__libc_start_main+0xf1)[0x7fe4ebe1a741]
> btrfs check(_start+0x29)[0x40fcd9]
> ===========================
>
> I also tried running it WITHOUT --repair first, as well as ONLY with
> --repair, followed by --clearcache and same thing happened.

--clear-space-cache implies --repair, but override dangerous repair 
behavior and only do space cache clean things.
So without or without --repair, it's the same.

> I must add that I feel cheated by the "-p" option, it's more of a
> "print heartbeat" than "print progress" IMHO :p

Thanks for the -p option, clear space cache doesn't support progress 
report yet.

I'll add them in next version.

Thanks,
Qu
>
> @David Sterba: you're welcome.
>
> Regards,
> Ivan.
>
> On Wed, May 18, 2016 at 3:00 AM, Qu Wenruo <quwenruo@cn.fujitsu.com> wrote:
>> Not familiar about the problem, but you don't really need to install the
>> patch.
>>
>> You can just execute '<your btrfs-progs source dir>/btrfsck' directly to
>> call the new fsck.
>>
>> And there is still some time before the patch is merged, there is really no
>> need to install.
>>
>> Thanks,
>> Qu
>>
>>
>> Ivan P wrote on 2016/05/17 20:12 +0200:
>>>
>>> Thank you, however I can't seem to be able to compile that snapshot, I'm
>>> getting
>>>
>>> ===========================
>>> /usr/bin/install -c -m644 -d 64-btrfs-dm.rules
>>> /home/myuser/aur/btrfs-progs-git/pkg/btrfs-progs-git/usr/lib/udev/rules.d
>>> /usr/bin/install: cannot create directory ‘64-btrfs-dm.rules’: File exists
>>> Makefile:400: recipe for target 'install' failed
>>> make: *** [install] Error 1
>>> ==> ERROR: A failure occurred in package()
>>> ===========================
>>>
>>> Just to make sure I wasn't screwing up somewhere, I tried the
>>> btrfs-progs-git AUR package and I'm getting the same thing.
>>> It's not only me, however: according to [1] it could be that this
>>> commit has introduced it: [2]
>>>
>>> Regards,
>>> Ivan.
>>>
>>> [1] https://aur.archlinux.org/packages/btrfs-progs-git/
>>> [2]
>>> http://repo.or.cz/btrfs-progs-unstable/devel.git?a=commit;h=ebe5b1cc7885027521db3c1d16d84bd54cc1321b
>>>
>>> On Tue, May 17, 2016 at 5:56 AM, Qu Wenruo <quwenruo@cn.fujitsu.com>
>>> wrote:
>>>>
>>>> Also to Ivan P, the initial reporter of the problem.
>>>>
>>>> Now "btrfsck --clear-cache" should help you to prevent kernel warning on
>>>> free space cache problem.
>>>>
>>>> Thanks,
>>>> Qu
>>>>
>>>> Qu Wenruo wrote on 2016/05/17 11:47 +0800:
>>>>>
>>>>>
>>>>> The patchset can be also fetched from github, just in case mail list
>>>>> blocks the last patch, which contains binary file.
>>>>> https://github.com/adam900710/btrfs-progs.git fsck_clear_cache
>>>>>
>>>>> Just as describe in the first patch, btrfs kernel "clear_cache" mount
>>>>> option can't rebuild all free space cache, due to the design of free
>>>>> space cache.
>>>>> (Although I pretty like the idea to delay the load of free space cache,
>>>>> as it hugely reduce the mount time of large fs)
>>>>>
>>>>> So this makes users to report error in mail list, complaining
>>>>> "clear_cache" doesn't wipe the annoying kernel warning on corrupted free
>>>>> space cache.
>>>>>
>>>>> Since kernel can't handle it, and user consider it more like an error,
>>>>> we'd better handle it like an error, to fix it in btrfs check.
>>>>>
>>>>> So this patchset adds the ability to clear free space cache, and add
>>>>> test case for it.
>>>>>
>>>>> The clear procedure is different from kernel, it will remove all free
>>>>> space cache inodes and its contents(with backrefs), and set
>>>>> cache_generation of super block to -1.
>>>>> So there will be no free space cache at all and kernel will be happy
>>>>> with that.
>>>>>
>>>>> This patch also enhances btrfs_previous_item() to use min_objectid to
>>>>> return as early as possible.
>>>>>
>>>>> Lu Fengqi (1):
>>>>>   btrfs-progs: tests: add 020-bad-free-space-cache
>>>>>
>>>>> Qu Wenruo (3):
>>>>>   btrfs-progs: corrupt-block: Add ability to corrupt free space cache
>>>>>     file
>>>>>   btrfs-progs: ctree: return earlier for btrfs_previous_item
>>>>>   btrfs-progs: fsck: Add support to clear free space cache
>>>>>
>>>>>  Documentation/btrfs-check.asciidoc                 |   8 ++
>>>>>  btrfs-corrupt-block.c                              | 124
>>>>> ++++++++++++++++++++-
>>>>>  cmds-check.c                                       |  58 +++++++++-
>>>>>  ctree.c                                            |   2 +
>>>>>  free-space-cache.c                                 | 124
>>>>> +++++++++++++++++++++
>>>>>  free-space-cache.h                                 |   4 +
>>>>>  .../020-bad-free-space-cache/default_case.raw.xz   | Bin 0 -> 164068
>>>>> bytes
>>>>>  tests/fsck-tests/020-bad-free-space-cache/test.sh  |  16 +++
>>>>>  8 files changed, 334 insertions(+), 2 deletions(-)
>>>>>  create mode 100644
>>>>> tests/fsck-tests/020-bad-free-space-cache/default_case.raw.xz
>>>>>  create mode 100755 tests/fsck-tests/020-bad-free-space-cache/test.sh
>>>>>
>>>>
>>>>
>>>
>>>
>>
>>
>
>



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/4] Add support to clear v1 free space cache for btrfs check
  2016-05-20  0:44         ` Qu Wenruo
@ 2016-05-22 17:44           ` Ivan P
  2016-05-23  1:18             ` Qu Wenruo
  0 siblings, 1 reply; 19+ messages in thread
From: Ivan P @ 2016-05-22 17:44 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: btrfs, dsterba

Sorry for the delay, I just got home today.

Here is the output after running --clearcache:
=======================================
./btrfsck --readonly /dev/sda
Checking filesystem on /dev/sda
UUID: 013cda95-8aab-4cb2-acdd-2f0f78036e02
checking extents
checking free space cache
cache and super generation don't match, space cache will be invalidated
checking fs roots
checking csums
checking root refs
found 859613876224 bytes used err is 0
total csum bytes: 838453732
total tree bytes: 980369408
total fs tree bytes: 38387712
total extent tree bytes: 11075584
btree space waste bytes: 71010494
file data blocks allocated: 858633506816
referenced 858632945664
=======================================

The debug logs:
https://dl.dropboxusercontent.com/u/19330332/btrfs-debug-tree.tar.xz


>> I must add that I feel cheated by the "-p" option, it's more of a
>>"print heartbeat" than "print progress" IMHO :p
>
> Thanks for the -p option, clear space cache doesn't support progress report yet.

Ah, I was referring to the btrfsck overall, it seems all the -p option
does in --repair mode is print . -> o -> O in succession.

Regards,
Ivan.

On Fri, May 20, 2016 at 2:44 AM, Qu Wenruo <quwenruo@cn.fujitsu.com> wrote:
>
>
> Ivan P wrote on 2016/05/19 18:54 +0200:
>>
>> Not sure why I was so fixated on installing it - thanks for reminding.
>>
>> I was able to run it, but it fails:
>> ===========================
>> # ./btrfsck --repair --clear-space-cache -p /dev/sda
>> enabling repair mode
>> Clearing all free space cache
>> btrfs unable to find ref byte nr 812846673920 parent 0 root 1  owner
>
>
> Error output here means btrfsck find the free space cache inode and its data
> extent, but failed to delete the backref.
>
> In that case, the only successfully cleaned space cache will be commited to
> disk, and the deletion failure won't be commited to disk.
>
> Would please run "btrfs check --readonly" on the image after clear space
> cache and paste the output?
>
> I'm a little interested in what the root problem is.
> If it's just a normal backref mismatch problem, it's my patch to blame as
> it's too restrict to handle such error. (but for a good safety reason)
> If that's the case, I can add some option to allow clear cache to be a
> little more aggressive.
>
>
> And it's better to paste "btrfs-debug-tree -t 1" and "btrfs-debug-tree -t 2"
> on your btrfs image for better debugging.
>
> Such output won't leak much personal info, except the subvolume name.
> And feel free to fuzz the subvolume name.
>
>> 1050 offset 0
>> ERROR: failed to remove backref for disk bytenr 812846673920
>> ERROR: failed to clear free space cache
>> transaction.h:41: btrfs_start_transaction: Assertion `root->commit_root`
>> failed.
>> btrfs check[0x4437ee]
>> btrfs check(close_ctree_fs_info+0x213)[0x445ab3]
>> btrfs check(cmd_check+0x638)[0x42b718]
>> btrfs check(main+0x7b)[0x40fbdb]
>> /usr/lib/libc.so.6(__libc_start_main+0xf1)[0x7fe4ebe1a741]
>> btrfs check(_start+0x29)[0x40fcd9]
>> ===========================
>>
>> I also tried running it WITHOUT --repair first, as well as ONLY with
>> --repair, followed by --clearcache and same thing happened.
>
>
> --clear-space-cache implies --repair, but override dangerous repair behavior
> and only do space cache clean things.
> So without or without --repair, it's the same.
>
>> I must add that I feel cheated by the "-p" option, it's more of a
>> "print heartbeat" than "print progress" IMHO :p
>
>
> Thanks for the -p option, clear space cache doesn't support progress report
> yet.
>
> I'll add them in next version.
>
> Thanks,
> Qu
>
>>
>> @David Sterba: you're welcome.
>>
>> Regards,
>> Ivan.
>>
>> On Wed, May 18, 2016 at 3:00 AM, Qu Wenruo <quwenruo@cn.fujitsu.com>
>> wrote:
>>>
>>> Not familiar about the problem, but you don't really need to install the
>>> patch.
>>>
>>> You can just execute '<your btrfs-progs source dir>/btrfsck' directly to
>>> call the new fsck.
>>>
>>> And there is still some time before the patch is merged, there is really
>>> no
>>> need to install.
>>>
>>> Thanks,
>>> Qu
>>>
>>>
>>> Ivan P wrote on 2016/05/17 20:12 +0200:
>>>>
>>>>
>>>> Thank you, however I can't seem to be able to compile that snapshot, I'm
>>>> getting
>>>>
>>>> ===========================
>>>> /usr/bin/install -c -m644 -d 64-btrfs-dm.rules
>>>>
>>>> /home/myuser/aur/btrfs-progs-git/pkg/btrfs-progs-git/usr/lib/udev/rules.d
>>>> /usr/bin/install: cannot create directory ‘64-btrfs-dm.rules’: File
>>>> exists
>>>> Makefile:400: recipe for target 'install' failed
>>>> make: *** [install] Error 1
>>>> ==> ERROR: A failure occurred in package()
>>>> ===========================
>>>>
>>>> Just to make sure I wasn't screwing up somewhere, I tried the
>>>> btrfs-progs-git AUR package and I'm getting the same thing.
>>>> It's not only me, however: according to [1] it could be that this
>>>> commit has introduced it: [2]
>>>>
>>>> Regards,
>>>> Ivan.
>>>>
>>>> [1] https://aur.archlinux.org/packages/btrfs-progs-git/
>>>> [2]
>>>>
>>>> http://repo.or.cz/btrfs-progs-unstable/devel.git?a=commit;h=ebe5b1cc7885027521db3c1d16d84bd54cc1321b
>>>>
>>>> On Tue, May 17, 2016 at 5:56 AM, Qu Wenruo <quwenruo@cn.fujitsu.com>
>>>> wrote:
>>>>>
>>>>>
>>>>> Also to Ivan P, the initial reporter of the problem.
>>>>>
>>>>> Now "btrfsck --clear-cache" should help you to prevent kernel warning
>>>>> on
>>>>> free space cache problem.
>>>>>
>>>>> Thanks,
>>>>> Qu
>>>>>
>>>>> Qu Wenruo wrote on 2016/05/17 11:47 +0800:
>>>>>>
>>>>>>
>>>>>>
>>>>>> The patchset can be also fetched from github, just in case mail list
>>>>>> blocks the last patch, which contains binary file.
>>>>>> https://github.com/adam900710/btrfs-progs.git fsck_clear_cache
>>>>>>
>>>>>> Just as describe in the first patch, btrfs kernel "clear_cache" mount
>>>>>> option can't rebuild all free space cache, due to the design of free
>>>>>> space cache.
>>>>>> (Although I pretty like the idea to delay the load of free space
>>>>>> cache,
>>>>>> as it hugely reduce the mount time of large fs)
>>>>>>
>>>>>> So this makes users to report error in mail list, complaining
>>>>>> "clear_cache" doesn't wipe the annoying kernel warning on corrupted
>>>>>> free
>>>>>> space cache.
>>>>>>
>>>>>> Since kernel can't handle it, and user consider it more like an error,
>>>>>> we'd better handle it like an error, to fix it in btrfs check.
>>>>>>
>>>>>> So this patchset adds the ability to clear free space cache, and add
>>>>>> test case for it.
>>>>>>
>>>>>> The clear procedure is different from kernel, it will remove all free
>>>>>> space cache inodes and its contents(with backrefs), and set
>>>>>> cache_generation of super block to -1.
>>>>>> So there will be no free space cache at all and kernel will be happy
>>>>>> with that.
>>>>>>
>>>>>> This patch also enhances btrfs_previous_item() to use min_objectid to
>>>>>> return as early as possible.
>>>>>>
>>>>>> Lu Fengqi (1):
>>>>>>   btrfs-progs: tests: add 020-bad-free-space-cache
>>>>>>
>>>>>> Qu Wenruo (3):
>>>>>>   btrfs-progs: corrupt-block: Add ability to corrupt free space cache
>>>>>>     file
>>>>>>   btrfs-progs: ctree: return earlier for btrfs_previous_item
>>>>>>   btrfs-progs: fsck: Add support to clear free space cache
>>>>>>
>>>>>>  Documentation/btrfs-check.asciidoc                 |   8 ++
>>>>>>  btrfs-corrupt-block.c                              | 124
>>>>>> ++++++++++++++++++++-
>>>>>>  cmds-check.c                                       |  58 +++++++++-
>>>>>>  ctree.c                                            |   2 +
>>>>>>  free-space-cache.c                                 | 124
>>>>>> +++++++++++++++++++++
>>>>>>  free-space-cache.h                                 |   4 +
>>>>>>  .../020-bad-free-space-cache/default_case.raw.xz   | Bin 0 -> 164068
>>>>>> bytes
>>>>>>  tests/fsck-tests/020-bad-free-space-cache/test.sh  |  16 +++
>>>>>>  8 files changed, 334 insertions(+), 2 deletions(-)
>>>>>>  create mode 100644
>>>>>> tests/fsck-tests/020-bad-free-space-cache/default_case.raw.xz
>>>>>>  create mode 100755 tests/fsck-tests/020-bad-free-space-cache/test.sh
>>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>
>>>
>>
>>
>
>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/4] Add support to clear v1 free space cache for btrfs check
  2016-05-22 17:44           ` Ivan P
@ 2016-05-23  1:18             ` Qu Wenruo
  0 siblings, 0 replies; 19+ messages in thread
From: Qu Wenruo @ 2016-05-23  1:18 UTC (permalink / raw)
  To: Ivan P; +Cc: btrfs, dsterba



Ivan P wrote on 2016/05/22 19:44 +0200:
> Sorry for the delay, I just got home today.
>
> Here is the output after running --clearcache:
> =======================================
> ./btrfsck --readonly /dev/sda
> Checking filesystem on /dev/sda
> UUID: 013cda95-8aab-4cb2-acdd-2f0f78036e02
> checking extents
> checking free space cache
> cache and super generation don't match, space cache will be invalidated
> checking fs roots
> checking csums
> checking root refs
> found 859613876224 bytes used err is 0
> total csum bytes: 838453732
> total tree bytes: 980369408
> total fs tree bytes: 38387712
> total extent tree bytes: 11075584
> btree space waste bytes: 71010494
> file data blocks allocated: 858633506816
> referenced 858632945664
> =======================================

So the fs is still safe.

>
> The debug logs:
> https://dl.dropboxusercontent.com/u/19330332/btrfs-debug-tree.tar.xz

Quite strange, there is nothing wrong at all.

So some hidden bug is in the code.
I'll update the patchset to fix them and add new process support.

Thanks,
Qu
>
>
>>> I must add that I feel cheated by the "-p" option, it's more of a
>>> "print heartbeat" than "print progress" IMHO :p
>>
>> Thanks for the -p option, clear space cache doesn't support progress report yet.
>
> Ah, I was referring to the btrfsck overall, it seems all the -p option
> does in --repair mode is print . -> o -> O in succession.
>
> Regards,
> Ivan.
>
> On Fri, May 20, 2016 at 2:44 AM, Qu Wenruo <quwenruo@cn.fujitsu.com> wrote:
>>
>>
>> Ivan P wrote on 2016/05/19 18:54 +0200:
>>>
>>> Not sure why I was so fixated on installing it - thanks for reminding.
>>>
>>> I was able to run it, but it fails:
>>> ===========================
>>> # ./btrfsck --repair --clear-space-cache -p /dev/sda
>>> enabling repair mode
>>> Clearing all free space cache
>>> btrfs unable to find ref byte nr 812846673920 parent 0 root 1  owner
>>
>>
>> Error output here means btrfsck find the free space cache inode and its data
>> extent, but failed to delete the backref.
>>
>> In that case, the only successfully cleaned space cache will be commited to
>> disk, and the deletion failure won't be commited to disk.
>>
>> Would please run "btrfs check --readonly" on the image after clear space
>> cache and paste the output?
>>
>> I'm a little interested in what the root problem is.
>> If it's just a normal backref mismatch problem, it's my patch to blame as
>> it's too restrict to handle such error. (but for a good safety reason)
>> If that's the case, I can add some option to allow clear cache to be a
>> little more aggressive.
>>
>>
>> And it's better to paste "btrfs-debug-tree -t 1" and "btrfs-debug-tree -t 2"
>> on your btrfs image for better debugging.
>>
>> Such output won't leak much personal info, except the subvolume name.
>> And feel free to fuzz the subvolume name.
>>
>>> 1050 offset 0
>>> ERROR: failed to remove backref for disk bytenr 812846673920
>>> ERROR: failed to clear free space cache
>>> transaction.h:41: btrfs_start_transaction: Assertion `root->commit_root`
>>> failed.
>>> btrfs check[0x4437ee]
>>> btrfs check(close_ctree_fs_info+0x213)[0x445ab3]
>>> btrfs check(cmd_check+0x638)[0x42b718]
>>> btrfs check(main+0x7b)[0x40fbdb]
>>> /usr/lib/libc.so.6(__libc_start_main+0xf1)[0x7fe4ebe1a741]
>>> btrfs check(_start+0x29)[0x40fcd9]
>>> ===========================
>>>
>>> I also tried running it WITHOUT --repair first, as well as ONLY with
>>> --repair, followed by --clearcache and same thing happened.
>>
>>
>> --clear-space-cache implies --repair, but override dangerous repair behavior
>> and only do space cache clean things.
>> So without or without --repair, it's the same.
>>
>>> I must add that I feel cheated by the "-p" option, it's more of a
>>> "print heartbeat" than "print progress" IMHO :p
>>
>>
>> Thanks for the -p option, clear space cache doesn't support progress report
>> yet.
>>
>> I'll add them in next version.
>>
>> Thanks,
>> Qu
>>
>>>
>>> @David Sterba: you're welcome.
>>>
>>> Regards,
>>> Ivan.
>>>
>>> On Wed, May 18, 2016 at 3:00 AM, Qu Wenruo <quwenruo@cn.fujitsu.com>
>>> wrote:
>>>>
>>>> Not familiar about the problem, but you don't really need to install the
>>>> patch.
>>>>
>>>> You can just execute '<your btrfs-progs source dir>/btrfsck' directly to
>>>> call the new fsck.
>>>>
>>>> And there is still some time before the patch is merged, there is really
>>>> no
>>>> need to install.
>>>>
>>>> Thanks,
>>>> Qu
>>>>
>>>>
>>>> Ivan P wrote on 2016/05/17 20:12 +0200:
>>>>>
>>>>>
>>>>> Thank you, however I can't seem to be able to compile that snapshot, I'm
>>>>> getting
>>>>>
>>>>> ===========================
>>>>> /usr/bin/install -c -m644 -d 64-btrfs-dm.rules
>>>>>
>>>>> /home/myuser/aur/btrfs-progs-git/pkg/btrfs-progs-git/usr/lib/udev/rules.d
>>>>> /usr/bin/install: cannot create directory ‘64-btrfs-dm.rules’: File
>>>>> exists
>>>>> Makefile:400: recipe for target 'install' failed
>>>>> make: *** [install] Error 1
>>>>> ==> ERROR: A failure occurred in package()
>>>>> ===========================
>>>>>
>>>>> Just to make sure I wasn't screwing up somewhere, I tried the
>>>>> btrfs-progs-git AUR package and I'm getting the same thing.
>>>>> It's not only me, however: according to [1] it could be that this
>>>>> commit has introduced it: [2]
>>>>>
>>>>> Regards,
>>>>> Ivan.
>>>>>
>>>>> [1] https://aur.archlinux.org/packages/btrfs-progs-git/
>>>>> [2]
>>>>>
>>>>> http://repo.or.cz/btrfs-progs-unstable/devel.git?a=commit;h=ebe5b1cc7885027521db3c1d16d84bd54cc1321b
>>>>>
>>>>> On Tue, May 17, 2016 at 5:56 AM, Qu Wenruo <quwenruo@cn.fujitsu.com>
>>>>> wrote:
>>>>>>
>>>>>>
>>>>>> Also to Ivan P, the initial reporter of the problem.
>>>>>>
>>>>>> Now "btrfsck --clear-cache" should help you to prevent kernel warning
>>>>>> on
>>>>>> free space cache problem.
>>>>>>
>>>>>> Thanks,
>>>>>> Qu
>>>>>>
>>>>>> Qu Wenruo wrote on 2016/05/17 11:47 +0800:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> The patchset can be also fetched from github, just in case mail list
>>>>>>> blocks the last patch, which contains binary file.
>>>>>>> https://github.com/adam900710/btrfs-progs.git fsck_clear_cache
>>>>>>>
>>>>>>> Just as describe in the first patch, btrfs kernel "clear_cache" mount
>>>>>>> option can't rebuild all free space cache, due to the design of free
>>>>>>> space cache.
>>>>>>> (Although I pretty like the idea to delay the load of free space
>>>>>>> cache,
>>>>>>> as it hugely reduce the mount time of large fs)
>>>>>>>
>>>>>>> So this makes users to report error in mail list, complaining
>>>>>>> "clear_cache" doesn't wipe the annoying kernel warning on corrupted
>>>>>>> free
>>>>>>> space cache.
>>>>>>>
>>>>>>> Since kernel can't handle it, and user consider it more like an error,
>>>>>>> we'd better handle it like an error, to fix it in btrfs check.
>>>>>>>
>>>>>>> So this patchset adds the ability to clear free space cache, and add
>>>>>>> test case for it.
>>>>>>>
>>>>>>> The clear procedure is different from kernel, it will remove all free
>>>>>>> space cache inodes and its contents(with backrefs), and set
>>>>>>> cache_generation of super block to -1.
>>>>>>> So there will be no free space cache at all and kernel will be happy
>>>>>>> with that.
>>>>>>>
>>>>>>> This patch also enhances btrfs_previous_item() to use min_objectid to
>>>>>>> return as early as possible.
>>>>>>>
>>>>>>> Lu Fengqi (1):
>>>>>>>   btrfs-progs: tests: add 020-bad-free-space-cache
>>>>>>>
>>>>>>> Qu Wenruo (3):
>>>>>>>   btrfs-progs: corrupt-block: Add ability to corrupt free space cache
>>>>>>>     file
>>>>>>>   btrfs-progs: ctree: return earlier for btrfs_previous_item
>>>>>>>   btrfs-progs: fsck: Add support to clear free space cache
>>>>>>>
>>>>>>>  Documentation/btrfs-check.asciidoc                 |   8 ++
>>>>>>>  btrfs-corrupt-block.c                              | 124
>>>>>>> ++++++++++++++++++++-
>>>>>>>  cmds-check.c                                       |  58 +++++++++-
>>>>>>>  ctree.c                                            |   2 +
>>>>>>>  free-space-cache.c                                 | 124
>>>>>>> +++++++++++++++++++++
>>>>>>>  free-space-cache.h                                 |   4 +
>>>>>>>  .../020-bad-free-space-cache/default_case.raw.xz   | Bin 0 -> 164068
>>>>>>> bytes
>>>>>>>  tests/fsck-tests/020-bad-free-space-cache/test.sh  |  16 +++
>>>>>>>  8 files changed, 334 insertions(+), 2 deletions(-)
>>>>>>>  create mode 100644
>>>>>>> tests/fsck-tests/020-bad-free-space-cache/default_case.raw.xz
>>>>>>>  create mode 100755 tests/fsck-tests/020-bad-free-space-cache/test.sh
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>
>>>
>>
>>
>
>



^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2016-05-23  1:18 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-05-17  3:47 [PATCH 0/4] Add support to clear v1 free space cache for btrfs check Qu Wenruo
2016-05-17  3:47 ` [PATCH 1/4] btrfs-progs: corrupt-block: Add ability to corrupt free space cache file Qu Wenruo
2016-05-17 13:00   ` David Sterba
2016-05-17 13:02     ` David Sterba
2016-05-18  0:49     ` Qu Wenruo
2016-05-17  3:47 ` [PATCH 2/4] btrfs-progs: ctree: return earlier for btrfs_previous_item Qu Wenruo
2016-05-17 12:50   ` David Sterba
2016-05-18  0:40     ` Qu Wenruo
2016-05-17  3:47 ` [PATCH 3/4] btrfs-progs: fsck: Add support to clear free space cache Qu Wenruo
2016-05-17  3:53   ` [PATCH v1.1 " Qu Wenruo
2016-05-17  3:47 ` [PATCH 4/4] btrfs-progs: tests: add 020-bad-free-space-cache Qu Wenruo
2016-05-17  3:56 ` [PATCH 0/4] Add support to clear v1 free space cache for btrfs check Qu Wenruo
2016-05-17 18:12   ` Ivan P
2016-05-18  1:00     ` Qu Wenruo
2016-05-19 16:54       ` Ivan P
2016-05-20  0:44         ` Qu Wenruo
2016-05-22 17:44           ` Ivan P
2016-05-23  1:18             ` Qu Wenruo
2016-05-18  9:14     ` David Sterba

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).