Re: [PATCH 2/3] btrfs-progs: check/original: Add check and repair for invalid inode generation

Nikolay Borisov
2019-09-30T08:41:36Z

On 24.09.19 г. 11:11 ч., Qu Wenruo wrote:
> There are at least two bug reports of kernel tree-checker complaining
> about invalid inode generation.
>
> All offending inodes seem to be caused by old kernel around 2014, with
> inode generation overflow.
>
> So add such check and repair ability to lowmem mode check first.
>
> This involves:
> - Calculate the inode generation upper limit
> Unlike the original mode context, we don't have anyway to determine if

Perhaps you meant "lowmem mode context" since in lowmem you deduce
whether it's a log-tree inode or not.

> this inode belongs to log tree.
> So we use super_generation + 1 as upper limit, just like what we did
> in kernel tree checker.
>
> - Check if the inode generation is larger than the upper limit
>
> - Repair by resetting inode generation to current transaction
> generation
> The difference is, in original mode, we have a common trans handle for
> all repair and reset path for each repair.
>
> Reported-by: Charles Wright <charles.v.wright@gmail.com>
> Signed-off-by: Qu Wenruo <wqu@suse.com>

I tested that code with the image provided and it does work as expected
as well as it's fairly obvious what's happening. So:

Reviewed-by: Nikolay Borisov <nborisov@suse.com>
Tested-by: Nikolay Borisov <nborisov@suse.com>

> ---
> check/main.c | 50 ++++++++++++++++++++++++++++++++++++++++++-
> check/mode-original.h | 1 +
> 2 files changed, 50 insertions(+), 1 deletion(-)
>
> diff --git a/check/main.c b/check/main.c
> index c2d0f394..018707c8 100644
> --- a/check/main.c
> +++ b/check/main.c
> @@ -604,6 +604,8 @@ static void print_inode_error(struct btrfs_root *root, struct inode_record *rec)
> if (errors & I_ERR_INVALID_IMODE)
> fprintf(stderr, ", invalid inode mode bit 0%o",
> rec->imode & ~07777);
> + if (errors & I_ERR_INVALID_GEN)
> + fprintf(stderr, ", invalid inode generation");
> fprintf(stderr, "\n");
>
> /* Print the holes if needed */
> @@ -857,6 +859,7 @@ static int process_inode_item(struct extent_buffer *eb,
> {
> struct inode_record *rec;
> struct btrfs_inode_item *item;
> + u64 gen_uplimit;
> u64 flags;
>
> rec = active_node->current;
> @@ -879,6 +882,13 @@ static int process_inode_item(struct extent_buffer *eb,
> if (S_ISLNK(rec->imode) &&
> flags & (BTRFS_INODE_IMMUTABLE | BTRFS_INODE_APPEND))
> rec->errors |= I_ERR_ODD_INODE_FLAGS;
> + /*
> + * We don't have accurate root info to determine the correct
> + * inode generation uplimit, use super_generation + 1 anyway
> + */
> + gen_uplimit = btrfs_super_generation(global_info->super_copy) + 1;
> + if (btrfs_inode_generation(eb, item) > gen_uplimit)
> + rec->errors |= I_ERR_INVALID_GEN;
> maybe_free_inode_rec(&active_node->inode_cache, rec);
> return 0;
> }
> @@ -2774,6 +2784,41 @@ static int repair_imode_original(struct btrfs_trans_handle *trans,
> return ret;
> }
>
> +static int repair_inode_gen_original(struct btrfs_trans_handle *trans,
> + struct btrfs_root *root,
> + struct btrfs_path *path,
> + struct inode_record *rec)
> +{
> + struct btrfs_inode_item *ii;
> + struct btrfs_key key;
> + int ret;
> +
> + key.objectid = rec->ino;
> + key.offset = 0;
> + key.type = BTRFS_INODE_ITEM_KEY;
> +
> + ret = btrfs_search_slot(trans, root, &key, path, 0, 1);
> + if (ret > 0) {
> + ret = -ENOENT;
> + error("no inode item found for ino %llu", rec->ino);
> + return ret;
> + }
> + if (ret < 0) {
> + errno = -ret;
> + error("failed to search inode item for ino %llu: %m", rec->ino);
> + return ret;
> + }
> + ii = btrfs_item_ptr(path->nodes[0], path->slots[0],
> + struct btrfs_inode_item);
> + btrfs_set_inode_generation(path->nodes[0], ii, trans->transid);
> + btrfs_mark_buffer_dirty(path->nodes[0]);
> + btrfs_release_path(path);
> + printf("resetting inode generation to %llu for ino %llu\n",
> + trans->transid, rec->ino);
> + rec->errors &= ~I_ERR_INVALID_GEN;
> + return 0;
> +}
> +
> static int try_repair_inode(struct btrfs_root *root, struct inode_record *rec)
> {
> struct btrfs_trans_handle *trans;
> @@ -2794,7 +2839,8 @@ static int try_repair_inode(struct btrfs_root *root, struct inode_record *rec)
> I_ERR_FILE_NBYTES_WRONG |
> I_ERR_INLINE_RAM_BYTES_WRONG |
> I_ERR_MISMATCH_DIR_HASH |
> - I_ERR_UNALIGNED_EXTENT_REC)))
> + I_ERR_UNALIGNED_EXTENT_REC |
> + I_ERR_INVALID_GEN)))
> return rec->errors;
>
> /*
> @@ -2829,6 +2875,8 @@ static int try_repair_inode(struct btrfs_root *root, struct inode_record *rec)
> ret = repair_unaligned_extent_recs(trans, root, &path, rec);
> if (!ret && rec->errors & I_ERR_INVALID_IMODE)
> ret = repair_imode_original(trans, root, &path, rec);
> + if (!ret && rec->errors & I_ERR_INVALID_GEN)
> + ret = repair_inode_gen_original(trans, root, &path, rec);
> btrfs_commit_transaction(trans, root);
> btrfs_release_path(&path);
> return ret;
> diff --git a/check/mode-original.h b/check/mode-original.h
> index 78d6bb30..b075a95c 100644
> --- a/check/mode-original.h
> +++ b/check/mode-original.h
> @@ -185,6 +185,7 @@ struct unaligned_extent_rec_t {
> #define I_ERR_INLINE_RAM_BYTES_WRONG (1 << 17)
> #define I_ERR_MISMATCH_DIR_HASH (1 << 18)
> #define I_ERR_INVALID_IMODE (1 << 19)
> +#define I_ERR_INVALID_GEN (1 << 20)
>
> struct inode_record {
> struct list_head backrefs;
>