[RFC PATCH] f2fs: expand f2fs_compr_option to allow ioctl setting compression level

[RFC PATCH] f2fs: expand f2fs_compr_option to allow ioctl setting compression level

[Sheng Yong]

This patch adds `level` in `struct f2fs_compr_option` to allow ioctl
setting compression level.

The first byte of original f2fs_compr_option indicates which algorithm
is used. While the new f2fs_compr_option splits the first byte into two
parts:
  * the MBS 4 bits indicate the version
  * the LBS 4 bits indicate the algorithm

The original f2fs_compr_option is renamed to f2fs_compr_option_base,
which is used to calculate ioctl command. For now, the version could
be 0 or 1.

When getting and setting compression option, the first byte should be
copied from userspace in advance to get the version. Then copy the
whole option according to version size.

The new f2fs_compr_option could be compatible with old userspace tool:
    Old tool does not set the MSB 4 bits, which keep all 0. F2FS
    detects option is version 0, and will not return or set level.

But if new tool is used on old F2FS:
    New tool sets the MSB 4 bits to 1, get_option could return V0
    values, but set_option will fail.

Signed-off-by: Sheng Yong <shengyong@oppo.com>
---
 fs/f2fs/file.c            | 41 ++++++++++++++++++++++++++++++++++-----
 include/uapi/linux/f2fs.h | 39 ++++++++++++++++++++++++++++++++++---
 2 files changed, 72 insertions(+), 8 deletions(-)

diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 14e9a20e68df3..909da18208d76 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -3904,10 +3904,27 @@ static int f2fs_ioc_get_compress_option(struct file *filp, unsigned long arg)
 {
 	struct inode *inode = file_inode(filp);
 	struct f2fs_comp_option option;
+	__u8 ver;
+	size_t copt_sz;
 
 	if (!f2fs_sb_has_compression(F2FS_I_SB(inode)))
 		return -EOPNOTSUPP;
 
+	if (copy_from_user(&option.value, (__u8 __user *)arg, 1))
+		return -EFAULT;
+
+	ver = COPTION_VERSION(option.value);
+	copt_sz = COPTION_SIZE(ver);
+	if (copt_sz == UINT_MAX) {
+		/*
+		 * In order to be compatible with old version option, whose
+		 * algorithm is not initialized, the V0 option is returned
+		 * instead of error.
+		 */
+		ver = F2FS_COPTION_V0;
+		copt_sz = COPTION_SIZE(ver);
+	}
+
 	inode_lock_shared(inode);
 
 	if (!f2fs_compressed_file(inode)) {
@@ -3915,13 +3932,14 @@ static int f2fs_ioc_get_compress_option(struct file *filp, unsigned long arg)
 		return -ENODATA;
 	}
 
-	option.algorithm = F2FS_I(inode)->i_compress_algorithm;
+	option.value = COPTION_VALUE(ver, F2FS_I(inode)->i_compress_algorithm);
 	option.log_cluster_size = F2FS_I(inode)->i_log_cluster_size;
+	option.level = F2FS_I(inode)->i_compress_level;
 
 	inode_unlock_shared(inode);
 
 	if (copy_to_user((struct f2fs_comp_option __user *)arg, &option,
-				sizeof(option)))
+				copt_sz))
 		return -EFAULT;
 
 	return 0;
@@ -3932,6 +3950,8 @@ static int f2fs_ioc_set_compress_option(struct file *filp, unsigned long arg)
 	struct inode *inode = file_inode(filp);
 	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
 	struct f2fs_comp_option option;
+	__u8 ver, alg;
+	size_t copt_sz;
 	int ret = 0;
 
 	if (!f2fs_sb_has_compression(sbi))
@@ -3940,14 +3960,23 @@ static int f2fs_ioc_set_compress_option(struct file *filp, unsigned long arg)
 	if (!(filp->f_mode & FMODE_WRITE))
 		return -EBADF;
 
+	if (copy_from_user(&option.value, (__u8 __user *)arg, 1))
+		return -EFAULT;
+
+	ver = COPTION_VERSION(option.value);
+	alg = COPTION_ALGORITHM(option.value);
+	copt_sz = COPTION_SIZE(ver);
+	if (copt_sz == UINT_MAX)
+		return -EFAULT;
+
 	if (copy_from_user(&option, (struct f2fs_comp_option __user *)arg,
-				sizeof(option)))
+				copt_sz))
 		return -EFAULT;
 
 	if (!f2fs_compressed_file(inode) ||
 			option.log_cluster_size < MIN_COMPRESS_LOG_SIZE ||
 			option.log_cluster_size > MAX_COMPRESS_LOG_SIZE ||
-			option.algorithm >= COMPRESS_MAX)
+			alg >= COMPRESS_MAX)
 		return -EINVAL;
 
 	file_start_write(filp);
