[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-btrfs
Subject:    Re: [PATCH 2/3] btrfs-progs: check/original: Add check and repair for invalid inode generation
From:       Qu Wenruo <quwenruo.btrfs () gmx ! com>
Date:       2019-09-30 9:00:10
Message-ID: c7cf2823-91b9-bb09-471c-d43a19a24f05 () gmx ! com
[Download RAW message or body]



On 2019/9/30 下午4:41, Nikolay Borisov wrote:
>
>
> 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.

Oh, right.

Would you mind to fix this in commit time, David?

>
>>   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>

Thanks for the review and test.

Thanks,
Qu
>
>> ---
>>  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;
>>
[prev in list] [next in list] [prev in thread] [next in thread]