On 2019/5/31 下午1:56, Qu Wenruo wrote: > > > 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 <wqu@xxxxxxxx> >> >> 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 almost 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. Also add fsdevel list into the discussion. > > 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.jain@xxxxxxxxxx> >> >> 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)) { >>> >> >
Attachment:
signature.asc
Description: OpenPGP digital signature
