On 2019/8/7 下午4:44, Filipe Manana wrote: > On Wed, Aug 7, 2019 at 9:35 AM Anand Jain <anand.jain@xxxxxxxxxx> wrote: >> >> Commit 6ba9fc8e628b (btrfs: Ensure btrfs_trim_fs can trim the whole >> filesystem) makes sure we always trim starting from the first block group. >> However it also removed the range.start validity check which is set in the >> context of the user, where its range is from 0 to maximum of filesystem >> totalbytes and so we have to check its validity in the kernel. >> >> Also as in the fstrim(8) [1] the kernel layers may modify the trim range. >> >> [1] >> Further, the kernel block layer reserves the right to adjust the discard >> ranges to fit raid stripe geometry, non-trim capable devices in a LVM >> setup, etc. These reductions would not be reflected in fstrim_range.len >> (the --length option). >> >> This patch undos the deleted range::start validity check. >> >> Fixes: 6ba9fc8e628b (btrfs: Ensure btrfs_trim_fs can trim the whole filesystem) >> Signed-off-by: Anand Jain <anand.jain@xxxxxxxxxx> >> --- >> With this patch fstests generic/260 is successful now. >> >> fs/btrfs/ioctl.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c >> index b431f7877e88..9345fcdf80c7 100644 >> --- a/fs/btrfs/ioctl.c >> +++ b/fs/btrfs/ioctl.c >> @@ -521,6 +521,8 @@ static noinline int btrfs_ioctl_fitrim(struct file *file, void __user *arg) >> return -EOPNOTSUPP; >> if (copy_from_user(&range, arg, sizeof(range))) >> return -EFAULT; >> + if (range.start > btrfs_super_total_bytes(fs_info->super_copy)) >> + return -EINVAL; > > This makes it impossible to trim block groups that start at an offset > greater then the super_total_bytes values. Exactly. > In fact, in extreme cases > it's possible all block groups start at offsets larger then > super_total_bytes. > Have you considered that, or am I missing something? And, I have already mentioned exactly the same reason in that commit message. To address it once again, the bytenr is btrfs logical address space, has nothing to do with any device. Thus it can be [0, U64MAX]. Thanks, Qu > > The change log is also vague to me, doesn't explain why you are > re-adding that check. > > Thanks. > >> >> /* >> * NOTE: Don't truncate the range using super->total_bytes. Bytenr of >> -- >> 2.21.0 (Apple Git-120) >> > >
Attachment:
signature.asc
Description: OpenPGP digital signature
