On 24.06.20 г. 14:03 ч., Qu Wenruo wrote:
>
>
> On 2020/6/24 下午6:21, Johannes Thumshirn wrote:
>> With the recent addition of filesystem checksum types other than CRC32c,
>> it is not anymore hard-coded which checksum type a btrfs filesystem uses.
>>
>> Up to now there is no good way to read the filesystem checksum, apart from
>> reading the filesystem UUID and then query sysfs for the checksum type.
>>
>> Add a new csum_type field to the BTRFS_IOC_FS_INFO ioctl command which
>> usually is used to query filesystem features. Also add a flags member
>> indicating that the kernel responded with a set csum_type field.
>>
>> Fixes: 3951e7f050ac ("btrfs: add xxhash64 to checksumming algorithms")
>> Fixes: 3831bf0094ab ("btrfs: add sha256 to checksumming algorithm")
>
> I don't think the fixes tags are needed.
> You're just adding a new interface for IOC_FS_INFO to grab the algorithm.
True, but ideally this should have gone with the initial submission, so
it would make sense to have this interface for every kernel that
supports multiple checksums. With this in mind I think the fixes tags
are indeed very useful.
>
> Overall looks good.>
>> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@xxxxxxx>
>> ---
>> fs/btrfs/ioctl.c | 3 +++
>> include/uapi/linux/btrfs.h | 13 ++++++++++++-
>> 2 files changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
>> index b3e4c632d80c..16062720f5f3 100644
>> --- a/fs/btrfs/ioctl.c
>> +++ b/fs/btrfs/ioctl.c
>> @@ -3217,6 +3217,9 @@ static long btrfs_ioctl_fs_info(struct btrfs_fs_info *fs_info,
>> fi_args->nodesize = fs_info->nodesize;
>> fi_args->sectorsize = fs_info->sectorsize;
>> fi_args->clone_alignment = fs_info->sectorsize;
>> + fi_args->csum_type =
>> + le16_to_cpu(btrfs_super_csum_type(fs_info->super_copy));
>> + fi_args->flags |= BTRFS_FS_INFO_FLAG_CSUM_TYPE;
>>
>> if (copy_to_user(arg, fi_args, sizeof(*fi_args)))
>> ret = -EFAULT;
>> diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
>> index e6b6cb0f8bc6..161d9100c2a6 100644
>> --- a/include/uapi/linux/btrfs.h
>> +++ b/include/uapi/linux/btrfs.h
>> @@ -250,10 +250,21 @@ struct btrfs_ioctl_fs_info_args {
>> __u32 nodesize; /* out */
>> __u32 sectorsize; /* out */
>> __u32 clone_alignment; /* out */
>> + __u32 flags; /* out */
>
> The flags looks a little too generic.
> What about extra_members or things like that?
>
> This flag really indicates what extra info the ioctl args can provide,
> so a better name would be easier to understand.
>
> Thanks,
> Qu
>
>> + __u16 csum_type;
>> + __u16 reserved16;
>> __u32 reserved32;
>> - __u64 reserved[122]; /* pad to 1k */
>> + __u64 reserved[121]; /* pad to 1k */
>> };
>>
>> +/*
>> + * fs_info ioctl flags
>> + *
>> + * Used by:
>> + * struct btrfs_ioctl_fs_info_args
>> + */
>> +#define BTRFS_FS_INFO_FLAG_CSUM_TYPE (1 << 0)
>> +
>> /*
>> * feature flags
>> *
>>
>
