| Message ID | 20181023094147.7906-11-suy.fnst@cn.fujitsu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs-progs: fixes of file extent in original and lowmem check | expand |
On 2018/10/23 下午5:41, Su Yue wrote: > From: Su Yanjun <suyj.fnst@cn.fujitsu.com> > > It may cost more time to search all extent data of correspond files but > should not influence total speed too much cause that only corrupted > extent items are participated in. Sorry, I didn't really get the point of the modification from the commit message. Would you please explain the purpose of this patch first? Thanks, Qu > > Signed-off-by: Su Yanjun <suyj.fnst@cn.fujitsu.com> > --- > check/main.c | 110 ++++++++++++++++++++++++++++++++++++++++++--------- > 1 file changed, 92 insertions(+), 18 deletions(-) > > diff --git a/check/main.c b/check/main.c > index bd1f322e0f12..90d9fd570287 100644 > --- a/check/main.c > +++ b/check/main.c > @@ -7015,6 +7015,89 @@ out: > return ret ? ret : nr_del; > } > > +/* > + * Based extent backref item, we find all file extent items in the fs tree. By > + * the info we can rebuild the extent backref item > + */ > +static int __find_possible_backrefs(struct btrfs_root *root, > + u64 owner, u64 offset, u64 bytenr, u64 *refs_ret, > + u64 *bytes_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 backref_offset, disk_bytenr; > + int slot; > + > + btrfs_init_path(&path); > + > + key.objectid = owner; > + key.type = BTRFS_INODE_ITEM_KEY; > + key.offset = 0; > + > + ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0); > + if (ret > 0) > + ret = -ENOENT; > + if (ret) { > + btrfs_release_path(&path); > + return ret; > + } > + > + btrfs_release_path(&path); > + > + key.objectid = owner; > + key.type = BTRFS_EXTENT_DATA_KEY; > + key.offset = 0; > + > + ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0); > + if (ret < 0) { > + btrfs_release_path(&path); > + return ret; > + } > + > + while (1) { > + leaf = path.nodes[0]; > + slot = path.slots[0]; > + > + if (slot >= btrfs_header_nritems(leaf)) { > + ret = btrfs_next_leaf(root, &path); > + if (ret) { > + if (ret > 0) > + ret = 0; > + break; > + } > + > + leaf = path.nodes[0]; > + slot = path.slots[0]; > + } > + > + btrfs_item_key_to_cpu(leaf, &found_key, slot); > + if ((found_key.objectid != owner) || > + (found_key.type != BTRFS_EXTENT_DATA_KEY)) > + break; > + > + fi = btrfs_item_ptr(leaf, slot, > + struct btrfs_file_extent_item); > + > + backref_offset = found_key.offset - > + btrfs_file_extent_offset(leaf, fi); > + disk_bytenr = btrfs_file_extent_disk_bytenr(leaf, fi); > + *bytes_ret = btrfs_file_extent_disk_num_bytes(leaf, > + fi); > + if ((disk_bytenr == bytenr) && > + (backref_offset == offset)) { > + (*refs_ret)++; > + } > + path.slots[0]++; > + } > + > + btrfs_release_path(&path); > + return ret; > +} > + > static int find_possible_backrefs(struct btrfs_fs_info *info, > struct btrfs_path *path, > struct cache_tree *extent_cache, > @@ -7024,9 +7107,9 @@ static int find_possible_backrefs(struct btrfs_fs_info *info, > struct extent_backref *back, *tmp; > struct data_backref *dback; > struct cache_extent *cache; > - struct btrfs_file_extent_item *fi; > struct btrfs_key key; > u64 bytenr, bytes; > + u64 refs; > int ret; > > rbtree_postorder_for_each_entry_safe(back, tmp, > @@ -7054,24 +7137,15 @@ static int find_possible_backrefs(struct btrfs_fs_info *info, > if (IS_ERR(root)) > return PTR_ERR(root); > > - key.objectid = dback->owner; > - key.type = BTRFS_EXTENT_DATA_KEY; > - key.offset = dback->offset; > - ret = btrfs_search_slot(NULL, root, &key, path, 0, 0); > - if (ret) { > - btrfs_release_path(path); > - if (ret < 0) > - return ret; > - /* Didn't find it, we can carry on */ > - ret = 0; > + refs = 0; > + bytes = 0; > + ret = __find_possible_backrefs(root, dback->owner, > + dback->offset, rec->start, &refs, &bytes); > + if (ret) > continue; > - } > > - fi = btrfs_item_ptr(path->nodes[0], path->slots[0], > - struct btrfs_file_extent_item); > - bytenr = btrfs_file_extent_disk_bytenr(path->nodes[0], fi); > - bytes = btrfs_file_extent_disk_num_bytes(path->nodes[0], fi); > - btrfs_release_path(path); > + bytenr = rec->start; > + > cache = lookup_cache_extent(extent_cache, bytenr, 1); > if (cache) { > struct extent_record *tmp; > @@ -7090,7 +7164,7 @@ static int find_possible_backrefs(struct btrfs_fs_info *info, > continue; > } > > - dback->found_ref += 1; > + dback->found_ref += refs; > dback->disk_bytenr = bytenr; > dback->bytes = bytes; > >
On 10/24/2018 8:34 AM, Qu Wenruo wrote: > > On 2018/10/23 下午5:41, Su Yue wrote: >> From: Su Yanjun <suyj.fnst@cn.fujitsu.com> >> >> It may cost more time to search all extent data of correspond files but >> should not influence total speed too much cause that only corrupted >> extent items are participated in. > Sorry, I didn't really get the point of the modification from the commit > message. > > Would you please explain the purpose of this patch first? > > Thanks, > Qu In find_possible_backrefs() function: key.objectid = dback->owner; key.type = BTRFS_EXTENT_DATA_KEY; key.offset = dback->offset;// ret = btrfs_search_slot(NULL, root, &key, path, 0, 0); 'dback->offset' is not the offset in file range, we'll find wrong file extent. Thanks, Su >> Signed-off-by: Su Yanjun <suyj.fnst@cn.fujitsu.com> >> --- >> check/main.c | 110 ++++++++++++++++++++++++++++++++++++++++++--------- >> 1 file changed, 92 insertions(+), 18 deletions(-) >> >> diff --git a/check/main.c b/check/main.c >> index bd1f322e0f12..90d9fd570287 100644 >> --- a/check/main.c >> +++ b/check/main.c >> @@ -7015,6 +7015,89 @@ out: >> return ret ? ret : nr_del; >> } >> >> +/* >> + * Based extent backref item, we find all file extent items in the fs tree. By >> + * the info we can rebuild the extent backref item >> + */ >> +static int __find_possible_backrefs(struct btrfs_root *root, >> + u64 owner, u64 offset, u64 bytenr, u64 *refs_ret, >> + u64 *bytes_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 backref_offset, disk_bytenr; >> + int slot; >> + >> + btrfs_init_path(&path); >> + >> + key.objectid = owner; >> + key.type = BTRFS_INODE_ITEM_KEY; >> + key.offset = 0; >> + >> + ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0); >> + if (ret > 0) >> + ret = -ENOENT; >> + if (ret) { >> + btrfs_release_path(&path); >> + return ret; >> + } >> + >> + btrfs_release_path(&path); >> + >> + key.objectid = owner; >> + key.type = BTRFS_EXTENT_DATA_KEY; >> + key.offset = 0; >> + >> + ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0); >> + if (ret < 0) { >> + btrfs_release_path(&path); >> + return ret; >> + } >> + >> + while (1) { >> + leaf = path.nodes[0]; >> + slot = path.slots[0]; >> + >> + if (slot >= btrfs_header_nritems(leaf)) { >> + ret = btrfs_next_leaf(root, &path); >> + if (ret) { >> + if (ret > 0) >> + ret = 0; >> + break; >> + } >> + >> + leaf = path.nodes[0]; >> + slot = path.slots[0]; >> + } >> + >> + btrfs_item_key_to_cpu(leaf, &found_key, slot); >> + if ((found_key.objectid != owner) || >> + (found_key.type != BTRFS_EXTENT_DATA_KEY)) >> + break; >> + >> + fi = btrfs_item_ptr(leaf, slot, >> + struct btrfs_file_extent_item); >> + >> + backref_offset = found_key.offset - >> + btrfs_file_extent_offset(leaf, fi); >> + disk_bytenr = btrfs_file_extent_disk_bytenr(leaf, fi); >> + *bytes_ret = btrfs_file_extent_disk_num_bytes(leaf, >> + fi); >> + if ((disk_bytenr == bytenr) && >> + (backref_offset == offset)) { >> + (*refs_ret)++; >> + } >> + path.slots[0]++; >> + } >> + >> + btrfs_release_path(&path); >> + return ret; >> +} >> + >> static int find_possible_backrefs(struct btrfs_fs_info *info, >> struct btrfs_path *path, >> struct cache_tree *extent_cache, >> @@ -7024,9 +7107,9 @@ static int find_possible_backrefs(struct btrfs_fs_info *info, >> struct extent_backref *back, *tmp; >> struct data_backref *dback; >> struct cache_extent *cache; >> - struct btrfs_file_extent_item *fi; >> struct btrfs_key key; >> u64 bytenr, bytes; >> + u64 refs; >> int ret; >> >> rbtree_postorder_for_each_entry_safe(back, tmp, >> @@ -7054,24 +7137,15 @@ static int find_possible_backrefs(struct btrfs_fs_info *info, >> if (IS_ERR(root)) >> return PTR_ERR(root); >> >> - key.objectid = dback->owner; >> - key.type = BTRFS_EXTENT_DATA_KEY; >> - key.offset = dback->offset; >> - ret = btrfs_search_slot(NULL, root, &key, path, 0, 0); >> - if (ret) { >> - btrfs_release_path(path); >> - if (ret < 0) >> - return ret; >> - /* Didn't find it, we can carry on */ >> - ret = 0; >> + refs = 0; >> + bytes = 0; >> + ret = __find_possible_backrefs(root, dback->owner, >> + dback->offset, rec->start, &refs, &bytes); >> + if (ret) >> continue; >> - } >> >> - fi = btrfs_item_ptr(path->nodes[0], path->slots[0], >> - struct btrfs_file_extent_item); >> - bytenr = btrfs_file_extent_disk_bytenr(path->nodes[0], fi); >> - bytes = btrfs_file_extent_disk_num_bytes(path->nodes[0], fi); >> - btrfs_release_path(path); >> + bytenr = rec->start; >> + >> cache = lookup_cache_extent(extent_cache, bytenr, 1); >> if (cache) { >> struct extent_record *tmp; >> @@ -7090,7 +7164,7 @@ static int find_possible_backrefs(struct btrfs_fs_info *info, >> continue; >> } >> >> - dback->found_ref += 1; >> + dback->found_ref += refs; >> dback->disk_bytenr = bytenr; >> dback->bytes = bytes; >> >> >
On 2018/11/7 下午2:28, Su Yanjun <suyj.fnst@cn.fujitsu.com> wrote: > > > On 10/24/2018 8:34 AM, Qu Wenruo wrote: >> >> On 2018/10/23 下午5:41, Su Yue wrote: >>> From: Su Yanjun <suyj.fnst@cn.fujitsu.com> >>> >>> It may cost more time to search all extent data of correspond files but >>> should not influence total speed too much cause that only corrupted >>> extent items are participated in. >> Sorry, I didn't really get the point of the modification from the commit >> message. >> >> Would you please explain the purpose of this patch first? >> >> Thanks, >> Qu > > In find_possible_backrefs() function: > > key.objectid = dback->owner; > key.type = BTRFS_EXTENT_DATA_KEY; > key.offset = dback->offset;// > ret = btrfs_search_slot(NULL, root, &key, path, 0, 0); > > 'dback->offset' is not the offset in file range, we'll find wrong file > extent. Now this makes sense. The original commit message doesn't touch this part, and focused on the side effect, not the problem itself. Please update the commit message. Thanks, Qu > > Thanks, > > Su > >>> Signed-off-by: Su Yanjun <suyj.fnst@cn.fujitsu.com> >>> --- >>> check/main.c | 110 ++++++++++++++++++++++++++++++++++++++++++--------- >>> 1 file changed, 92 insertions(+), 18 deletions(-) >>> >>> diff --git a/check/main.c b/check/main.c >>> index bd1f322e0f12..90d9fd570287 100644 >>> --- a/check/main.c >>> +++ b/check/main.c >>> @@ -7015,6 +7015,89 @@ out: >>> return ret ? ret : nr_del; >>> } >>> +/* >>> + * Based extent backref item, we find all file extent items in the >>> fs tree. By >>> + * the info we can rebuild the extent backref item >>> + */ >>> +static int __find_possible_backrefs(struct btrfs_root *root, >>> + u64 owner, u64 offset, u64 bytenr, u64 *refs_ret, >>> + u64 *bytes_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 backref_offset, disk_bytenr; >>> + int slot; >>> + >>> + btrfs_init_path(&path); >>> + >>> + key.objectid = owner; >>> + key.type = BTRFS_INODE_ITEM_KEY; >>> + key.offset = 0; >>> + >>> + ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0); >>> + if (ret > 0) >>> + ret = -ENOENT; >>> + if (ret) { >>> + btrfs_release_path(&path); >>> + return ret; >>> + } >>> + >>> + btrfs_release_path(&path); >>> + >>> + key.objectid = owner; >>> + key.type = BTRFS_EXTENT_DATA_KEY; >>> + key.offset = 0; >>> + >>> + ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0); >>> + if (ret < 0) { >>> + btrfs_release_path(&path); >>> + return ret; >>> + } >>> + >>> + while (1) { >>> + leaf = path.nodes[0]; >>> + slot = path.slots[0]; >>> + >>> + if (slot >= btrfs_header_nritems(leaf)) { >>> + ret = btrfs_next_leaf(root, &path); >>> + if (ret) { >>> + if (ret > 0) >>> + ret = 0; >>> + break; >>> + } >>> + >>> + leaf = path.nodes[0]; >>> + slot = path.slots[0]; >>> + } >>> + >>> + btrfs_item_key_to_cpu(leaf, &found_key, slot); >>> + if ((found_key.objectid != owner) || >>> + (found_key.type != BTRFS_EXTENT_DATA_KEY)) >>> + break; >>> + >>> + fi = btrfs_item_ptr(leaf, slot, >>> + struct btrfs_file_extent_item); >>> + >>> + backref_offset = found_key.offset - >>> + btrfs_file_extent_offset(leaf, fi); >>> + disk_bytenr = btrfs_file_extent_disk_bytenr(leaf, fi); >>> + *bytes_ret = btrfs_file_extent_disk_num_bytes(leaf, >>> + fi); >>> + if ((disk_bytenr == bytenr) && >>> + (backref_offset == offset)) { >>> + (*refs_ret)++; >>> + } >>> + path.slots[0]++; >>> + } >>> + >>> + btrfs_release_path(&path); >>> + return ret; >>> +} >>> + >>> static int find_possible_backrefs(struct btrfs_fs_info *info, >>> struct btrfs_path *path, >>> struct cache_tree *extent_cache, >>> @@ -7024,9 +7107,9 @@ static int find_possible_backrefs(struct >>> btrfs_fs_info *info, >>> struct extent_backref *back, *tmp; >>> struct data_backref *dback; >>> struct cache_extent *cache; >>> - struct btrfs_file_extent_item *fi; >>> struct btrfs_key key; >>> u64 bytenr, bytes; >>> + u64 refs; >>> int ret; >>> rbtree_postorder_for_each_entry_safe(back, tmp, >>> @@ -7054,24 +7137,15 @@ static int find_possible_backrefs(struct >>> btrfs_fs_info *info, >>> if (IS_ERR(root)) >>> return PTR_ERR(root); >>> - key.objectid = dback->owner; >>> - key.type = BTRFS_EXTENT_DATA_KEY; >>> - key.offset = dback->offset; >>> - ret = btrfs_search_slot(NULL, root, &key, path, 0, 0); >>> - if (ret) { >>> - btrfs_release_path(path); >>> - if (ret < 0) >>> - return ret; >>> - /* Didn't find it, we can carry on */ >>> - ret = 0; >>> + refs = 0; >>> + bytes = 0; >>> + ret = __find_possible_backrefs(root, dback->owner, >>> + dback->offset, rec->start, &refs, &bytes); >>> + if (ret) >>> continue; >>> - } >>> - fi = btrfs_item_ptr(path->nodes[0], path->slots[0], >>> - struct btrfs_file_extent_item); >>> - bytenr = btrfs_file_extent_disk_bytenr(path->nodes[0], fi); >>> - bytes = btrfs_file_extent_disk_num_bytes(path->nodes[0], fi); >>> - btrfs_release_path(path); >>> + bytenr = rec->start; >>> + >>> cache = lookup_cache_extent(extent_cache, bytenr, 1); >>> if (cache) { >>> struct extent_record *tmp; >>> @@ -7090,7 +7164,7 @@ static int find_possible_backrefs(struct >>> btrfs_fs_info *info, >>> continue; >>> } >>> - dback->found_ref += 1; >>> + dback->found_ref += refs; >>> dback->disk_bytenr = bytenr; >>> dback->bytes = bytes; >>> >> > > >
diff --git a/check/main.c b/check/main.c index bd1f322e0f12..90d9fd570287 100644 --- a/check/main.c +++ b/check/main.c @@ -7015,6 +7015,89 @@ out: return ret ? ret : nr_del; } +/* + * Based extent backref item, we find all file extent items in the fs tree. By + * the info we can rebuild the extent backref item + */ +static int __find_possible_backrefs(struct btrfs_root *root, + u64 owner, u64 offset, u64 bytenr, u64 *refs_ret, + u64 *bytes_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 backref_offset, disk_bytenr; + int slot; + + btrfs_init_path(&path); + + key.objectid = owner; + key.type = BTRFS_INODE_ITEM_KEY; + key.offset = 0; + + ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0); + if (ret > 0) + ret = -ENOENT; + if (ret) { + btrfs_release_path(&path); + return ret; + } + + btrfs_release_path(&path); + + key.objectid = owner; + key.type = BTRFS_EXTENT_DATA_KEY; + key.offset = 0; + + ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0); + if (ret < 0) { + btrfs_release_path(&path); + return ret; + } + + while (1) { + leaf = path.nodes[0]; + slot = path.slots[0]; + + if (slot >= btrfs_header_nritems(leaf)) { + ret = btrfs_next_leaf(root, &path); + if (ret) { + if (ret > 0) + ret = 0; + break; + } + + leaf = path.nodes[0]; + slot = path.slots[0]; + } + + btrfs_item_key_to_cpu(leaf, &found_key, slot); + if ((found_key.objectid != owner) || + (found_key.type != BTRFS_EXTENT_DATA_KEY)) + break; + + fi = btrfs_item_ptr(leaf, slot, + struct btrfs_file_extent_item); + + backref_offset = found_key.offset - + btrfs_file_extent_offset(leaf, fi); + disk_bytenr = btrfs_file_extent_disk_bytenr(leaf, fi); + *bytes_ret = btrfs_file_extent_disk_num_bytes(leaf, + fi); + if ((disk_bytenr == bytenr) && + (backref_offset == offset)) { + (*refs_ret)++; + } + path.slots[0]++; + } + + btrfs_release_path(&path); + return ret; +} + static int find_possible_backrefs(struct btrfs_fs_info *info, struct btrfs_path *path, struct cache_tree *extent_cache, @@ -7024,9 +7107,9 @@ static int find_possible_backrefs(struct btrfs_fs_info *info, struct extent_backref *back, *tmp; struct data_backref *dback; struct cache_extent *cache; - struct btrfs_file_extent_item *fi; struct btrfs_key key; u64 bytenr, bytes; + u64 refs; int ret; rbtree_postorder_for_each_entry_safe(back, tmp, @@ -7054,24 +7137,15 @@ static int find_possible_backrefs(struct btrfs_fs_info *info, if (IS_ERR(root)) return PTR_ERR(root); - key.objectid = dback->owner; - key.type = BTRFS_EXTENT_DATA_KEY; - key.offset = dback->offset; - ret = btrfs_search_slot(NULL, root, &key, path, 0, 0); - if (ret) { - btrfs_release_path(path); - if (ret < 0) - return ret; - /* Didn't find it, we can carry on */ - ret = 0; + refs = 0; + bytes = 0; + ret = __find_possible_backrefs(root, dback->owner, + dback->offset, rec->start, &refs, &bytes); + if (ret) continue; - } - fi = btrfs_item_ptr(path->nodes[0], path->slots[0], - struct btrfs_file_extent_item); - bytenr = btrfs_file_extent_disk_bytenr(path->nodes[0], fi); - bytes = btrfs_file_extent_disk_num_bytes(path->nodes[0], fi); - btrfs_release_path(path); + bytenr = rec->start; + cache = lookup_cache_extent(extent_cache, bytenr, 1); if (cache) { struct extent_record *tmp; @@ -7090,7 +7164,7 @@ static int find_possible_backrefs(struct btrfs_fs_info *info, continue; } - dback->found_ref += 1; + dback->found_ref += refs; dback->disk_bytenr = bytenr; dback->bytes = bytes;