On 9.06.20 г. 13:57 ч., Filipe Manana wrote:
> On Tue, Jun 9, 2020 at 11:50 AM Nikolay Borisov <nborisov@xxxxxxxx> wrote:
>>
>>
>>
>> On 9.06.20 г. 13:19 ч., fdmanana@xxxxxxxxxx wrote:
>>> From: Filipe Manana <fdmanana@xxxxxxxx>
>>>
>>> When relocating a data block group we group extents from the block group
>>> into a cluster and when the cluster reaches a certain number of extents
>>> we do the relocation.
>>
>> We don't reserve more space because the cluster is guaranteed to contain
>> contiguous extents, namely in relocate_data_extent we have:
>>
>> if (cluster->nr > 0 && extent_key->objectid != cluster->end + 1)
>> {
>> ret = relocate_file_extent_cluster(inode, cluster);
>> if (ret)
>> return ret;
>> cluster->nr = 0;
>> }
>>
>> Cluster->end points to the end offset of the last added extent and the
>> check above ensures that the one which is currently added is also
>> contiguous. So relocate_file_extent_cluster is always called with
>> contiguous range of data extents, which might actually be less than
>> MAX_EXTENTS.
>>
>> As a matter of fact this is a rather hard requirement since
>> prealloc_file_extent_cluster assumes that since when it iterates the
>> extents in the cluster it does:
>>
>> if (nr + 1 < cluster->nr)
>>
>> end = cluster->boundary[nr + 1] - 1 - offset;
>>
>> else
>>
>> end = cluster->end - offset;
>>
>> If those extents weren't contiguous we'd be preallocating more space in
>> the data reloc inode.
>
> Yes, thanks.
> I was biased by the gap detection at prealloc_file_extent_cluster()
> and missed that, which in this case can't happen.
>
> So lets drop this.
>
Thinking a bit more about this the following check is redundant, since
extents are guaranteed to be contiguous:
if (cur_offset < start)
btrfs_free_reserved_data_space(inode, data_reserved,
cur_offset, start - cur_offset);
This was added as part of 18513091af948 but IMO it's bogus since
clustered data extent reloc landed much earlier, in
0257bb82d21bedff26541bcf12f1461c23f9ed61. A good cleanup would be to
simply remove this check.
