* [PATCH 0/3] btrfs-progs: Do proper extent item generation repair
@ 2020-01-17 7:29 Qu Wenruo
2020-01-17 7:29 ` [PATCH 1/3] btrfs-progs: check/lowmem: Repair invalid extent item generation Qu Wenruo
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Qu Wenruo @ 2020-01-17 7:29 UTC (permalink / raw)
To: linux-btrfs
Before this patchset, the only way to repair invalid extent item
generation is to use --init-extent-tree, which is really a bad idea.
To rebuild the whole extent tree just for one corrupted extent item?
I must be insane at that time.
This patch introduces the proper extent item generation repair
functionality for both mode, and alter existing test case to also test
repair.
Qu Wenruo (3):
btrfs-progs: check/lowmem: Repair invalid extent item generation
btrfs-progs: check/original: Repair extent item generation
btrfs-progs: tests/fsck-044: Enable repair test for invalid extent
item generation
check/main.c | 66 +++++++++++++++++
check/mode-lowmem.c | 74 +++++++++++++++++++
.../.lowmem_repairable | 0
.../test.sh | 19 -----
4 files changed, 140 insertions(+), 19 deletions(-)
create mode 100644 tests/fsck-tests/044-invalid-extent-item-generation/.lowmem_repairable
delete mode 100755 tests/fsck-tests/044-invalid-extent-item-generation/test.sh
--
2.24.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/3] btrfs-progs: check/lowmem: Repair invalid extent item generation
2020-01-17 7:29 [PATCH 0/3] btrfs-progs: Do proper extent item generation repair Qu Wenruo
@ 2020-01-17 7:29 ` Qu Wenruo
2020-01-17 7:29 ` [PATCH 2/3] btrfs-progs: check/original: Repair " Qu Wenruo
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Qu Wenruo @ 2020-01-17 7:29 UTC (permalink / raw)
To: linux-btrfs
Before this patch, the only way to repair extent item generation is
--init-extent-tree.
Unfortunately that operation is super slow since it needs to start and
commit transaction to each extent backref repaired.
This has caused end-user several failed repair, so introduce the
repairability for lowmem mode.
Please note that, there is a trade off between accurate generation
against complexity.
In theory we should grab the accurate generation number from EXTENT_DATA
item, but that's a little complex to parse in lowmem mode.
So this patch will just reset extent generation to current transid,
passing tree-checker.
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
check/mode-lowmem.c | 74 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 74 insertions(+)
diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index 2f76d634bdb5..6fe9dc780285 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -4140,6 +4140,68 @@ out:
return ret;
}
+static int reset_extent_item_gen(struct btrfs_fs_info *fs_info,
+ struct btrfs_path *path)
+{
+ struct btrfs_trans_handle *trans;
+ struct btrfs_key key;
+ struct btrfs_extent_item *ei;
+ struct extent_buffer *leaf;
+ u64 transid;
+ int slot;
+ int ret;
+
+ btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
+ ASSERT(key.type == BTRFS_EXTENT_ITEM_KEY ||
+ key.type == BTRFS_METADATA_ITEM_KEY);
+
+ btrfs_release_path(path);
+ trans = btrfs_start_transaction(fs_info->extent_root, 1);
+ if (IS_ERR(trans)) {
+ ret = PTR_ERR(trans);
+ errno = -ret;
+ error("failed to start transaction: %m");
+ return ret;
+ }
+ transid = trans->transid;
+ ret = btrfs_search_slot(trans, fs_info->extent_root, &key, path, 0, 1);
+ if (ret < 0) {
+ errno = -ret;
+ error("failed to search extent tree: %m");
+ goto abort;
+ }
+ if (ret > 0) {
+ ret = -ENOENT;
+ error("unable to find key (%llu %u %llu)", key.objectid,
+ key.type, key.offset);
+ goto abort;
+ }
+ slot = path->slots[0];
+ leaf = path->nodes[0];
+ if (btrfs_item_size_nr(leaf, slot) < sizeof(*ei)) {
+ ret = -EUCLEAN;
+ error("invalid extent item size, have %u expect >= %zu",
+ btrfs_item_size_nr(leaf, slot), sizeof(*ei));
+ goto abort;
+ }
+ ei = btrfs_item_ptr(leaf, slot, struct btrfs_extent_item);
+ /*
+ * The best generation should be fetched from EXTENT_DATA item,
+ * but in lowmem mode we hadn't get the info.
+ * So here use transid for it, which is good enough to pass
+ * tree-checker and not cause any problem.
+ */
+ btrfs_set_extent_generation(leaf, ei, transid);
+ ret = btrfs_commit_transaction(trans, fs_info->extent_root);
+ if (!ret)
+ printf("reset extent item generation to %llu for extent %llu\n",
+ transid, key.objectid);
+ return ret;
+abort:
+ btrfs_abort_transaction(trans, ret);
+ return ret;
+}
+
/*
* This function will check a given extent item, including its backref and
* itself (like crossing stripe boundary and type)
@@ -4203,6 +4265,18 @@ static int check_extent_item(struct btrfs_fs_info *fs_info,
"invalid generation for extent %llu, have %llu expect (0, %llu]",
key.objectid, gen, super_gen + 1);
err |= INVALID_GENERATION;
+ if (repair) {
+ ret = reset_extent_item_gen(fs_info, path);
+ if (!ret)
+ err &= ~INVALID_GENERATION;
+ /* Fatal error in repair, path unreliable */
+ if (!path->nodes[0])
+ return ret;
+ /* Reset related eb/slots to new path */
+ eb = path->nodes[0];
+ slot = path->slots[0];
+ ei = btrfs_item_ptr(eb, slot, struct btrfs_extent_item);
+ }
}
if (flags & BTRFS_EXTENT_FLAG_TREE_BLOCK)
--
2.24.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/3] btrfs-progs: check/original: Repair extent item generation
2020-01-17 7:29 [PATCH 0/3] btrfs-progs: Do proper extent item generation repair Qu Wenruo
2020-01-17 7:29 ` [PATCH 1/3] btrfs-progs: check/lowmem: Repair invalid extent item generation Qu Wenruo
@ 2020-01-17 7:29 ` Qu Wenruo
2020-01-17 7:29 ` [PATCH 3/3] btrfs-progs: tests/fsck-044: Enable repair test for invalid " Qu Wenruo
2020-01-17 14:28 ` [PATCH 0/3] btrfs-progs: Do proper extent item generation repair Josef Bacik
3 siblings, 0 replies; 6+ messages in thread
From: Qu Wenruo @ 2020-01-17 7:29 UTC (permalink / raw)
To: linux-btrfs
Just like lowmem mode, reset extent item generation using current
transid.
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
check/main.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 66 insertions(+)
diff --git a/check/main.c b/check/main.c
index 4115049a1529..74316eab85ec 100644
--- a/check/main.c
+++ b/check/main.c
@@ -7885,6 +7885,65 @@ static int record_unaligned_extent_rec(struct btrfs_fs_info *fs_info,
return ret;
}
+static int reset_extent_item_gen(struct btrfs_fs_info *fs_info,
+ struct extent_record *rec)
+{
+ struct btrfs_root *root = fs_info->extent_root;
+ struct btrfs_trans_handle *trans;
+ struct btrfs_extent_item *ei;
+ struct extent_buffer *leaf;
+ struct btrfs_path path;
+ struct btrfs_key key;
+ u64 transid;
+ int slot;
+ int ret;
+
+ key.objectid = rec->start;
+ if (rec->metadata && btrfs_fs_incompat(fs_info, SKINNY_METADATA)) {
+ key.type = BTRFS_METADATA_ITEM_KEY;
+ key.offset = rec->info_level;
+ } else {
+ key.type = BTRFS_EXTENT_ITEM_KEY;
+ key.offset = rec->max_size;
+ }
+ btrfs_init_path(&path);
+ trans = btrfs_start_transaction(root, 1);
+ if (IS_ERR(trans)) {
+ ret = PTR_ERR(trans);
+ errno = -ret;
+ error("failed to start transaction: %m");
+ return ret;
+ }
+ transid = trans->transid;
+
+ ret = btrfs_search_slot(trans, root, &key, &path, 0, 1);
+ if (ret < 0) {
+ errno = ret;
+ error("failed to search extent tree: %m");
+ goto abort;
+ }
+ if (ret > 0) {
+ ret = -ENOENT;
+ error("failed to find extent item for bytenr %llu",
+ rec->start);
+ goto abort;
+ }
+ slot = path.slots[0];
+ leaf = path.nodes[0];
+ ei = btrfs_item_ptr(leaf, slot, struct btrfs_extent_item);
+ btrfs_set_extent_generation(leaf, ei, transid);
+ ret = btrfs_commit_transaction(trans, root);
+ if (!ret)
+ printf("reset extent item generation to %llu for extent %llu\n",
+ transid, key.objectid);
+ btrfs_release_path(&path);
+ return ret;
+abort:
+ btrfs_abort_transaction(trans, ret);
+ btrfs_release_path(&path);
+ return ret;
+}
+
static int check_extent_refs(struct btrfs_root *root,
struct cache_tree *extent_cache)
{
@@ -7962,6 +8021,7 @@ static int check_extent_refs(struct btrfs_root *root,
while (1) {
int cur_err = 0;
int fix = 0;
+ bool reset_gen = false;
cache = search_cache_extent(extent_cache, 0);
if (!cache)
@@ -7979,6 +8039,7 @@ static int check_extent_refs(struct btrfs_root *root,
"invalid generation for extent %llu, have %llu expect (0, %llu]",
rec->start, rec->generation,
super_gen + 1);
+ reset_gen = true;
cur_err = 1;
}
if (rec->refs != rec->extent_item_refs) {
@@ -8019,6 +8080,11 @@ static int check_extent_refs(struct btrfs_root *root,
cur_err = 1;
}
+ if (repair && reset_gen) {
+ ret = reset_extent_item_gen(root->fs_info, rec);
+ if (ret)
+ goto repair_abort;
+ }
if (repair && fix) {
ret = fixup_extent_refs(root->fs_info, extent_cache,
rec);
--
2.24.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/3] btrfs-progs: tests/fsck-044: Enable repair test for invalid extent item generation
2020-01-17 7:29 [PATCH 0/3] btrfs-progs: Do proper extent item generation repair Qu Wenruo
2020-01-17 7:29 ` [PATCH 1/3] btrfs-progs: check/lowmem: Repair invalid extent item generation Qu Wenruo
2020-01-17 7:29 ` [PATCH 2/3] btrfs-progs: check/original: Repair " Qu Wenruo
@ 2020-01-17 7:29 ` Qu Wenruo
2020-01-17 14:28 ` [PATCH 0/3] btrfs-progs: Do proper extent item generation repair Josef Bacik
3 siblings, 0 replies; 6+ messages in thread
From: Qu Wenruo @ 2020-01-17 7:29 UTC (permalink / raw)
To: linux-btrfs
Since both original and lowmem mode can repair it, make that test case
to cover both detection and repair.
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
.../.lowmem_repairable | 0
.../test.sh | 19 -------------------
2 files changed, 19 deletions(-)
create mode 100644 tests/fsck-tests/044-invalid-extent-item-generation/.lowmem_repairable
delete mode 100755 tests/fsck-tests/044-invalid-extent-item-generation/test.sh
diff --git a/tests/fsck-tests/044-invalid-extent-item-generation/.lowmem_repairable b/tests/fsck-tests/044-invalid-extent-item-generation/.lowmem_repairable
new file mode 100644
index 000000000000..e69de29bb2d1
diff --git a/tests/fsck-tests/044-invalid-extent-item-generation/test.sh b/tests/fsck-tests/044-invalid-extent-item-generation/test.sh
deleted file mode 100755
index 2b88a3c7b3bb..000000000000
--- a/tests/fsck-tests/044-invalid-extent-item-generation/test.sh
+++ /dev/null
@@ -1,19 +0,0 @@
-#!/bin/bash
-#
-# Due to a bug in --init-extent-tree option, we may create bad generation
-# number for data extents.
-#
-# This test case will ensure btrfs check can at least detect such problem,
-# just like kernel tree-checker.
-
-source "$TEST_TOP/common"
-
-check_prereq btrfs
-
-check_image() {
- run_mustfail \
- "btrfs check failed to detect invalid extent item generation" \
- "$TOP/btrfs" check "$1"
-}
-
-check_all_images
--
2.24.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 0/3] btrfs-progs: Do proper extent item generation repair
2020-01-17 7:29 [PATCH 0/3] btrfs-progs: Do proper extent item generation repair Qu Wenruo
` (2 preceding siblings ...)
2020-01-17 7:29 ` [PATCH 3/3] btrfs-progs: tests/fsck-044: Enable repair test for invalid " Qu Wenruo
@ 2020-01-17 14:28 ` Josef Bacik
2020-01-18 1:08 ` Qu Wenruo
3 siblings, 1 reply; 6+ messages in thread
From: Josef Bacik @ 2020-01-17 14:28 UTC (permalink / raw)
To: Qu Wenruo, linux-btrfs
On 1/17/20 2:29 AM, Qu Wenruo wrote:
> Before this patchset, the only way to repair invalid extent item
> generation is to use --init-extent-tree, which is really a bad idea.
>
> To rebuild the whole extent tree just for one corrupted extent item?
> I must be insane at that time.
>
> This patch introduces the proper extent item generation repair
> functionality for both mode, and alter existing test case to also test
> repair.
>
> Qu Wenruo (3):
> btrfs-progs: check/lowmem: Repair invalid extent item generation
> btrfs-progs: check/original: Repair extent item generation
> btrfs-progs: tests/fsck-044: Enable repair test for invalid extent
> item generation
>
> check/main.c | 66 +++++++++++++++++
> check/mode-lowmem.c | 74 +++++++++++++++++++
> .../.lowmem_repairable | 0
> .../test.sh | 19 -----
> 4 files changed, 140 insertions(+), 19 deletions(-)
> create mode 100644 tests/fsck-tests/044-invalid-extent-item-generation/.lowmem_repairable
> delete mode 100755 tests/fsck-tests/044-invalid-extent-item-generation/test.sh
>
If we have a generation > super generation that means that block is from the
future and we shouldn't trust anything in it right? I haven't touched this code
in a while, but that just meant we threw it away and any extent references that
were in that block were just re-created. Is that not what's happening now?
This seems like a bad way to go about fixing this particular problem. Thanks,
Josef
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 0/3] btrfs-progs: Do proper extent item generation repair
2020-01-17 14:28 ` [PATCH 0/3] btrfs-progs: Do proper extent item generation repair Josef Bacik
@ 2020-01-18 1:08 ` Qu Wenruo
0 siblings, 0 replies; 6+ messages in thread
From: Qu Wenruo @ 2020-01-18 1:08 UTC (permalink / raw)
To: Josef Bacik, Qu Wenruo, linux-btrfs
[-- Attachment #1.1: Type: text/plain, Size: 2300 bytes --]
On 2020/1/17 下午10:28, Josef Bacik wrote:
> On 1/17/20 2:29 AM, Qu Wenruo wrote:
>> Before this patchset, the only way to repair invalid extent item
>> generation is to use --init-extent-tree, which is really a bad idea.
>>
>> To rebuild the whole extent tree just for one corrupted extent item?
>> I must be insane at that time.
>>
>> This patch introduces the proper extent item generation repair
>> functionality for both mode, and alter existing test case to also test
>> repair.
>>
>> Qu Wenruo (3):
>> btrfs-progs: check/lowmem: Repair invalid extent item generation
>> btrfs-progs: check/original: Repair extent item generation
>> btrfs-progs: tests/fsck-044: Enable repair test for invalid extent
>> item generation
>>
>> check/main.c | 66 +++++++++++++++++
>> check/mode-lowmem.c | 74 +++++++++++++++++++
>> .../.lowmem_repairable | 0
>> .../test.sh | 19 -----
>> 4 files changed, 140 insertions(+), 19 deletions(-)
>> create mode 100644
>> tests/fsck-tests/044-invalid-extent-item-generation/.lowmem_repairable
>> delete mode 100755
>> tests/fsck-tests/044-invalid-extent-item-generation/test.sh
>>
>
> If we have a generation > super generation that means that block is from
> the future and we shouldn't trust anything in it right?
From current report, it's mostly caused by:
- A bug around 2014
- Memory corruption
For the later case, as long as it's the only bug, it's easy to fix.
For the former case, although we don't have a concrete cause, it doesn't
seem to cause tons of similar problems.
Either way, from current reports they can be fixed, so I think it's
kinda OK to do such simple fix instead of always go slow --init-extent-tree.
Thanks,
Qu
> I haven't
> touched this code in a while, but that just meant we threw it away and
> any extent references that were in that block were just re-created. Is
> that not what's happening now? This seems like a bad way to go about
> fixing this particular problem. Thanks,
>
> Josef
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-01-18 1:09 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-17 7:29 [PATCH 0/3] btrfs-progs: Do proper extent item generation repair Qu Wenruo
2020-01-17 7:29 ` [PATCH 1/3] btrfs-progs: check/lowmem: Repair invalid extent item generation Qu Wenruo
2020-01-17 7:29 ` [PATCH 2/3] btrfs-progs: check/original: Repair " Qu Wenruo
2020-01-17 7:29 ` [PATCH 3/3] btrfs-progs: tests/fsck-044: Enable repair test for invalid " Qu Wenruo
2020-01-17 14:28 ` [PATCH 0/3] btrfs-progs: Do proper extent item generation repair Josef Bacik
2020-01-18 1:08 ` Qu Wenruo
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).