Skip to content

Commit 0cad8f1

Browse files
fdmananakdave
authored andcommitted
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 6ce6ba5 ("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: Vladimir Panteleev <[email protected]> Link: https://lore.kernel.org/linux-btrfs/CAHhfkvwo=nmzrJSqZ2qMfF-rZB-ab6ahHnCD_sq9h4o8v+M7QQ@mail.gmail.com/ Fixes: 6ce6ba5 ("btrfs: use a single argument for extent offset in backref walking functions") CC: [email protected] # 6.2+ Tested-by: Vladimir Panteleev <[email protected]> Signed-off-by: Filipe Manana <[email protected]> Signed-off-by: David Sterba <[email protected]>
1 parent 0004ff1 commit 0cad8f1

File tree

3 files changed

+17
-10
lines changed

3 files changed

+17
-10
lines changed

fs/btrfs/backref.c

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,8 @@ static int check_extent_in_eb(struct btrfs_backref_walk_ctx *ctx,
4545
int root_count;
4646
bool cached;
4747

48-
if (!btrfs_file_extent_compression(eb, fi) &&
48+
if (!ctx->ignore_extent_item_pos &&
49+
!btrfs_file_extent_compression(eb, fi) &&
4950
!btrfs_file_extent_encryption(eb, fi) &&
5051
!btrfs_file_extent_other_encoding(eb, fi)) {
5152
u64 data_offset;
@@ -552,7 +553,7 @@ static int add_all_parents(struct btrfs_backref_walk_ctx *ctx,
552553
count++;
553554
else
554555
goto next;
555-
if (!ctx->ignore_extent_item_pos) {
556+
if (!ctx->skip_inode_ref_list) {
556557
ret = check_extent_in_eb(ctx, &key, eb, fi, &eie);
557558
if (ret == BTRFS_ITERATE_EXTENT_INODES_STOP ||
558559
ret < 0)
@@ -564,7 +565,7 @@ static int add_all_parents(struct btrfs_backref_walk_ctx *ctx,
564565
eie, (void **)&old, GFP_NOFS);
565566
if (ret < 0)
566567
break;
567-
if (!ret && !ctx->ignore_extent_item_pos) {
568+
if (!ret && !ctx->skip_inode_ref_list) {
568569
while (old->next)
569570
old = old->next;
570571
old->next = eie;
@@ -1606,7 +1607,7 @@ static int find_parent_nodes(struct btrfs_backref_walk_ctx *ctx,
16061607
goto out;
16071608
}
16081609
if (ref->count && ref->parent) {
1609-
if (!ctx->ignore_extent_item_pos && !ref->inode_list &&
1610+
if (!ctx->skip_inode_ref_list && !ref->inode_list &&
16101611
ref->level == 0) {
16111612
struct btrfs_tree_parent_check check = { 0 };
16121613
struct extent_buffer *eb;
@@ -1647,7 +1648,7 @@ static int find_parent_nodes(struct btrfs_backref_walk_ctx *ctx,
16471648
(void **)&eie, GFP_NOFS);
16481649
if (ret < 0)
16491650
goto out;
1650-
if (!ret && !ctx->ignore_extent_item_pos) {
1651+
if (!ret && !ctx->skip_inode_ref_list) {
16511652
/*
16521653
* We've recorded that parent, so we must extend
16531654
* its inode list here.
@@ -1743,7 +1744,7 @@ int btrfs_find_all_leafs(struct btrfs_backref_walk_ctx *ctx)
17431744
static int btrfs_find_all_roots_safe(struct btrfs_backref_walk_ctx *ctx)
17441745
{
17451746
const u64 orig_bytenr = ctx->bytenr;
1746-
const bool orig_ignore_extent_item_pos = ctx->ignore_extent_item_pos;
1747+
const bool orig_skip_inode_ref_list = ctx->skip_inode_ref_list;
17471748
bool roots_ulist_allocated = false;
17481749
struct ulist_iterator uiter;
17491750
int ret = 0;
@@ -1764,7 +1765,7 @@ static int btrfs_find_all_roots_safe(struct btrfs_backref_walk_ctx *ctx)
17641765
roots_ulist_allocated = true;
17651766
}
17661767

1767-
ctx->ignore_extent_item_pos = true;
1768+
ctx->skip_inode_ref_list = true;
17681769

17691770
ULIST_ITER_INIT(&uiter);
17701771
while (1) {
@@ -1789,7 +1790,7 @@ static int btrfs_find_all_roots_safe(struct btrfs_backref_walk_ctx *ctx)
17891790
ulist_free(ctx->refs);
17901791
ctx->refs = NULL;
17911792
ctx->bytenr = orig_bytenr;
1792-
ctx->ignore_extent_item_pos = orig_ignore_extent_item_pos;
1793+
ctx->skip_inode_ref_list = orig_skip_inode_ref_list;
17931794

17941795
return ret;
17951796
}
@@ -1912,7 +1913,7 @@ int btrfs_is_data_extent_shared(struct btrfs_inode *inode, u64 bytenr,
19121913
goto out_trans;
19131914
}
19141915

1915-
walk_ctx.ignore_extent_item_pos = true;
1916+
walk_ctx.skip_inode_ref_list = true;
19161917
walk_ctx.trans = trans;
19171918
walk_ctx.fs_info = fs_info;
19181919
walk_ctx.refs = &ctx->refs;

fs/btrfs/backref.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,12 @@ struct btrfs_backref_walk_ctx {
6060
* @extent_item_pos is ignored.
6161
*/
6262
bool ignore_extent_item_pos;
63+
/*
64+
* If true and bytenr corresponds to a data extent, then the inode list
65+
* (each member describing inode number, file offset and root) is not
66+
* added to each reference added to the @refs ulist.
67+
*/
68+
bool skip_inode_ref_list;
6369
/* A valid transaction handle or NULL. */
6470
struct btrfs_trans_handle *trans;
6571
/*

fs/btrfs/relocation.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3422,7 +3422,7 @@ int add_data_references(struct reloc_control *rc,
34223422
btrfs_release_path(path);
34233423

34243424
ctx.bytenr = extent_key->objectid;
3425-
ctx.ignore_extent_item_pos = true;
3425+
ctx.skip_inode_ref_list = true;
34263426
ctx.fs_info = rc->extent_root->fs_info;
34273427

34283428
ret = btrfs_find_all_leafs(&ctx);

0 commit comments

Comments
 (0)