On 2019/5/31 下午1:35, Anand Jain wrote: > On 5/28/19 4:21 PM, Qu Wenruo wrote: >> Normally the range->len is set to default value (U64_MAX), but when it's >> not default value, we should check if the range overflows. >> >> And if overflows, return -EINVAL before doing anything. >> >> Signed-off-by: Qu Wenruo <w...@suse.com> > > fstests patch > https://patchwork.kernel.org/patch/10964105/ > makes the sub-test like [1] in generic/260 skipped > > [1] > ----- > fssize=$($DF_PROG -k | grep "$SCRATCH_MNT" | grep "$SCRATCH_DEV" | awk > '{print $3}') > beyond_eofs=$((fssize*2048)) > fstrim -o $beyond_eofs $SCRATCH_MNT <-- should_fail > -----
As I mentioned in the commit message and offline, the idea of *end of
filesystem* is not clear enough.
For regular fs, they have every byte mapped directly to its block device
(except external journal).
So its end of filesystem is easy to determine.
But we can still argue, how to trim the external journal device? Or
should the external journal device contribute to the end of the fs?
Now for btrfs, it's a dm-linear space, then dm-raid/dm-linear for each
chunk. Thus we can argue either the end of btrfs is U64MAX (from
dm-linear view), or the end of last block group (from mapped chunk view).
Further more, how to define end of a filesystem when the fs spans across
several devices?
I'd say this is a good timing for us to make the fstrim behavior more clear.
Thanks,
Qu
>
> Originally [1] reported expected EINVAL until the patch
> 6ba9fc8e628b btrfs: Ensure btrfs_trim_fs can trim the whole filesystem
>
> Not sure how will some of the production machines will find this as,
> not compatible with previous versions? Nevertheless in practical terms
> things are fine.
>
> Reviewed-by: Anand Jain <anand.j...@oracle.com>
>
> Thanks, Anand
>
>> ---
>> fs/btrfs/extent-tree.c | 14 +++++++++++---
>> 1 file changed, 11 insertions(+), 3 deletions(-)
>>
>> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
>> index f79e477a378e..62bfba6d3c07 100644
>> --- a/fs/btrfs/extent-tree.c
>> +++ b/fs/btrfs/extent-tree.c
>> @@ -11245,6 +11245,7 @@ int btrfs_trim_fs(struct btrfs_fs_info
>> *fs_info, struct fstrim_range *range)
>> struct btrfs_device *device;
>> struct list_head *devices;
>> u64 group_trimmed;
>> + u64 range_end = U64_MAX;
>> u64 start;
>> u64 end;
>> u64 trimmed = 0;
>> @@ -11254,16 +11255,23 @@ int btrfs_trim_fs(struct btrfs_fs_info
>> *fs_info, struct fstrim_range *range)
>> int dev_ret = 0;
>> int ret = 0;
>> + /*
>> + * Check range overflow if range->len is set.
>> + * The default range->len is U64_MAX.
>> + */
>> + if (range->len != U64_MAX && check_add_overflow(range->start,
>> + range->len, &range_end))
>> + return -EINVAL;
>> +
>> cache = btrfs_lookup_first_block_group(fs_info, range->start);
>> for (; cache; cache = next_block_group(cache)) {
>> - if (cache->key.objectid >= (range->start + range->len)) {
>> + if (cache->key.objectid >= range_end) {
>> btrfs_put_block_group(cache);
>> break;
>> }
>> start = max(range->start, cache->key.objectid);
>> - end = min(range->start + range->len,
>> - cache->key.objectid + cache->key.offset);
>> + end = min(range_end, cache->key.objectid + cache->key.offset);
>> if (end - start >= range->minlen) {
>> if (!block_group_cache_done(cache)) {
>>
>
signature.asc
Description: OpenPGP digital signature
