Commit 0cad8f14 authored by Filipe Manana's avatar Filipe Manana Committed by David Sterba

btrfs: fix backref walking not returning all inode refs

When using the logical to ino ioctl v2, if the flag to ignore offsets of
file extent items (BTRFS_LOGICAL_INO_ARGS_IGNORE_OFFSET) is given, the
backref walking code ends up not returning references for all file offsets
of an inode that point to the given logical bytenr. This happens since
kernel 6.2, commit 6ce6ba53 ("btrfs: use a single argument for extent
offset in backref walking functions") because:

1) It mistakenly skipped the search for file extent items in a leaf that
   point to the target extent if that flag is given. Instead it should
   only skip the filtering done by check_extent_in_eb() - that is, it
   should not avoid the calls to that function (or find_extent_in_eb(),
   which uses it).

2) It was also not building a list of inode extent elements (struct
   extent_inode_elem) if we have multiple inode references for an extent
   when the ignore offset flag is given to the logical to ino ioctl - it
   would leave a single element, only the last one that was found.

These stem from the confusing old interface for backref walking functions
where we had an extent item offset argument that was a pointer to a u64
and another boolean argument that indicated if the offset should be
ignored, but the pointer could be NULL. That NULL case is used by
relocation, qgroup extent accounting and fiemap, simply to avoid building
the inode extent list for each reference, as it's not necessary for those
use cases and therefore avoids memory allocations and some computations.

Fix this by adding a boolean argument to the backref walk context
structure to indicate that the inode extent list should not be built,
make relocation set that argument to true and fix the backref walking
logic to skip the calls to check_extent_in_eb() and find_extent_in_eb()
only if this new argument is true, instead of 'ignore_extent_item_pos'
being true.

