[v2,1/4] btrfs: pass checksum type via BTRFS_IOC_FS_INFO ioctl
diff mbox series

Message ID 20200713122901.1773-2-johannes.thumshirn@wdc.com
State New
Headers show
Series
  • Two furhter additions for fsinfo ioctl
Related show

Commit Message

Johannes Thumshirn July 13, 2020, 12:28 p.m. UTC
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 and csum_size fields 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 and
csum_size field.

For compatibility reasons, only return the csum_type and csum_size if
the BTRFS_FS_INFO_FLAG_CSUM_INFO flag was passed to the kernel. Also
clear any unknown flags so we don't pass false positives to user-space
newer than the kernel.

To simplify further additions to the ioctl, also switch the padding to a
u8 array. Pahole was used to verify the result of this switch:

pahole -C btrfs_ioctl_fs_info_args fs/btrfs/btrfs.ko
struct btrfs_ioctl_fs_info_args {
        __u64                      max_id;               /*     0     8 */
        __u64                      num_devices;          /*     8     8 */
        __u8                       fsid[16];             /*    16    16 */
        __u32                      nodesize;             /*    32     4 */
        __u32                      sectorsize;           /*    36     4 */
        __u32                      clone_alignment;      /*    40     4 */
        __u16                      csum_type;            /*    44     2 */
        __u16                      csum_size;            /*    46     2 */
        __u64                      flags;                /*    48     8 */
        __u8                       reserved[968];        /*    56   968 */

        /* size: 1024, cachelines: 16, members: 10 */
};

Fixes: 3951e7f050ac ("btrfs: add xxhash64 to checksumming algorithms")
Fixes: 3831bf0094ab ("btrfs: add sha256 to checksumming algorithm")
CC: stable@vger.kernel.org # 5.5+
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 fs/btrfs/ioctl.c           | 16 +++++++++++++---
 include/uapi/linux/btrfs.h | 15 +++++++++++++--
 2 files changed, 26 insertions(+), 5 deletions(-)

Comments

