| Message ID | c78571e0ea619aefd33e2c6d1a6ac274cb15581f.1682798736.git.josef@toxicpanda.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: various cleanups to make ctree.c sync easier | expand |
On 29.04.23 22:08, Josef Bacik wrote: > diff --git a/fs/btrfs/tree-checker.h b/fs/btrfs/tree-checker.h > index 3b8de6d36141..c0861ce1429b 100644 > --- a/fs/btrfs/tree-checker.h > +++ b/fs/btrfs/tree-checker.h > @@ -58,6 +58,7 @@ enum btrfs_tree_block_status { > * btrfs_tree_block_status return codes. > */ > enum btrfs_tree_block_status __btrfs_check_leaf(struct extent_buffer *leaf); > +enum btrfs_tree_block_status __btrfs_check_node(struct extent_buffer *node); Sorry for only noticing now, but shouldn't we do something like #ifndef KERNEL enum btrfs_tree_block_status __btrfs_check_leaf(struct extent_buffer *leaf); enum btrfs_tree_block_status __btrfs_check_node(struct extent_buffer *node); #endif and in fs/btrfs/tree-checker.c (or fs.h): #ifdef KERNEL #define EXPORT_FOR_PROGS static #else #define EXPORT_FOR_PROGS #endif Just like we did with EXPORT_FOR_TESTS?
On Tue, May 02, 2023 at 12:10:20PM +0000, Johannes Thumshirn wrote: > On 29.04.23 22:08, Josef Bacik wrote: > > diff --git a/fs/btrfs/tree-checker.h b/fs/btrfs/tree-checker.h > > index 3b8de6d36141..c0861ce1429b 100644 > > --- a/fs/btrfs/tree-checker.h > > +++ b/fs/btrfs/tree-checker.h > > @@ -58,6 +58,7 @@ enum btrfs_tree_block_status { > > * btrfs_tree_block_status return codes. > > */ > > enum btrfs_tree_block_status __btrfs_check_leaf(struct extent_buffer *leaf); > > +enum btrfs_tree_block_status __btrfs_check_node(struct extent_buffer *node); > > Sorry for only noticing now, but shouldn't we do something like > > #ifndef KERNEL > enum btrfs_tree_block_status __btrfs_check_leaf(struct extent_buffer *leaf); > enum btrfs_tree_block_status __btrfs_check_node(struct extent_buffer *node); > #endif > > and in fs/btrfs/tree-checker.c (or fs.h): > > #ifdef KERNEL > #define EXPORT_FOR_PROGS static > #else > #define EXPORT_FOR_PROGS > #endif > > Just like we did with EXPORT_FOR_TESTS? That's a good idea. Could we do it at the end of the series though? It's not the cleanest way but the patches are otherwise simple and we won't need to backport them so having the change split like that should not cause trouble. I can also edit the patches in place if we insist. With a separate patch I'd like to see how the conditional definitions for kernel and userspace should be done first.
diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c index 2c330e9d123a..eb9ba48d92aa 100644 --- a/fs/btrfs/tree-checker.c +++ b/fs/btrfs/tree-checker.c @@ -1846,7 +1846,7 @@ int btrfs_check_leaf(struct extent_buffer *leaf) } ALLOW_ERROR_INJECTION(btrfs_check_leaf, ERRNO); -int btrfs_check_node(struct extent_buffer *node) +enum btrfs_tree_block_status __btrfs_check_node(struct extent_buffer *node) { struct btrfs_fs_info *fs_info = node->fs_info; unsigned long nr = btrfs_header_nritems(node); @@ -1854,13 +1854,12 @@ int btrfs_check_node(struct extent_buffer *node) int slot; int level = btrfs_header_level(node); u64 bytenr; - int ret = 0; if (unlikely(level <= 0 || level >= BTRFS_MAX_LEVEL)) { generic_err(node, 0, "invalid level for node, have %d expect [1, %d]", level, BTRFS_MAX_LEVEL - 1); - return -EUCLEAN; + return BTRFS_TREE_BLOCK_INVALID_LEVEL; } if (unlikely(nr == 0 || nr > BTRFS_NODEPTRS_PER_BLOCK(fs_info))) { btrfs_crit(fs_info, @@ -1868,7 +1867,7 @@ int btrfs_check_node(struct extent_buffer *node) btrfs_header_owner(node), node->start, nr == 0 ? "small" : "large", nr, BTRFS_NODEPTRS_PER_BLOCK(fs_info)); - return -EUCLEAN; + return BTRFS_TREE_BLOCK_INVALID_NRITEMS; } for (slot = 0; slot < nr - 1; slot++) { @@ -1879,15 +1878,13 @@ int btrfs_check_node(struct extent_buffer *node) if (unlikely(!bytenr)) { generic_err(node, slot, "invalid NULL node pointer"); - ret = -EUCLEAN; - goto out; + return BTRFS_TREE_BLOCK_INVALID_BLOCKPTR; } if (unlikely(!IS_ALIGNED(bytenr, fs_info->sectorsize))) { generic_err(node, slot, "unaligned pointer, have %llu should be aligned to %u", bytenr, fs_info->sectorsize); - ret = -EUCLEAN; - goto out; + return BTRFS_TREE_BLOCK_INVALID_BLOCKPTR; } if (unlikely(btrfs_comp_cpu_keys(&key, &next_key) >= 0)) { @@ -1896,12 +1893,20 @@ int btrfs_check_node(struct extent_buffer *node) key.objectid, key.type, key.offset, next_key.objectid, next_key.type, next_key.offset); - ret = -EUCLEAN; - goto out; + return BTRFS_TREE_BLOCK_BAD_KEY_ORDER; } } -out: - return ret; + return BTRFS_TREE_BLOCK_CLEAN; +} + +int btrfs_check_node(struct extent_buffer *node) +{ + enum btrfs_tree_block_status ret; + + ret = __btrfs_check_node(node); + if (unlikely(ret != BTRFS_TREE_BLOCK_CLEAN)) + return -EUCLEAN; + return 0; } ALLOW_ERROR_INJECTION(btrfs_check_node, ERRNO); diff --git a/fs/btrfs/tree-checker.h b/fs/btrfs/tree-checker.h index 3b8de6d36141..c0861ce1429b 100644 --- a/fs/btrfs/tree-checker.h +++ b/fs/btrfs/tree-checker.h @@ -58,6 +58,7 @@ enum btrfs_tree_block_status { * btrfs_tree_block_status return codes. */ enum btrfs_tree_block_status __btrfs_check_leaf(struct extent_buffer *leaf); +enum btrfs_tree_block_status __btrfs_check_node(struct extent_buffer *node); int btrfs_check_leaf(struct extent_buffer *leaf); int btrfs_check_node(struct extent_buffer *node);
This helper returns a btrfs_tree_block_status for the various errors, and then btrfs_check_node() will return -EUCLEAN if it gets anything other than BTRFS_TREE_BLOCK_CLEAN which will be used by the kernel. In the future btrfs-progs will use this helper instead. Signed-off-by: Josef Bacik <josef@toxicpanda.com> --- fs/btrfs/tree-checker.c | 29 +++++++++++++++++------------ fs/btrfs/tree-checker.h | 1 + 2 files changed, 18 insertions(+), 12 deletions(-)