Re: [PATCH 3/4] btrfs: Skip first megabyte on device when trimming

[Nikolay Borisov <nborisov@suse.com>]

On 5.06.19 г. 12:14 ч., Qu Wenruo wrote:
> 
> 
> On 2019/6/3 下午6:06, Nikolay Borisov wrote:
>> Currently the first megabyte on a device housing a btrfs filesystem is
>> exempt from allocation and trimming. Currently this is not a problem
>> since 'start' is set to 1m at the beginning of btrfs_trim_free_extents
>> and find_first_clear_extent_bit always returns a range that is >= start.
>> However, in a follow up patch find_first_clear_extent_bit will be
>> changed such that it will return a range containing 'start' and this
>> range may very well be 0...>=1M so 'start'.
>>
>> Future proof the sole user of find_first_clear_extent_bit by setting
>> 'start' after the function is called. No functional changes.
>>
>> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> 
> Doesn't that previous patch already address this by:
> 
> +	u64 start = SZ_1M, len = 0, end = 0;

No, because with the changes introduced in the next patch start can
actually be made to point to 0 for example. One of the self-test cases
covers this, e.g. :

find_first_clear_extent_bit(&tree, SZ_512K, &start, &end,
+				    CHUNK_TRIMMED | CHUNK_ALLOCATED);

> 
> Thanks,
> Qu
> 
> 
>> ---
>>  fs/btrfs/extent-tree.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
>> index d8c5febf7636..5a11e4988243 100644
>> --- a/fs/btrfs/extent-tree.c
>> +++ b/fs/btrfs/extent-tree.c
>> @@ -11183,6 +11183,10 @@ static int btrfs_trim_free_extents(struct btrfs_device *device, u64 *trimmed)
>>  		 * to the caller to trim the value to the size of the device.
>>  		 */
>>  		end = min(end, device->total_bytes - 1);
>> +
>> +		/* Ensure we skip first mb in case we have a bootloader there */
>> +		start = max_t(u64, start, SZ_1M);
>> +
>>  		len = end - start + 1;
>>
>>  		/* We didn't find any extents */
>>
>