Re: [PATCH 11/13] btrfs-progs: check: Delete file extent item with unaligned extent backref
On 2018/11/7 下午3:04, Su Yanjun wrote: > > > On 11/7/2018 2:38 PM, Qu Wenruo wrote: >> >> On 2018/11/7 下午2:21, Su Yanjun wrote: >>> >>> On 10/24/2018 8:45 AM, Qu Wenruo wrote: On 2018/10/23 下午5:41, Su Yue wrote: > From: Su Yanjun > > In original mode, if some file extent item has unaligned extent > backref, > fixup_extent_refs can't repair it. This patch will check extent > alignment > then delete file extent with unaligned extent backref. This looks a little strange to me. You mean, an unaligned FILE EXTENT has an unaligned EXTENT_ITEM? Then why not just delete the EXTENT_ITEM directly? No need to go back checking if it has a corresponding EXTENT_DATA since unaligned one is definitely corrupted. For corrupted EXTENT_DATA, it should get deleted when we check fs tree. This would save you a lot of codes. Thanks, Qu >>> The situation is that the file extent has wrong extent backref, actually >>> it doesn't exist. >> Did you mean extent EXTENT_ITEM key's objectid is unaligned? >> >> Would you please give an example on this case? Like: >> ( EXTENT_DATA >> disk bytenr disk len >> >> And its backref like: >> ( EXTENT_ITEM ) >> >> And then mark where the number is incorrect. >> >> Thanks, >> Qu > > As in /btrfs-progs/tests/fsck-tests/001-bad-file-extent-bytenr case: > > item 7 key (257 EXTENT_DATA 0) itemoff 3453 itemsize 53 > > generation 6 type 1 (regular) > > extent data disk byte 755944791 nr 1048576 > ^ > > extent data offset 0 nr 1048576 ram 1048576 > > extent compression 0 (none) Then there is no "unaligned extent backref". It's just a unaligned disk bytenr of a file extent. Nothing to do with backref. Please update the commit message to avoid such confusing words, and include above info, which is pretty easy to understand. Thanks, Qu > > Thanks, > > Su > >>> Thanks, >>> Su >>> >>> >>> >>> >>> >> > > >
Re: [PATCH 11/13] btrfs-progs: check: Delete file extent item with unaligned extent backref
On 11/7/2018 2:38 PM, Qu Wenruo wrote: On 2018/11/7 下午2:21, Su Yanjun wrote: On 10/24/2018 8:45 AM, Qu Wenruo wrote: On 2018/10/23 下午5:41, Su Yue wrote: From: Su Yanjun In original mode, if some file extent item has unaligned extent backref, fixup_extent_refs can't repair it. This patch will check extent alignment then delete file extent with unaligned extent backref. This looks a little strange to me. You mean, an unaligned FILE EXTENT has an unaligned EXTENT_ITEM? Then why not just delete the EXTENT_ITEM directly? No need to go back checking if it has a corresponding EXTENT_DATA since unaligned one is definitely corrupted. For corrupted EXTENT_DATA, it should get deleted when we check fs tree. This would save you a lot of codes. Thanks, Qu The situation is that the file extent has wrong extent backref, actually it doesn't exist. Did you mean extent EXTENT_ITEM key's objectid is unaligned? Would you please give an example on this case? Like: ( EXTENT_DATA disk bytenr disk len And its backref like: ( EXTENT_ITEM ) And then mark where the number is incorrect. Thanks, Qu As in /btrfs-progs/tests/fsck-tests/001-bad-file-extent-bytenr case: item 7 key (257 EXTENT_DATA 0) itemoff 3453 itemsize 53 generation 6 type 1 (regular) extent data disk byte 755944791 nr 1048576 ^ extent data offset 0 nr 1048576 ram 1048576 extent compression 0 (none) Thanks, Su Thanks, Su
Re: [PATCH 11/13] btrfs-progs: check: Delete file extent item with unaligned extent backref
On 2018/11/7 下午2:21, Su Yanjun wrote: > > > On 10/24/2018 8:45 AM, Qu Wenruo wrote: >> >> On 2018/10/23 下午5:41, Su Yue wrote: >>> From: Su Yanjun >>> >>> In original mode, if some file extent item has unaligned extent backref, >>> fixup_extent_refs can't repair it. This patch will check extent >>> alignment >>> then delete file extent with unaligned extent backref. >> This looks a little strange to me. >> >> You mean, an unaligned FILE EXTENT has an unaligned EXTENT_ITEM? >> >> Then why not just delete the EXTENT_ITEM directly? No need to go back >> checking if it has a corresponding EXTENT_DATA since unaligned one is >> definitely corrupted. >> >> For corrupted EXTENT_DATA, it should get deleted when we check fs tree. >> >> This would save you a lot of codes. >> >> Thanks, >> Qu > The situation is that the file extent has wrong extent backref, actually > it doesn't exist. Did you mean extent EXTENT_ITEM key's objectid is unaligned? Would you please give an example on this case? Like: ( EXTENT_DATA disk bytenr disk len And its backref like: ( EXTENT_ITEM ) And then mark where the number is incorrect. Thanks, Qu > > Thanks, > Su > > > > >
Re: [PATCH 11/13] btrfs-progs: check: Delete file extent item with unaligned extent backref
On 10/24/2018 8:45 AM, Qu Wenruo wrote: On 2018/10/23 下午5:41, Su Yue wrote: From: Su Yanjun In original mode, if some file extent item has unaligned extent backref, fixup_extent_refs can't repair it. This patch will check extent alignment then delete file extent with unaligned extent backref. This looks a little strange to me. You mean, an unaligned FILE EXTENT has an unaligned EXTENT_ITEM? Then why not just delete the EXTENT_ITEM directly? No need to go back checking if it has a corresponding EXTENT_DATA since unaligned one is definitely corrupted. For corrupted EXTENT_DATA, it should get deleted when we check fs tree. This would save you a lot of codes. Thanks, Qu The situation is that the file extent has wrong extent backref, actually it doesn't exist. Thanks, Su
Re: [PATCH 11/13] btrfs-progs: check: Delete file extent item with unaligned extent backref
On 2018/10/23 下午5:41, Su Yue wrote:
> From: Su Yanjun
>
> In original mode, if some file extent item has unaligned extent backref,
> fixup_extent_refs can't repair it. This patch will check extent alignment
> then delete file extent with unaligned extent backref.
This looks a little strange to me.
You mean, an unaligned FILE EXTENT has an unaligned EXTENT_ITEM?
Then why not just delete the EXTENT_ITEM directly? No need to go back
checking if it has a corresponding EXTENT_DATA since unaligned one is
definitely corrupted.
For corrupted EXTENT_DATA, it should get deleted when we check fs tree.
This would save you a lot of codes.
Thanks,
Qu
>
> Signed-off-by: Su Yanjun
> ---
> check/main.c | 278 +-
> check/mode-original.h | 13 ++
> ctree.h | 2 +
> disk-io.c | 1 +
> 4 files changed, 293 insertions(+), 1 deletion(-)
>
> diff --git a/check/main.c b/check/main.c
> index 90d9fd570287..b5e68b3241e5 100644
> --- a/check/main.c
> +++ b/check/main.c
> @@ -460,6 +460,8 @@ static struct inode_record *clone_inode_rec(struct
> inode_record *orig_rec)
> struct inode_backref *backref;
> struct inode_backref *orig;
> struct inode_backref *tmp;
> + struct unaligned_extent_rec_t *src;
> + struct unaligned_extent_rec_t *dst;
> struct rb_node *rb;
> size_t size;
> int ret;
> @@ -470,6 +472,7 @@ static struct inode_record *clone_inode_rec(struct
> inode_record *orig_rec)
> memcpy(rec, orig_rec, sizeof(*rec));
> rec->refs = 1;
> INIT_LIST_HEAD(>backrefs);
> + INIT_LIST_HEAD(>unaligned_extent_recs);
> rec->holes = RB_ROOT;
>
> list_for_each_entry(orig, _rec->backrefs, list) {
> @@ -483,6 +486,17 @@ static struct inode_record *clone_inode_rec(struct
> inode_record *orig_rec)
> list_add_tail(>list, >backrefs);
> }
>
> + list_for_each_entry(src, _rec->unaligned_extent_recs, list) {
> + size = sizeof(*src);
> + dst = malloc(size);
> + if (!dst) {
> + ret = -ENOMEM;
> + goto cleanup;
> + }
> + memcpy(dst, src, size);
> + list_add_tail(>list, >unaligned_extent_recs);
> + }
> +
> ret = copy_file_extent_holes(>holes, _rec->holes);
> if (ret < 0)
> goto cleanup_rb;
> @@ -506,6 +520,13 @@ cleanup:
> free(orig);
> }
>
> + if (!list_empty(>unaligned_extent_recs))
> + list_for_each_entry_safe(src, dst, >unaligned_extent_recs,
> + list) {
> + list_del(>list);
> + free(src);
> + }
> +
> free(rec);
>
> return ERR_PTR(ret);
> @@ -643,6 +664,7 @@ static struct inode_record *get_inode_rec(struct
> cache_tree *inode_cache,
> rec->extent_start = (u64)-1;
> rec->refs = 1;
> INIT_LIST_HEAD(>backrefs);
> + INIT_LIST_HEAD(>unaligned_extent_recs);
> rec->holes = RB_ROOT;
>
> node = malloc(sizeof(*node));
> @@ -664,6 +686,18 @@ static struct inode_record *get_inode_rec(struct
> cache_tree *inode_cache,
> return rec;
> }
>
> +static void free_unaligned_extent_recs(struct list_head
> *unaligned_extent_recs)
> +{
> + struct unaligned_extent_rec_t *urec;
> +
> + while (!list_empty(unaligned_extent_recs)) {
> + urec = list_entry(unaligned_extent_recs->next,
> + struct unaligned_extent_rec_t, list);
> + list_del(>list);
> + free(urec);
> + }
> +}
> +
> static void free_inode_rec(struct inode_record *rec)
> {
> struct inode_backref *backref;
> @@ -676,6 +710,7 @@ static void free_inode_rec(struct inode_record *rec)
> list_del(>list);
> free(backref);
> }
> + free_unaligned_extent_recs(>unaligned_extent_recs);
> free_file_extent_holes(>holes);
> free(rec);
> }
> @@ -2474,18 +2509,154 @@ out:
> return ret;
> }
>
> +static int btrfs_delete_item(struct btrfs_trans_handle *trans,
> + struct btrfs_root *root, struct btrfs_key *key)
> +{
> + struct btrfs_path path;
> + int ret = 0;
> +
> + btrfs_init_path();
> +
> + ret = btrfs_search_slot(trans, root, key, , -1, 1);
> + if (ret) {
> + if (ret > 0)
> + ret = -ENOENT;
> +
> + btrfs_release_path();
> + return ret;
> + }
> +
> + ret = btrfs_del_item(trans, root, );
> +
> + btrfs_release_path();
> + return ret;
> +}
> +
> +static int find_file_extent_offset_by_bytenr(struct btrfs_root *root,
> + u64 owner, u64 bytenr, u64 *offset_ret)
> +{
> + int ret = 0;
> + struct btrfs_path path;
> + struct btrfs_key key;
> + struct btrfs_key found_key;
> + struct
[PATCH 11/13] btrfs-progs: check: Delete file extent item with unaligned extent backref
From: Su Yanjun
In original mode, if some file extent item has unaligned extent backref,
fixup_extent_refs can't repair it. This patch will check extent alignment
then delete file extent with unaligned extent backref.
Signed-off-by: Su Yanjun
---
check/main.c | 278 +-
check/mode-original.h | 13 ++
ctree.h | 2 +
disk-io.c | 1 +
4 files changed, 293 insertions(+), 1 deletion(-)
diff --git a/check/main.c b/check/main.c
index 90d9fd570287..b5e68b3241e5 100644
--- a/check/main.c
+++ b/check/main.c
@@ -460,6 +460,8 @@ static struct inode_record *clone_inode_rec(struct
inode_record *orig_rec)
struct inode_backref *backref;
struct inode_backref *orig;
struct inode_backref *tmp;
+ struct unaligned_extent_rec_t *src;
+ struct unaligned_extent_rec_t *dst;
struct rb_node *rb;
size_t size;
int ret;
@@ -470,6 +472,7 @@ static struct inode_record *clone_inode_rec(struct
inode_record *orig_rec)
memcpy(rec, orig_rec, sizeof(*rec));
rec->refs = 1;
INIT_LIST_HEAD(>backrefs);
+ INIT_LIST_HEAD(>unaligned_extent_recs);
rec->holes = RB_ROOT;
list_for_each_entry(orig, _rec->backrefs, list) {
@@ -483,6 +486,17 @@ static struct inode_record *clone_inode_rec(struct
inode_record *orig_rec)
list_add_tail(>list, >backrefs);
}
+ list_for_each_entry(src, _rec->unaligned_extent_recs, list) {
+ size = sizeof(*src);
+ dst = malloc(size);
+ if (!dst) {
+ ret = -ENOMEM;
+ goto cleanup;
+ }
+ memcpy(dst, src, size);
+ list_add_tail(>list, >unaligned_extent_recs);
+ }
+
ret = copy_file_extent_holes(>holes, _rec->holes);
if (ret < 0)
goto cleanup_rb;
@@ -506,6 +520,13 @@ cleanup:
free(orig);
}
+ if (!list_empty(>unaligned_extent_recs))
+ list_for_each_entry_safe(src, dst, >unaligned_extent_recs,
+ list) {
+ list_del(>list);
+ free(src);
+ }
+
free(rec);
return ERR_PTR(ret);
@@ -643,6 +664,7 @@ static struct inode_record *get_inode_rec(struct cache_tree
*inode_cache,
rec->extent_start = (u64)-1;
rec->refs = 1;
INIT_LIST_HEAD(>backrefs);
+ INIT_LIST_HEAD(>unaligned_extent_recs);
rec->holes = RB_ROOT;
node = malloc(sizeof(*node));
@@ -664,6 +686,18 @@ static struct inode_record *get_inode_rec(struct
cache_tree *inode_cache,
return rec;
}
+static void free_unaligned_extent_recs(struct list_head *unaligned_extent_recs)
+{
+ struct unaligned_extent_rec_t *urec;
+
+ while (!list_empty(unaligned_extent_recs)) {
+ urec = list_entry(unaligned_extent_recs->next,
+ struct unaligned_extent_rec_t, list);
+ list_del(>list);
+ free(urec);
+ }
+}
+
static void free_inode_rec(struct inode_record *rec)
{
struct inode_backref *backref;
@@ -676,6 +710,7 @@ static void free_inode_rec(struct inode_record *rec)
list_del(>list);
free(backref);
}
+ free_unaligned_extent_recs(>unaligned_extent_recs);
free_file_extent_holes(>holes);
free(rec);
}
@@ -2474,18 +2509,154 @@ out:
return ret;
}
+static int btrfs_delete_item(struct btrfs_trans_handle *trans,
+ struct btrfs_root *root, struct btrfs_key *key)
+{
+ struct btrfs_path path;
+ int ret = 0;
+
+ btrfs_init_path();
+
+ ret = btrfs_search_slot(trans, root, key, , -1, 1);
+ if (ret) {
+ if (ret > 0)
+ ret = -ENOENT;
+
+ btrfs_release_path();
+ return ret;
+ }
+
+ ret = btrfs_del_item(trans, root, );
+
+ btrfs_release_path();
+ return ret;
+}
+
+static int find_file_extent_offset_by_bytenr(struct btrfs_root *root,
+ u64 owner, u64 bytenr, u64 *offset_ret)
+{
+ int ret = 0;
+ struct btrfs_path path;
+ struct btrfs_key key;
+ struct btrfs_key found_key;
+ struct btrfs_file_extent_item *fi;
+ struct extent_buffer *leaf;
+ u64 disk_bytenr;
+ int slot;
+
+ btrfs_init_path();
+
+ key.objectid = owner;
+ key.type = BTRFS_INODE_ITEM_KEY;
+ key.offset = 0;
+
+ ret = btrfs_search_slot(NULL, root, , , 0, 0);
+ if (ret) {
+ if (ret > 0)
+ ret = -ENOENT;
+ btrfs_release_path();
+ return ret;
+ }
+
+ btrfs_release_path();
+
+ key.objectid = owner;
+ key.type = BTRFS_EXTENT_DATA_KEY;
+