David Sterba July 13, 2020, 2:27 p.m. UTC | #1
On Mon, Jul 13, 2020 at 09:28:58PM +0900, Johannes Thumshirn wrote:
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -3217,11 +3217,15 @@ static long btrfs_ioctl_fs_info(struct btrfs_fs_info *fs_info,
>  	struct btrfs_ioctl_fs_info_args *fi_args;
>  	struct btrfs_device *device;
>  	struct btrfs_fs_devices *fs_devices = fs_info->fs_devices;
> +	u32 flags_in;

u64 here too, I'll fix it.
Johannes Thumshirn July 13, 2020, 2:28 p.m. UTC | #2
On 13/07/2020 16:27, David Sterba wrote:
> On Mon, Jul 13, 2020 at 09:28:58PM +0900, Johannes Thumshirn wrote:
>> --- a/fs/btrfs/ioctl.c
>> +++ b/fs/btrfs/ioctl.c
>> @@ -3217,11 +3217,15 @@ static long btrfs_ioctl_fs_info(struct btrfs_fs_info *fs_info,
>>  	struct btrfs_ioctl_fs_info_args *fi_args;
>>  	struct btrfs_device *device;
>>  	struct btrfs_fs_devices *fs_devices = fs_info->fs_devices;
>> +	u32 flags_in;
> 
> u64 here too, I'll fix it.
> 

Uh, surprised why GCC didn't warn me about the truncation
David Sterba July 13, 2020, 2:50 p.m. UTC | #3
On Mon, Jul 13, 2020 at 02:28:36PM +0000, Johannes Thumshirn wrote:
> On 13/07/2020 16:27, David Sterba wrote:
> > On Mon, Jul 13, 2020 at 09:28:58PM +0900, Johannes Thumshirn wrote:
> >> --- a/fs/btrfs/ioctl.c
> >> +++ b/fs/btrfs/ioctl.c
> >> @@ -3217,11 +3217,15 @@ static long btrfs_ioctl_fs_info(struct btrfs_fs_info *fs_info,
> >>  	struct btrfs_ioctl_fs_info_args *fi_args;
> >>  	struct btrfs_device *device;
> >>  	struct btrfs_fs_devices *fs_devices = fs_info->fs_devices;
> >> +	u32 flags_in;
> > 
> > u64 here too, I'll fix it.
> 
> Uh, surprised why GCC didn't warn me about the truncation

The right warning for that is -Wconversion but it's off by default.
Integer type truncations are common becasue we know what we're doing
(except for the bugs).

fs/btrfs/ioctl.c: In function ‘btrfs_ioctl_fs_info’:
fs/btrfs/ioctl.c:3228:13: warning: conversion from ‘__u64’ {aka ‘long long unsigned int’} to ‘u32’ {aka ‘unsigned int’} may change value [-Wconversion]
 3228 |  flags_in = fi_args->flags;

Otherwise:

$ make ccflags-y=-Wconversion fs/btrfs/ioctl.o 2>&1 | grep warning: | wc -l
533

Most of them are from included headers.

Patch
diff mbox series

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index ab34179d7cbc..3a566cf71fc6 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -3217,11 +3217,15 @@  static long btrfs_ioctl_fs_info(struct btrfs_fs_info *fs_info,
 	struct btrfs_ioctl_fs_info_args *fi_args;
 	struct btrfs_device *device;
 	struct btrfs_fs_devices *fs_devices = fs_info->fs_devices;
+	u32 flags_in;
 	int ret = 0;
 
-	fi_args = kzalloc(sizeof(*fi_args), GFP_KERNEL);
-	if (!fi_args)
-		return -ENOMEM;
+	fi_args = memdup_user(arg, sizeof(*fi_args));
+	if (IS_ERR(fi_args))
+		return PTR_ERR(fi_args);
+
+	flags_in = fi_args->flags;
+	memset(fi_args, 0, sizeof(*fi_args));
 
 	rcu_read_lock();
 	fi_args->num_devices = fs_devices->num_devices;
@@ -3237,6 +3241,12 @@  static long btrfs_ioctl_fs_info(struct btrfs_fs_info *fs_info,
 	fi_args->sectorsize = fs_info->sectorsize;
 	fi_args->clone_alignment = fs_info->sectorsize;
 
+	if (flags_in & BTRFS_FS_INFO_FLAG_CSUM_INFO) {
+		fi_args->csum_type = btrfs_super_csum_type(fs_info->super_copy);
+		fi_args->csum_size = btrfs_super_csum_size(fs_info->super_copy);
+		fi_args->flags |= BTRFS_FS_INFO_FLAG_CSUM_INFO;
+	}
+
 	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..b3e0af77642f 100644
--- a/include/uapi/linux/btrfs.h
+++ b/include/uapi/linux/btrfs.h
@@ -243,6 +243,13 @@  struct btrfs_ioctl_dev_info_args {
 	__u8 path[BTRFS_DEVICE_PATH_NAME_MAX];	/* out */
 };
 
+/*
+ * Retrieve information about the filesystem
+ */
+
+/* Request information about checksum type and size */
+#define BTRFS_FS_INFO_FLAG_CSUM_INFO			(1 << 0)
+
 struct btrfs_ioctl_fs_info_args {
 	__u64 max_id;				/* out */
 	__u64 num_devices;			/* out */
@@ -250,10 +257,14 @@  struct btrfs_ioctl_fs_info_args {
 	__u32 nodesize;				/* out */
 	__u32 sectorsize;			/* out */
 	__u32 clone_alignment;			/* out */
-	__u32 reserved32;
-	__u64 reserved[122];			/* pad to 1k */
+	/* See BTRFS_FS_INFO_FLAG_* */
+	__u16 csum_type;			/* out */
+	__u16 csum_size;			/* out */
+	__u64 flags;				/* in/out */
+	__u8 reserved[968];			/* pad to 1k */
 };
 
+
 /*
  * feature flags
  *