A test case for fstests will be added soon, to provide cover not only
for these cases but to the logical to ino ioctl in general as well, as
currently we do not have a test case for it.
Reported-by: default avatarVladimir Panteleev <git@vladimir.panteleev.md>
Link: https://lore.kernel.org/linux-btrfs/CAHhfkvwo=nmzrJSqZ2qMfF-rZB-ab6ahHnCD_sq9h4o8v+M7QQ@mail.gmail.com/
Fixes: 6ce6ba53 ("btrfs: use a single argument for extent offset in backref walking functions")
CC: stable@vger.kernel.org # 6.2+
Tested-by: default avatarVladimir Panteleev <git@vladimir.panteleev.md>
Signed-off-by: default avatarFilipe Manana <fdmanana@suse.com>
Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
parent 0004ff15
...@@ -45,7 +45,8 @@ static int check_extent_in_eb(struct btrfs_backref_walk_ctx *ctx, ...@@ -45,7 +45,8 @@ static int check_extent_in_eb(struct btrfs_backref_walk_ctx *ctx,
int root_count; int root_count;
bool cached; bool cached;
if (!btrfs_file_extent_compression(eb, fi) && if (!ctx->ignore_extent_item_pos &&
!btrfs_file_extent_compression(eb, fi) &&
!btrfs_file_extent_encryption(eb, fi) && !btrfs_file_extent_encryption(eb, fi) &&
!btrfs_file_extent_other_encoding(eb, fi)) { !btrfs_file_extent_other_encoding(eb, fi)) {
u64 data_offset; u64 data_offset;
...@@ -552,7 +553,7 @@ static int add_all_parents(struct btrfs_backref_walk_ctx *ctx, ...@@ -552,7 +553,7 @@ static int add_all_parents(struct btrfs_backref_walk_ctx *ctx,
count++; count++;
else else
goto next; goto next;
if (!ctx->ignore_extent_item_pos) { if (!ctx->skip_inode_ref_list) {
ret = check_extent_in_eb(ctx, &key, eb, fi, &eie); ret = check_extent_in_eb(ctx, &key, eb, fi, &eie);
if (ret == BTRFS_ITERATE_EXTENT_INODES_STOP || if (ret == BTRFS_ITERATE_EXTENT_INODES_STOP ||
ret < 0) ret < 0)
...@@ -564,7 +565,7 @@ static int add_all_parents(struct btrfs_backref_walk_ctx *ctx, ...@@ -564,7 +565,7 @@ static int add_all_parents(struct btrfs_backref_walk_ctx *ctx,
eie, (void **)&old, GFP_NOFS); eie, (void **)&old, GFP_NOFS);
if (ret < 0) if (ret < 0)
break; break;
if (!ret && !ctx->ignore_extent_item_pos) { if (!ret && !ctx->skip_inode_ref_list) {
while (old->next) while (old->next)
old = old->next; old = old->next;
old->next = eie; old->next = eie;
...@@ -1606,7 +1607,7 @@ static int find_parent_nodes(struct btrfs_backref_walk_ctx *ctx, ...@@ -1606,7 +1607,7 @@ static int find_parent_nodes(struct btrfs_backref_walk_ctx *ctx,
goto out; goto out;
} }
if (ref->count && ref->parent) { if (ref->count && ref->parent) {
if (!ctx->ignore_extent_item_pos && !ref->inode_list && if (!ctx->skip_inode_ref_list && !ref->inode_list &&
ref->level == 0) { ref->level == 0) {
struct btrfs_tree_parent_check check = { 0 }; struct btrfs_tree_parent_check check = { 0 };
struct extent_buffer *eb; struct extent_buffer *eb;
...@@ -1647,7 +1648,7 @@ static int find_parent_nodes(struct btrfs_backref_walk_ctx *ctx, ...@@ -1647,7 +1648,7 @@ static int find_parent_nodes(struct btrfs_backref_walk_ctx *ctx,
(void **)&eie, GFP_NOFS); (void **)&eie, GFP_NOFS);
if (ret < 0) if (ret < 0)
goto out; goto out;
if (!ret && !ctx->ignore_extent_item_pos) { if (!ret && !ctx->skip_inode_ref_list) {
/* /*
* We've recorded that parent, so we must extend * We've recorded that parent, so we must extend
* its inode list here. * its inode list here.
...@@ -1743,7 +1744,7 @@ int btrfs_find_all_leafs(struct btrfs_backref_walk_ctx *ctx) ...@@ -1743,7 +1744,7 @@ int btrfs_find_all_leafs(struct btrfs_backref_walk_ctx *ctx)
static int btrfs_find_all_roots_safe(struct btrfs_backref_walk_ctx *ctx) static int btrfs_find_all_roots_safe(struct btrfs_backref_walk_ctx *ctx)
{ {
const u64 orig_bytenr = ctx->bytenr; const u64 orig_bytenr = ctx->bytenr;
const bool orig_ignore_extent_item_pos = ctx->ignore_extent_item_pos; const bool orig_skip_inode_ref_list = ctx->skip_inode_ref_list;
bool roots_ulist_allocated = false; bool roots_ulist_allocated = false;
struct ulist_iterator uiter; struct ulist_iterator uiter;
int ret = 0; int ret = 0;
...@@ -1764,7 +1765,7 @@ static int btrfs_find_all_roots_safe(struct btrfs_backref_walk_ctx *ctx) ...@@ -1764,7 +1765,7 @@ static int btrfs_find_all_roots_safe(struct btrfs_backref_walk_ctx *ctx)
roots_ulist_allocated = true; roots_ulist_allocated = true;
} }
ctx->ignore_extent_item_pos = true; ctx->skip_inode_ref_list = true;
ULIST_ITER_INIT(&uiter); ULIST_ITER_INIT(&uiter);
while (1) { while (1) {
...@@ -1789,7 +1790,7 @@ static int btrfs_find_all_roots_safe(struct btrfs_backref_walk_ctx *ctx) ...@@ -1789,7 +1790,7 @@ static int btrfs_find_all_roots_safe(struct btrfs_backref_walk_ctx *ctx)
ulist_free(ctx->refs); ulist_free(ctx->refs);
ctx->refs = NULL; ctx->refs = NULL;
ctx->bytenr = orig_bytenr; ctx->bytenr = orig_bytenr;
ctx->ignore_extent_item_pos = orig_ignore_extent_item_pos; ctx->skip_inode_ref_list = orig_skip_inode_ref_list;
return ret; return ret;
} }
...@@ -1912,7 +1913,7 @@ int btrfs_is_data_extent_shared(struct btrfs_inode *inode, u64 bytenr, ...@@ -1912,7 +1913,7 @@ int btrfs_is_data_extent_shared(struct btrfs_inode *inode, u64 bytenr,
goto out_trans; goto out_trans;
} }
walk_ctx.ignore_extent_item_pos = true; walk_ctx.skip_inode_ref_list = true;
walk_ctx.trans = trans; walk_ctx.trans = trans;
walk_ctx.fs_info = fs_info; walk_ctx.fs_info = fs_info;
walk_ctx.refs = &ctx->refs; walk_ctx.refs = &ctx->refs;
......
...@@ -60,6 +60,12 @@ struct btrfs_backref_walk_ctx { ...@@ -60,6 +60,12 @@ struct btrfs_backref_walk_ctx {
* @extent_item_pos is ignored. * @extent_item_pos is ignored.
*/ */
bool ignore_extent_item_pos; bool ignore_extent_item_pos;
/*
* If true and bytenr corresponds to a data extent, then the inode list
* (each member describing inode number, file offset and root) is not
* added to each reference added to the @refs ulist.
*/
bool skip_inode_ref_list;
/* A valid transaction handle or NULL. */ /* A valid transaction handle or NULL. */
struct btrfs_trans_handle *trans; struct btrfs_trans_handle *trans;
/* /*
......
...@@ -3422,7 +3422,7 @@ int add_data_references(struct reloc_control *rc, ...@@ -3422,7 +3422,7 @@ int add_data_references(struct reloc_control *rc,
btrfs_release_path(path); btrfs_release_path(path);
ctx.bytenr = extent_key->objectid; ctx.bytenr = extent_key->objectid;
ctx.ignore_extent_item_pos = true; ctx.skip_inode_ref_list = true;
ctx.fs_info = rc->extent_root->fs_info; ctx.fs_info = rc->extent_root->fs_info;
ret = btrfs_find_all_leafs(&ctx); ret = btrfs_find_all_leafs(&ctx);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment