| Message ID | 3487ee70ac2e8fd2c82027c892e91f12a4a47324.1713550368.git.josef@toxicpanda.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: snapshot delete cleanups | expand |
在 2024/4/20 03:46, Josef Bacik 写道: > Currently we're only doing this in read_tree_block(), however > btrfs_check_eb_owner() properly deals with ->owner_root being set to 0, > and in fact we're duplicating this check in read_block_for_search(). > Push this check up into btrfs_read_extent_buffer() and fixup > read_block_for_search() to just return the result from > btrfs_read_extent_buffer() and drop the duplicate check. Since end_bbio_meta_read() is already calling btrfs_validate_extent_buffer() with bbio->parent_check copied from the callers, can we just remove the btrfs_check_eb_owner() calls directly from all the higher layer callers? Even the check in btrfs_read_extent_buffer() seems unnecessary now. Thanks, Qu > > Signed-off-by: Josef Bacik <josef@toxicpanda.com> > --- > fs/btrfs/ctree.c | 7 +------ > fs/btrfs/disk-io.c | 6 ++---- > 2 files changed, 3 insertions(+), 10 deletions(-) > > diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c > index 1a49b9232990..48aa14046343 100644 > --- a/fs/btrfs/ctree.c > +++ b/fs/btrfs/ctree.c > @@ -1551,12 +1551,7 @@ read_block_for_search(struct btrfs_root *root, struct btrfs_path *p, > if (ret) { > free_extent_buffer(tmp); > btrfs_release_path(p); > - return -EIO; > - } > - if (btrfs_check_eb_owner(tmp, btrfs_root_id(root))) { > - free_extent_buffer(tmp); > - btrfs_release_path(p); > - return -EUCLEAN; > + return ret; > } > > if (unlock_up) > diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c > index c2dc88f909b0..64523dc1060d 100644 > --- a/fs/btrfs/disk-io.c > +++ b/fs/btrfs/disk-io.c > @@ -251,6 +251,8 @@ int btrfs_read_extent_buffer(struct extent_buffer *eb, > if (failed && !ret && failed_mirror) > btrfs_repair_eb_io_failure(eb, failed_mirror); > > + if (!ret) > + ret = btrfs_check_eb_owner(eb, check->owner_root); > return ret; > } > > @@ -635,10 +637,6 @@ struct extent_buffer *read_tree_block(struct btrfs_fs_info *fs_info, u64 bytenr, > free_extent_buffer_stale(buf); > return ERR_PTR(ret); > } > - if (btrfs_check_eb_owner(buf, check->owner_root)) { > - free_extent_buffer_stale(buf); > - return ERR_PTR(-EUCLEAN); > - } > return buf; > > }
diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c index 1a49b9232990..48aa14046343 100644 --- a/fs/btrfs/ctree.c +++ b/fs/btrfs/ctree.c @@ -1551,12 +1551,7 @@ read_block_for_search(struct btrfs_root *root, struct btrfs_path *p, if (ret) { free_extent_buffer(tmp); btrfs_release_path(p); - return -EIO; - } - if (btrfs_check_eb_owner(tmp, btrfs_root_id(root))) { - free_extent_buffer(tmp); - btrfs_release_path(p); - return -EUCLEAN; + return ret; } if (unlock_up) diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c index c2dc88f909b0..64523dc1060d 100644 --- a/fs/btrfs/disk-io.c +++ b/fs/btrfs/disk-io.c @@ -251,6 +251,8 @@ int btrfs_read_extent_buffer(struct extent_buffer *eb, if (failed && !ret && failed_mirror) btrfs_repair_eb_io_failure(eb, failed_mirror); + if (!ret) + ret = btrfs_check_eb_owner(eb, check->owner_root); return ret; } @@ -635,10 +637,6 @@ struct extent_buffer *read_tree_block(struct btrfs_fs_info *fs_info, u64 bytenr, free_extent_buffer_stale(buf); return ERR_PTR(ret); } - if (btrfs_check_eb_owner(buf, check->owner_root)) { - free_extent_buffer_stale(buf); - return ERR_PTR(-EUCLEAN); - } return buf; }
Currently we're only doing this in read_tree_block(), however btrfs_check_eb_owner() properly deals with ->owner_root being set to 0, and in fact we're duplicating this check in read_block_for_search(). Push this check up into btrfs_read_extent_buffer() and fixup read_block_for_search() to just return the result from btrfs_read_extent_buffer() and drop the duplicate check. Signed-off-by: Josef Bacik <josef@toxicpanda.com> --- fs/btrfs/ctree.c | 7 +------ fs/btrfs/disk-io.c | 6 ++---- 2 files changed, 3 insertions(+), 10 deletions(-)