On 2020/1/3 下午7:37, ethanwu wrote:
> Qu Wenruo 於 2020-01-03 18:15 寫到:
>> On 2020/1/3 下午5:44, ethanwu wrote:
[snip]
>> What if the backref offset already underflows?
>> Like this:
>> item 10 key (13631488 EXTENT_ITEM 1048576) itemoff 15860 itemsize 111
>> refs 3 gen 6 flags DATA
>> extent data backref root FS_TREE objectid 259 offset
>> 18446744073709547520 count 1 <<<
>> extent data backref root FS_TREE objectid 257 offset 0 count 1
>> extent data backref root FS_TREE objectid 258 offset 4096 count 1
>>
>>
>> Since backref offset is not file offset, but file_extent_item::offset -
>> file_offset, it can be a super large number for reflinked extents.
>>
>>
>> Current kernel handles this by a very ugly but working hack: resetting
>> key_for_search.offset to 0 in add_prelim_ref() if it detects such case.
>>
>> Then this would screw up your check, causing unexpected early exit.
>
> Thanks for the reminder.
> I think in this case the check won't fail. Even when we revert the
> working hack
> in the future, it still works unless we use u64 to do the calculation.
>
> (u64) 18446744073709547520 = (s64) -4096
>
> Suppose this very large offset is equal to X
> The next line is the original file view.
> [ ]
> ^ ^ ^ ...... ^
> 0 (u64)X + num_bytes EOF X
> [----oooooooooooo] Original range to check. - part is
> ^ the very large offset where no file extents
> X in terms of s64 exist, so actually the range [0,X+num_bytes)
> [oooooooooooooooo] range to check after hack X=>0
> ^
> 0 + num_bytes
>
> With my patch, applying this hack will only make my check condition looser.
> Causing more range to be checked (represented by o) compared to no hack.
Ah, you're right.
Since file_extent_item::offset can never be larger than extent size, so
we backref offset can only be in the range of [file_pos - 0, file_pos -
extent_size).
If we got a minus backref offset, it means file_pos -
file_extent_item::offset < 0, which means file_pos <
file_extent_item::offset.
And since file_extent_item::offset < extent_size, file_pos < extent_isze.
Thus even with current hack, the check still works, as we search from
file_pos 0, ends at file_pos extent size, which covers the file_extent_item.
Great explanation and patch.
Reviewed-by: Qu Wenruo <wqu@xxxxxxxx>
Thanks,
Qu
>
> The only way I think this check would fail would be:
> File at offset 2^64 - 4096 uses offset 0 of a 1MB data extent,
> key_for_search->offset + num_bytes = 2^64 - 4096 + 1048576 = 1044480
> Therefore, when iterating through the leafs, we'll break early at
> offset 1044480, leave the EXTENT_DATA key @2^64 - 4096 behind.
> But AFAIK, file of that size is not allowed in btrfs.
>
> Thanks,
> ethanwu
>>
>> I guess we have to find a new method to solve the problem then.
>>
>> Thanks,
>> Qu
>>
[...]
>>> + if (key_for_search->type == BTRFS_EXTENT_DATA_KEY &&
>>> + key.offset >= key_for_search->offset + num_bytes)
>>> + break;
>>> if (disk_byte == wanted_disk_byte) {
Attachment:
signature.asc
Description: OpenPGP digital signature
