On 2018年05月18日 13:02, Nikolay Borisov wrote:
>
>
> On 18.05.2018 05:59, Liu Bo wrote:
>> As verify_level_key() is checked after verify_parent_transid(), i.e.
>>
>> if (verify_parent_transid())
>> ret = -EIO;
>> else if (verify_level_key())
>> ret = -EUCLEAN;
>>
>> if parent_transid is 0, verify_parent_transid() skips verifying
>> parent_transid and considers eb as valid, and if verify_level_key()
>> reports something wrong, we're not going to know if it's caused by
>> corrupted metadata or non-checkecd eb (e.g. stale eb).
>
> It's really unclear (from the changelog) how the stale eb ties with
> parent_transid. You should have explained the relationship between
> checking parenttransid and stale/non-stale ebs
It's in fact related to the fixing patch:
Btrfs: fix the corruption by reading stale btree blocks
But whatever, extra mention won't hurt.
>
>>
>> @parent_transid is able to tell whether the eb's generation has been
>> verified by the caller.
>
> It's really unclear why parent_transid is able to tell whether the
> generation is verified by the caller.
At least this looks pretty straightforward to me.
@parent_transid = 0 means no generation check, and @parent_transid != 0
means already checked.
So at least for this part I didn't get any confusion.
Thanks,
Qu
>
>>
>> Signed-off-by: Liu Bo <bo....@linux.alibaba.com>
>> ---
>> v2: - more explicit commit log.
>> - adjust the position shown in error msg.
>>
>> fs/btrfs/disk-io.c | 13 +++++++------
>> 1 file changed, 7 insertions(+), 6 deletions(-)
>>
>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
>> index 60caa68c3618..ad865176a3ca 100644
>> --- a/fs/btrfs/disk-io.c
>> +++ b/fs/btrfs/disk-io.c
>> @@ -416,7 +416,7 @@ static int btrfs_check_super_csum(struct btrfs_fs_info
>> *fs_info,
>>
>> static int verify_level_key(struct btrfs_fs_info *fs_info,
>> struct extent_buffer *eb, int level,
>> - struct btrfs_key *first_key)
>> + struct btrfs_key *first_key, u64 parent_transid)
>> {
>> int found_level;
>> struct btrfs_key found_key;
>> @@ -454,10 +454,11 @@ static int verify_level_key(struct btrfs_fs_info
>> *fs_info,
>> if (ret) {
>> WARN_ON(1);
>> btrfs_err(fs_info,
>> -"tree first key mismatch detected, bytenr=%llu key expected=(%llu, %u,
>> %llu) has=(%llu, %u, %llu)",
>> - eb->start, first_key->objectid, first_key->type,
>> - first_key->offset, found_key.objectid,
>> - found_key.type, found_key.offset);
>> +"tree first key mismatch detected, bytenr=%llu parent_transid=%llu key
>> expected=(%llu, %u, %llu) has=(%llu, %u, %llu)",
>> + eb->start, parent_transid, first_key->objectid,
>> + first_key->type, first_key->offset,
>> + found_key.objectid, found_key.type,
>> + found_key.offset);
>> }
>> #endif
>> return ret;
>> @@ -493,7 +494,7 @@ static int btree_read_extent_buffer_pages(struct
>> btrfs_fs_info *fs_info,
>> parent_transid, 0))
>> ret = -EIO;
>> else if (verify_level_key(fs_info, eb, level,
>> - first_key))
>> + first_key, parent_transid))
>> ret = -EUCLEAN;
>> else
>> break;
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