@@ -3963,9 +3992,11 @@ static int f2fs_ioc_set_compress_option(struct file *filp, unsigned long arg)
 		goto out;
 	}
 
-	F2FS_I(inode)->i_compress_algorithm = option.algorithm;
+	F2FS_I(inode)->i_compress_algorithm = alg;
 	F2FS_I(inode)->i_log_cluster_size = option.log_cluster_size;
 	F2FS_I(inode)->i_cluster_size = BIT(option.log_cluster_size);
+	if (ver == F2FS_COPTION_V1)
+		F2FS_I(inode)->i_compress_level = option.level;
 	f2fs_mark_inode_dirty_sync(inode, true);
 
 	if (!f2fs_is_compress_backend_ready(inode))
diff --git a/include/uapi/linux/f2fs.h b/include/uapi/linux/f2fs.h
index 955d440be1046..940cf46174357 100644
--- a/include/uapi/linux/f2fs.h
+++ b/include/uapi/linux/f2fs.h
@@ -37,9 +37,9 @@
 #define F2FS_IOC_SEC_TRIM_FILE		_IOW(F2FS_IOCTL_MAGIC, 20,	\
 						struct f2fs_sectrim_range)
 #define F2FS_IOC_GET_COMPRESS_OPTION	_IOR(F2FS_IOCTL_MAGIC, 21,	\
-						struct f2fs_comp_option)
+						struct f2fs_comp_option_base)
 #define F2FS_IOC_SET_COMPRESS_OPTION	_IOW(F2FS_IOCTL_MAGIC, 22,	\
-						struct f2fs_comp_option)
+						struct f2fs_comp_option_base)
 #define F2FS_IOC_DECOMPRESS_FILE	_IO(F2FS_IOCTL_MAGIC, 23)
 #define F2FS_IOC_COMPRESS_FILE		_IO(F2FS_IOCTL_MAGIC, 24)
 #define F2FS_IOC_START_ATOMIC_REPLACE	_IO(F2FS_IOCTL_MAGIC, 25)
@@ -91,9 +91,42 @@ struct f2fs_sectrim_range {
 	__u64 flags;
 };
 
-struct f2fs_comp_option {
+#define F2FS_COPTION_V0	0
+#define F2FS_COPTION_V1	1
+
+#define COPTION_VER_SHIFT	4
+#define COPTION_VER_MASK	(~((1 << COPTION_VER_SHIFT) - 1))
+
+struct f2fs_comp_option_base {
 	__u8 algorithm;
 	__u8 log_cluster_size;
 };
 
+struct f2fs_comp_option {
+	union {
+		struct f2fs_comp_option_base base;
+		struct {
+			__u8 value; // MSB 4 bit is version, LSB 4 bit is algorithm
+			__u8 log_cluster_size;
+		};
+	};
+	__u8 level;
+};
+
+#define COPTION_VERSION(val) ((val) >> COPTION_VER_SHIFT)
+#define COPTION_ALGORITHM(val) ((val) & ((1 << COPTION_VER_SHIFT) - 1))
+#define COPTION_VALUE(ver, alg) (((__u8)(ver) << COPTION_VER_SHIFT) | (__u8)(alg))
+#define COPTION_SIZE(ver) ({					\
+	size_t sz = UINT_MAX;					\
+	switch (ver) {						\
+	case F2FS_COPTION_V0:					\
+		sz = offsetof(struct f2fs_comp_option, level);	\
+		break;						\
+	case F2FS_COPTION_V1:					\
+		sz = sizeof(struct f2fs_comp_option);		\
+		break;						\
+	}							\
+	sz;							\
+})
+
 #endif /* _UAPI_LINUX_F2FS_H */
-- 
2.25.1

Re: [RFC PATCH] f2fs: expand f2fs_compr_option to allow ioctl setting compression level

[Yangtao Li]

Hi Sheng Yong,

Your idea, I also put forward before.

And has been sent to version 2, but Chao and Jaegeuk have no comments yet.
Time to talk about the series?

https://lore.kernel.org/linux-f2fs-devel/20230112133503.16802-1-frank.li@vivo.com/

Thx,
Yangtao

Re: [RFC PATCH] f2fs: expand f2fs_compr_option to allow ioctl setting compression level

[Sheng Yong]

On 2023/3/31 0:15, Yangtao Li wrote:
> Hi Sheng Yong,
> 
> Your idea, I also put forward before.
> 
> And has been sent to version 2, but Chao and Jaegeuk have no comments yet.
> Time to talk about the series?
> 
> https://lore.kernel.org/linux-f2fs-devel/20230112133503.16802-1-frank.li@vivo.com/
> 
Hi, Yangtao

Thanks for giving me the information. The f2fs_comp_option_v2 could also help
to get/set more options for compression. And I suggest to add a version or size
at the beginning of the struct so that it is easy to expand the struct in the
future.

Thanks,
shengyong

> Thx,
> Yangtao