* [PATCH v2 1/2] f2fs: add F2FS_IOC_GET_COMPRESS_OPTION ioctl
@ 2020-10-22 3:58 Daeho Jeong
2020-10-22 3:58 ` [PATCH v2 2/2] f2fs: add F2FS_IOC_SET_COMPRESS_OPTION ioctl Daeho Jeong
2020-10-22 4:22 ` [f2fs-dev] [PATCH v2 1/2] f2fs: add F2FS_IOC_GET_COMPRESS_OPTION ioctl Eric Biggers
0 siblings, 2 replies; 10+ messages in thread
From: Daeho Jeong @ 2020-10-22 3:58 UTC (permalink / raw)
To: linux-kernel, linux-f2fs-devel, kernel-team; +Cc: Daeho Jeong
From: Daeho Jeong <daehojeong@google.com>
Added a new F2FS_IOC_GET_COMPRESS_OPTION ioctl to get file compression
option of a file.
struct f2fs_comp_option {
u8 algorithm; => compression algorithm
=> 0:lzo, 1:lz4, 2:zstd, 3:lzorle
u8 log_cluster_size; => log scale cluster size
=> 2 ~ 8
};
struct f2fs_comp_option option;
ioctl(fd, F2FS_IOC_GET_COMPRESS_OPTION, &option);
Signed-off-by: Daeho Jeong <daehojeong@google.com>
---
fs/f2fs/f2fs.h | 7 +++++++
fs/f2fs/file.c | 30 ++++++++++++++++++++++++++++++
2 files changed, 37 insertions(+)
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 53fe2853579c..a33c90cf979b 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -433,6 +433,8 @@ static inline bool __has_cursum_space(struct f2fs_journal *journal,
_IOR(F2FS_IOCTL_MAGIC, 19, __u64)
#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)
/*
* should be same as XFS_IOC_GOINGDOWN.
@@ -481,6 +483,11 @@ struct f2fs_sectrim_range {
u64 flags;
};
+struct f2fs_comp_option {
+ u8 algorithm;
+ u8 log_cluster_size;
+};
+
/* for inline stuff */
#define DEF_INLINE_RESERVED_SIZE 1
static inline int get_extra_isize(struct inode *inode);
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index ef5a844de53f..7e64259f6f5e 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -3936,6 +3936,33 @@ static int f2fs_sec_trim_file(struct file *filp, unsigned long arg)
return ret;
}
+static int f2fs_ioc_get_compress_option(struct file *filp, unsigned long arg)
+{
+ struct inode *inode = file_inode(filp);
+ struct f2fs_comp_option option;
+
+ if (!f2fs_sb_has_compression(F2FS_I_SB(inode)))
+ return -EOPNOTSUPP;
+
+ inode_lock(inode);
+
+ if (!f2fs_compressed_file(inode)) {
+ inode_unlock(inode);
+ return -EINVAL;
+ }
+
+ option.algorithm = F2FS_I(inode)->i_compress_algorithm;
+ option.log_cluster_size = F2FS_I(inode)->i_log_cluster_size;
+
+ inode_unlock(inode);
+
+ if (copy_to_user((struct f2fs_comp_option __user *)arg, &option,
+ sizeof(option)))
+ return -EFAULT;
+
+ return 0;
+}
+
long f2fs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
{
if (unlikely(f2fs_cp_error(F2FS_I_SB(file_inode(filp)))))
@@ -4024,6 +4051,8 @@ long f2fs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
return f2fs_reserve_compress_blocks(filp, arg);
case F2FS_IOC_SEC_TRIM_FILE:
return f2fs_sec_trim_file(filp, arg);
+ case F2FS_IOC_GET_COMPRESS_OPTION:
+ return f2fs_ioc_get_compress_option(filp, arg);
default:
return -ENOTTY;
}
@@ -4194,6 +4223,7 @@ long f2fs_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
case F2FS_IOC_RELEASE_COMPRESS_BLOCKS:
case F2FS_IOC_RESERVE_COMPRESS_BLOCKS:
case F2FS_IOC_SEC_TRIM_FILE:
+ case F2FS_IOC_GET_COMPRESS_OPTION:
break;
default:
return -ENOIOCTLCMD;
--
2.29.0.rc1.297.gfa9743e501-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v2 2/2] f2fs: add F2FS_IOC_SET_COMPRESS_OPTION ioctl 2020-10-22 3:58 [PATCH v2 1/2] f2fs: add F2FS_IOC_GET_COMPRESS_OPTION ioctl Daeho Jeong @ 2020-10-22 3:58 ` Daeho Jeong 2020-10-22 4:26 ` [f2fs-dev] " Eric Biggers ` (3 more replies) 2020-10-22 4:22 ` [f2fs-dev] [PATCH v2 1/2] f2fs: add F2FS_IOC_GET_COMPRESS_OPTION ioctl Eric Biggers 1 sibling, 4 replies; 10+ messages in thread From: Daeho Jeong @ 2020-10-22 3:58 UTC (permalink / raw) To: linux-kernel, linux-f2fs-devel, kernel-team; +Cc: Daeho Jeong From: Daeho Jeong <daehojeong@google.com> Added a new F2FS_IOC_SET_COMPRESS_OPTION ioctl to change file compression option of a file. struct f2fs_comp_option { u8 algorithm; => compression algorithm => 0:lzo, 1:lz4, 2:zstd, 3:lzorle u8 log_cluster_size; => log scale cluster size => 2 ~ 8 }; struct f2fs_comp_option option; option.algorithm = 1; option.log_cluster_size = 7; ioctl(fd, F2FS_IOC_SET_COMPRESS_OPTION, &option); Signed-off-by: Daeho Jeong <daehojeong@google.com> --- fs/f2fs/compress.c | 5 +++++ fs/f2fs/f2fs.h | 7 +++++++ fs/f2fs/file.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+) diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c index 7895186cc765..3b58a41223f8 100644 --- a/fs/f2fs/compress.c +++ b/fs/f2fs/compress.c @@ -514,6 +514,11 @@ bool f2fs_is_compress_backend_ready(struct inode *inode) return f2fs_cops[F2FS_I(inode)->i_compress_algorithm]; } +bool f2fs_is_compress_algorithm_ready(unsigned char algorithm) +{ + return algorithm >= COMPRESS_MAX ? false : f2fs_cops[algorithm]; +} + static mempool_t *compress_page_pool; static int num_compress_pages = 512; module_param(num_compress_pages, uint, 0444); diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h index a33c90cf979b..cc38afde6c04 100644 --- a/fs/f2fs/f2fs.h +++ b/fs/f2fs/f2fs.h @@ -435,6 +435,8 @@ static inline bool __has_cursum_space(struct f2fs_journal *journal, struct f2fs_sectrim_range) #define F2FS_IOC_GET_COMPRESS_OPTION _IOR(F2FS_IOCTL_MAGIC, 21, \ struct f2fs_comp_option) +#define F2FS_IOC_SET_COMPRESS_OPTION _IOW(F2FS_IOCTL_MAGIC, 22, \ + struct f2fs_comp_option) /* * should be same as XFS_IOC_GOINGDOWN. @@ -3915,6 +3917,7 @@ bool f2fs_compress_write_end(struct inode *inode, void *fsdata, int f2fs_truncate_partial_cluster(struct inode *inode, u64 from, bool lock); void f2fs_compress_write_end_io(struct bio *bio, struct page *page); bool f2fs_is_compress_backend_ready(struct inode *inode); +bool f2fs_is_compress_algorithm_ready(unsigned char algorithm); int f2fs_init_compress_mempool(void); void f2fs_destroy_compress_mempool(void); void f2fs_decompress_pages(struct bio *bio, struct page *page, bool verity); @@ -3945,6 +3948,10 @@ static inline bool f2fs_is_compress_backend_ready(struct inode *inode) /* not support compression */ return false; } +static inline bool f2fs_is_compress_algorithm_ready(unsigned char algorithm) +{ + return false; +} static inline struct page *f2fs_compress_control_page(struct page *page) { WARN_ON_ONCE(1); diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c index 7e64259f6f5e..b18ef22e2d9d 100644 --- a/fs/f2fs/file.c +++ b/fs/f2fs/file.c @@ -3963,6 +3963,51 @@ static int f2fs_ioc_get_compress_option(struct file *filp, unsigned long arg) return 0; } +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; + int ret; + + if (!f2fs_sb_has_compression(sbi)) + return -EOPNOTSUPP; + + if (!f2fs_compressed_file(inode)) + return -EINVAL; + + if (!(filp->f_mode & FMODE_WRITE)) + return -EBADF; + + if (copy_from_user(&option, (struct f2fs_comp_option __user *)arg, + sizeof(option))) + return -EFAULT; + + if (option.log_cluster_size < MIN_COMPRESS_LOG_SIZE || + option.log_cluster_size > MAX_COMPRESS_LOG_SIZE || + !f2fs_is_compress_algorithm_ready(option.algorithm)) + return -EINVAL; + + file_start_write(filp); + inode_lock(inode); + + if (f2fs_is_mmap_file(inode) || + get_dirty_pages(inode) || inode->i_size) { + ret = -EINVAL; + goto out; + } + + F2FS_I(inode)->i_compress_algorithm = option.algorithm; + F2FS_I(inode)->i_log_cluster_size = option.log_cluster_size; + F2FS_I(inode)->i_cluster_size = 1 << option.log_cluster_size; + f2fs_mark_inode_dirty_sync(inode, true); +out: + inode_unlock(inode); + file_end_write(filp); + + return ret; +} + long f2fs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) { if (unlikely(f2fs_cp_error(F2FS_I_SB(file_inode(filp))))) @@ -4053,6 +4098,8 @@ long f2fs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) return f2fs_sec_trim_file(filp, arg); case F2FS_IOC_GET_COMPRESS_OPTION: return f2fs_ioc_get_compress_option(filp, arg); + case F2FS_IOC_SET_COMPRESS_OPTION: + return f2fs_ioc_set_compress_option(filp, arg); default: return -ENOTTY; } @@ -4224,6 +4271,7 @@ long f2fs_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg) case F2FS_IOC_RESERVE_COMPRESS_BLOCKS: case F2FS_IOC_SEC_TRIM_FILE: case F2FS_IOC_GET_COMPRESS_OPTION: + case F2FS_IOC_SET_COMPRESS_OPTION: break; default: return -ENOIOCTLCMD; -- 2.29.0.rc1.297.gfa9743e501-goog ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [f2fs-dev] [PATCH v2 2/2] f2fs: add F2FS_IOC_SET_COMPRESS_OPTION ioctl 2020-10-22 3:58 ` [PATCH v2 2/2] f2fs: add F2FS_IOC_SET_COMPRESS_OPTION ioctl Daeho Jeong @ 2020-10-22 4:26 ` Eric Biggers 2020-10-22 4:36 ` Daeho Jeong 2020-10-22 6:50 ` kernel test robot ` (2 subsequent siblings) 3 siblings, 1 reply; 10+ messages in thread From: Eric Biggers @ 2020-10-22 4:26 UTC (permalink / raw) To: Daeho Jeong; +Cc: linux-kernel, linux-f2fs-devel, kernel-team, Daeho Jeong On Thu, Oct 22, 2020 at 12:58:48PM +0900, Daeho Jeong wrote: > diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c > index 7895186cc765..3b58a41223f8 100644 > --- a/fs/f2fs/compress.c > +++ b/fs/f2fs/compress.c > @@ -514,6 +514,11 @@ bool f2fs_is_compress_backend_ready(struct inode *inode) > return f2fs_cops[F2FS_I(inode)->i_compress_algorithm]; > } > > +bool f2fs_is_compress_algorithm_ready(unsigned char algorithm) > +{ > + return algorithm >= COMPRESS_MAX ? false : f2fs_cops[algorithm]; > +} The use of ?: here is a bit strange. How about: return algorithm < COMPRESS_MAX && f2fs_cops[algorithm] != NULL; > + if (option.log_cluster_size < MIN_COMPRESS_LOG_SIZE || > + option.log_cluster_size > MAX_COMPRESS_LOG_SIZE || > + !f2fs_is_compress_algorithm_ready(option.algorithm)) > + return -EINVAL; Likewise, EINVAL tends to be over-used, which makes it ambiguous. Maybe use ENOPKG for the case where algorithm < COMPRESS_MAX but the algorithm wasn't compiled into the kernel? That would be similar to how opening an encrypted file fails with ENOPKG when the encryption algorithm isn't available. > + if (f2fs_is_mmap_file(inode) || > + get_dirty_pages(inode) || inode->i_size) { > + ret = -EINVAL; > + goto out; > + } How about EBUSY for f2fs_is_mmap_file(inode) || get_dirty_pages(inode), and EFBIG for inode->i_size != 0? - Eric ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [f2fs-dev] [PATCH v2 2/2] f2fs: add F2FS_IOC_SET_COMPRESS_OPTION ioctl 2020-10-22 4:26 ` [f2fs-dev] " Eric Biggers @ 2020-10-22 4:36 ` Daeho Jeong 0 siblings, 0 replies; 10+ messages in thread From: Daeho Jeong @ 2020-10-22 4:36 UTC (permalink / raw) To: Eric Biggers; +Cc: linux-kernel, linux-f2fs-devel, kernel-team, Daeho Jeong > The use of ?: here is a bit strange. How about: > > return algorithm < COMPRESS_MAX && f2fs_cops[algorithm] != NULL; > Ack > Likewise, EINVAL tends to be over-used, which makes it ambiguous. Maybe use > ENOPKG for the case where algorithm < COMPRESS_MAX but the algorithm wasn't > compiled into the kernel? That would be similar to how opening an encrypted > file fails with ENOPKG when the encryption algorithm isn't available. Ack > How about EBUSY for f2fs_is_mmap_file(inode) || get_dirty_pages(inode), > and EFBIG for inode->i_size != 0? Ack Thanks~! 2020년 10월 22일 (목) 오후 1:26, Eric Biggers <ebiggers@kernel.org>님이 작성: > > On Thu, Oct 22, 2020 at 12:58:48PM +0900, Daeho Jeong wrote: > > diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c > > index 7895186cc765..3b58a41223f8 100644 > > --- a/fs/f2fs/compress.c > > +++ b/fs/f2fs/compress.c > > @@ -514,6 +514,11 @@ bool f2fs_is_compress_backend_ready(struct inode *inode) > > return f2fs_cops[F2FS_I(inode)->i_compress_algorithm]; > > } > > > > +bool f2fs_is_compress_algorithm_ready(unsigned char algorithm) > > +{ > > + return algorithm >= COMPRESS_MAX ? false : f2fs_cops[algorithm]; > > +} > > The use of ?: here is a bit strange. How about: > > return algorithm < COMPRESS_MAX && f2fs_cops[algorithm] != NULL; > > > + if (option.log_cluster_size < MIN_COMPRESS_LOG_SIZE || > > + option.log_cluster_size > MAX_COMPRESS_LOG_SIZE || > > + !f2fs_is_compress_algorithm_ready(option.algorithm)) > > + return -EINVAL; > > Likewise, EINVAL tends to be over-used, which makes it ambiguous. Maybe use > ENOPKG for the case where algorithm < COMPRESS_MAX but the algorithm wasn't > compiled into the kernel? That would be similar to how opening an encrypted > file fails with ENOPKG when the encryption algorithm isn't available. > > > + if (f2fs_is_mmap_file(inode) || > > + get_dirty_pages(inode) || inode->i_size) { > > + ret = -EINVAL; > > + goto out; > > + } > > How about EBUSY for f2fs_is_mmap_file(inode) || get_dirty_pages(inode), > and EFBIG for inode->i_size != 0? > > - Eric ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] f2fs: add F2FS_IOC_SET_COMPRESS_OPTION ioctl 2020-10-22 3:58 ` [PATCH v2 2/2] f2fs: add F2FS_IOC_SET_COMPRESS_OPTION ioctl Daeho Jeong 2020-10-22 4:26 ` [f2fs-dev] " Eric Biggers @ 2020-10-22 6:50 ` kernel test robot 2020-10-22 10:14 ` kernel test robot 2020-10-23 12:00 ` Dan Carpenter 3 siblings, 0 replies; 10+ messages in thread From: kernel test robot @ 2020-10-22 6:50 UTC (permalink / raw) To: Daeho Jeong, linux-kernel, linux-f2fs-devel, kernel-team Cc: kbuild-all, clang-built-linux, Daeho Jeong [-- Attachment #1: Type: text/plain, Size: 3924 bytes --] Hi Daeho, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on f2fs/dev-test] [also build test WARNING on linus/master v5.9 next-20201022] [cannot apply to linux/master] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Daeho-Jeong/f2fs-add-F2FS_IOC_GET_COMPRESS_OPTION-ioctl/20201022-115947 base: https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git dev-test config: x86_64-randconfig-a005-20201022 (attached as .config) compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project ee6abef5323d59b983129bf3514ef6775d1d6cd5) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install x86_64 cross compiling tool for clang build # apt-get install binutils-x86-64-linux-gnu # https://github.com/0day-ci/linux/commit/d869d11ac39edb97c13765c59163d12f56dccd6f git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Daeho-Jeong/f2fs-add-F2FS_IOC_GET_COMPRESS_OPTION-ioctl/20201022-115947 git checkout d869d11ac39edb97c13765c59163d12f56dccd6f # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All warnings (new ones prefixed by >>): >> fs/f2fs/file.c:3997:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized] if (f2fs_is_mmap_file(inode) || ^~~~~~~~~~~~~~~~~~~~~~~~~~~ fs/f2fs/file.c:4011:9: note: uninitialized use occurs here return ret; ^~~ fs/f2fs/file.c:3997:2: note: remove the 'if' if its condition is always true if (f2fs_is_mmap_file(inode) || ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ fs/f2fs/file.c:3974:9: note: initialize the variable 'ret' to silence this warning int ret; ^ = 0 1 warning generated. vim +3997 fs/f2fs/file.c 3968 3969 static int f2fs_ioc_set_compress_option(struct file *filp, unsigned long arg) 3970 { 3971 struct inode *inode = file_inode(filp); 3972 struct f2fs_sb_info *sbi = F2FS_I_SB(inode); 3973 struct f2fs_comp_option option; 3974 int ret; 3975 3976 if (!f2fs_sb_has_compression(sbi)) 3977 return -EOPNOTSUPP; 3978 3979 if (!f2fs_compressed_file(inode)) 3980 return -EINVAL; 3981 3982 if (!(filp->f_mode & FMODE_WRITE)) 3983 return -EBADF; 3984 3985 if (copy_from_user(&option, (struct f2fs_comp_option __user *)arg, 3986 sizeof(option))) 3987 return -EFAULT; 3988 3989 if (option.log_cluster_size < MIN_COMPRESS_LOG_SIZE || 3990 option.log_cluster_size > MAX_COMPRESS_LOG_SIZE || 3991 !f2fs_is_compress_algorithm_ready(option.algorithm)) 3992 return -EINVAL; 3993 3994 file_start_write(filp); 3995 inode_lock(inode); 3996 > 3997 if (f2fs_is_mmap_file(inode) || 3998 get_dirty_pages(inode) || inode->i_size) { 3999 ret = -EINVAL; 4000 goto out; 4001 } 4002 4003 F2FS_I(inode)->i_compress_algorithm = option.algorithm; 4004 F2FS_I(inode)->i_log_cluster_size = option.log_cluster_size; 4005 F2FS_I(inode)->i_cluster_size = 1 << option.log_cluster_size; 4006 f2fs_mark_inode_dirty_sync(inode, true); 4007 out: 4008 inode_unlock(inode); 4009 file_end_write(filp); 4010 4011 return ret; 4012 } 4013 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 34832 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] f2fs: add F2FS_IOC_SET_COMPRESS_OPTION ioctl 2020-10-22 3:58 ` [PATCH v2 2/2] f2fs: add F2FS_IOC_SET_COMPRESS_OPTION ioctl Daeho Jeong 2020-10-22 4:26 ` [f2fs-dev] " Eric Biggers 2020-10-22 6:50 ` kernel test robot @ 2020-10-22 10:14 ` kernel test robot 2020-10-23 12:00 ` Dan Carpenter 3 siblings, 0 replies; 10+ messages in thread From: kernel test robot @ 2020-10-22 10:14 UTC (permalink / raw) To: Daeho Jeong, linux-kernel, linux-f2fs-devel, kernel-team Cc: kbuild-all, Daeho Jeong [-- Attachment #1: Type: text/plain, Size: 1819 bytes --] Hi Daeho, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on f2fs/dev-test] [also build test WARNING on linus/master v5.9 next-20201022] [cannot apply to linux/master] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Daeho-Jeong/f2fs-add-F2FS_IOC_GET_COMPRESS_OPTION-ioctl/20201022-115947 base: https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git dev-test config: x86_64-randconfig-s022-20201022 (attached as .config) compiler: gcc-9 (Debian 9.3.0-15) 9.3.0 reproduce: # apt-get install sparse # sparse version: v0.6.3-17-g2d3af347-dirty # https://github.com/0day-ci/linux/commit/d869d11ac39edb97c13765c59163d12f56dccd6f git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Daeho-Jeong/f2fs-add-F2FS_IOC_GET_COMPRESS_OPTION-ioctl/20201022-115947 git checkout d869d11ac39edb97c13765c59163d12f56dccd6f # save the attached .config to linux build tree make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=x86_64 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> "sparse warnings: (new ones prefixed by >>)" >> fs/f2fs/compress.c:543:44: sparse: sparse: Using plain integer as NULL pointer vim +543 fs/f2fs/compress.c 540 541 bool f2fs_is_compress_algorithm_ready(unsigned char algorithm) 542 { > 543 return algorithm >= COMPRESS_MAX ? false : f2fs_cops[algorithm]; 544 } 545 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 31833 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] f2fs: add F2FS_IOC_SET_COMPRESS_OPTION ioctl 2020-10-22 3:58 ` [PATCH v2 2/2] f2fs: add F2FS_IOC_SET_COMPRESS_OPTION ioctl Daeho Jeong ` (2 preceding siblings ...) 2020-10-22 10:14 ` kernel test robot @ 2020-10-23 12:00 ` Dan Carpenter 2020-10-23 12:03 ` Daeho Jeong 3 siblings, 1 reply; 10+ messages in thread From: Dan Carpenter @ 2020-10-23 12:00 UTC (permalink / raw) To: kbuild, Daeho Jeong, linux-kernel, linux-f2fs-devel, kernel-team Cc: lkp, kbuild-all, Daeho Jeong [-- Attachment #1: Type: text/plain, Size: 4342 bytes --] Hi Daeho, url: https://github.com/0day-ci/linux/commits/Daeho-Jeong/f2fs-add-F2FS_IOC_GET_COMPRESS_OPTION-ioctl/20201022-115947 base: https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git dev-test config: x86_64-randconfig-m001-20201022 (attached as .config) compiler: gcc-9 (Debian 9.3.0-15) 9.3.0 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> Reported-by: Dan Carpenter <dan.carpenter@oracle.com> New smatch warnings: fs/f2fs/file.c:4011 f2fs_ioc_set_compress_option() error: uninitialized symbol 'ret'. Old smatch warnings: fs/f2fs/f2fs.h:2127 dec_valid_block_count() warn: should 'count << 3' be a 64 bit type? fs/f2fs/file.c:2525 f2fs_ioc_gc_range() warn: inconsistent returns 'sbi->gc_lock'. fs/f2fs/file.c:2941 f2fs_ioc_flush_device() warn: potential spectre issue 'sbi->devs' [w] (local cap) fs/f2fs/file.c:2966 f2fs_ioc_flush_device() warn: inconsistent returns 'sbi->gc_lock'. fs/f2fs/file.c:3305 f2fs_precache_extents() error: uninitialized symbol 'err'. vim +/ret +4011 fs/f2fs/file.c d869d11ac39edb Daeho Jeong 2020-10-22 3969 static int f2fs_ioc_set_compress_option(struct file *filp, unsigned long arg) d869d11ac39edb Daeho Jeong 2020-10-22 3970 { d869d11ac39edb Daeho Jeong 2020-10-22 3971 struct inode *inode = file_inode(filp); d869d11ac39edb Daeho Jeong 2020-10-22 3972 struct f2fs_sb_info *sbi = F2FS_I_SB(inode); d869d11ac39edb Daeho Jeong 2020-10-22 3973 struct f2fs_comp_option option; d869d11ac39edb Daeho Jeong 2020-10-22 3974 int ret; d869d11ac39edb Daeho Jeong 2020-10-22 3975 d869d11ac39edb Daeho Jeong 2020-10-22 3976 if (!f2fs_sb_has_compression(sbi)) d869d11ac39edb Daeho Jeong 2020-10-22 3977 return -EOPNOTSUPP; d869d11ac39edb Daeho Jeong 2020-10-22 3978 d869d11ac39edb Daeho Jeong 2020-10-22 3979 if (!f2fs_compressed_file(inode)) d869d11ac39edb Daeho Jeong 2020-10-22 3980 return -EINVAL; d869d11ac39edb Daeho Jeong 2020-10-22 3981 d869d11ac39edb Daeho Jeong 2020-10-22 3982 if (!(filp->f_mode & FMODE_WRITE)) d869d11ac39edb Daeho Jeong 2020-10-22 3983 return -EBADF; d869d11ac39edb Daeho Jeong 2020-10-22 3984 d869d11ac39edb Daeho Jeong 2020-10-22 3985 if (copy_from_user(&option, (struct f2fs_comp_option __user *)arg, d869d11ac39edb Daeho Jeong 2020-10-22 3986 sizeof(option))) d869d11ac39edb Daeho Jeong 2020-10-22 3987 return -EFAULT; d869d11ac39edb Daeho Jeong 2020-10-22 3988 d869d11ac39edb Daeho Jeong 2020-10-22 3989 if (option.log_cluster_size < MIN_COMPRESS_LOG_SIZE || d869d11ac39edb Daeho Jeong 2020-10-22 3990 option.log_cluster_size > MAX_COMPRESS_LOG_SIZE || d869d11ac39edb Daeho Jeong 2020-10-22 3991 !f2fs_is_compress_algorithm_ready(option.algorithm)) d869d11ac39edb Daeho Jeong 2020-10-22 3992 return -EINVAL; d869d11ac39edb Daeho Jeong 2020-10-22 3993 d869d11ac39edb Daeho Jeong 2020-10-22 3994 file_start_write(filp); d869d11ac39edb Daeho Jeong 2020-10-22 3995 inode_lock(inode); d869d11ac39edb Daeho Jeong 2020-10-22 3996 d869d11ac39edb Daeho Jeong 2020-10-22 3997 if (f2fs_is_mmap_file(inode) || d869d11ac39edb Daeho Jeong 2020-10-22 3998 get_dirty_pages(inode) || inode->i_size) { d869d11ac39edb Daeho Jeong 2020-10-22 3999 ret = -EINVAL; d869d11ac39edb Daeho Jeong 2020-10-22 4000 goto out; d869d11ac39edb Daeho Jeong 2020-10-22 4001 } d869d11ac39edb Daeho Jeong 2020-10-22 4002 d869d11ac39edb Daeho Jeong 2020-10-22 4003 F2FS_I(inode)->i_compress_algorithm = option.algorithm; d869d11ac39edb Daeho Jeong 2020-10-22 4004 F2FS_I(inode)->i_log_cluster_size = option.log_cluster_size; d869d11ac39edb Daeho Jeong 2020-10-22 4005 F2FS_I(inode)->i_cluster_size = 1 << option.log_cluster_size; d869d11ac39edb Daeho Jeong 2020-10-22 4006 f2fs_mark_inode_dirty_sync(inode, true); "ret" is uninitialized on the success path. d869d11ac39edb Daeho Jeong 2020-10-22 4007 out: d869d11ac39edb Daeho Jeong 2020-10-22 4008 inode_unlock(inode); d869d11ac39edb Daeho Jeong 2020-10-22 4009 file_end_write(filp); d869d11ac39edb Daeho Jeong 2020-10-22 4010 d869d11ac39edb Daeho Jeong 2020-10-22 @4011 return ret; d869d11ac39edb Daeho Jeong 2020-10-22 4012 } --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 36000 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] f2fs: add F2FS_IOC_SET_COMPRESS_OPTION ioctl 2020-10-23 12:00 ` Dan Carpenter @ 2020-10-23 12:03 ` Daeho Jeong 0 siblings, 0 replies; 10+ messages in thread From: Daeho Jeong @ 2020-10-23 12:03 UTC (permalink / raw) To: Dan Carpenter Cc: kbuild, linux-kernel, linux-f2fs-devel, kernel-team, lkp, kbuild-all, Daeho Jeong Yep, sure~! 2020년 10월 23일 (금) 오후 9:00, Dan Carpenter <dan.carpenter@oracle.com>님이 작성: > > Hi Daeho, > > url: https://github.com/0day-ci/linux/commits/Daeho-Jeong/f2fs-add-F2FS_IOC_GET_COMPRESS_OPTION-ioctl/20201022-115947 > base: https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git dev-test > config: x86_64-randconfig-m001-20201022 (attached as .config) > compiler: gcc-9 (Debian 9.3.0-15) 9.3.0 > > If you fix the issue, kindly add following tag as appropriate > Reported-by: kernel test robot <lkp@intel.com> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > > New smatch warnings: > fs/f2fs/file.c:4011 f2fs_ioc_set_compress_option() error: uninitialized symbol 'ret'. > > Old smatch warnings: > fs/f2fs/f2fs.h:2127 dec_valid_block_count() warn: should 'count << 3' be a 64 bit type? > fs/f2fs/file.c:2525 f2fs_ioc_gc_range() warn: inconsistent returns 'sbi->gc_lock'. > fs/f2fs/file.c:2941 f2fs_ioc_flush_device() warn: potential spectre issue 'sbi->devs' [w] (local cap) > fs/f2fs/file.c:2966 f2fs_ioc_flush_device() warn: inconsistent returns 'sbi->gc_lock'. > fs/f2fs/file.c:3305 f2fs_precache_extents() error: uninitialized symbol 'err'. > > vim +/ret +4011 fs/f2fs/file.c > > d869d11ac39edb Daeho Jeong 2020-10-22 3969 static int f2fs_ioc_set_compress_option(struct file *filp, unsigned long arg) > d869d11ac39edb Daeho Jeong 2020-10-22 3970 { > d869d11ac39edb Daeho Jeong 2020-10-22 3971 struct inode *inode = file_inode(filp); > d869d11ac39edb Daeho Jeong 2020-10-22 3972 struct f2fs_sb_info *sbi = F2FS_I_SB(inode); > d869d11ac39edb Daeho Jeong 2020-10-22 3973 struct f2fs_comp_option option; > d869d11ac39edb Daeho Jeong 2020-10-22 3974 int ret; > d869d11ac39edb Daeho Jeong 2020-10-22 3975 > d869d11ac39edb Daeho Jeong 2020-10-22 3976 if (!f2fs_sb_has_compression(sbi)) > d869d11ac39edb Daeho Jeong 2020-10-22 3977 return -EOPNOTSUPP; > d869d11ac39edb Daeho Jeong 2020-10-22 3978 > d869d11ac39edb Daeho Jeong 2020-10-22 3979 if (!f2fs_compressed_file(inode)) > d869d11ac39edb Daeho Jeong 2020-10-22 3980 return -EINVAL; > d869d11ac39edb Daeho Jeong 2020-10-22 3981 > d869d11ac39edb Daeho Jeong 2020-10-22 3982 if (!(filp->f_mode & FMODE_WRITE)) > d869d11ac39edb Daeho Jeong 2020-10-22 3983 return -EBADF; > d869d11ac39edb Daeho Jeong 2020-10-22 3984 > d869d11ac39edb Daeho Jeong 2020-10-22 3985 if (copy_from_user(&option, (struct f2fs_comp_option __user *)arg, > d869d11ac39edb Daeho Jeong 2020-10-22 3986 sizeof(option))) > d869d11ac39edb Daeho Jeong 2020-10-22 3987 return -EFAULT; > d869d11ac39edb Daeho Jeong 2020-10-22 3988 > d869d11ac39edb Daeho Jeong 2020-10-22 3989 if (option.log_cluster_size < MIN_COMPRESS_LOG_SIZE || > d869d11ac39edb Daeho Jeong 2020-10-22 3990 option.log_cluster_size > MAX_COMPRESS_LOG_SIZE || > d869d11ac39edb Daeho Jeong 2020-10-22 3991 !f2fs_is_compress_algorithm_ready(option.algorithm)) > d869d11ac39edb Daeho Jeong 2020-10-22 3992 return -EINVAL; > d869d11ac39edb Daeho Jeong 2020-10-22 3993 > d869d11ac39edb Daeho Jeong 2020-10-22 3994 file_start_write(filp); > d869d11ac39edb Daeho Jeong 2020-10-22 3995 inode_lock(inode); > d869d11ac39edb Daeho Jeong 2020-10-22 3996 > d869d11ac39edb Daeho Jeong 2020-10-22 3997 if (f2fs_is_mmap_file(inode) || > d869d11ac39edb Daeho Jeong 2020-10-22 3998 get_dirty_pages(inode) || inode->i_size) { > d869d11ac39edb Daeho Jeong 2020-10-22 3999 ret = -EINVAL; > d869d11ac39edb Daeho Jeong 2020-10-22 4000 goto out; > d869d11ac39edb Daeho Jeong 2020-10-22 4001 } > d869d11ac39edb Daeho Jeong 2020-10-22 4002 > d869d11ac39edb Daeho Jeong 2020-10-22 4003 F2FS_I(inode)->i_compress_algorithm = option.algorithm; > d869d11ac39edb Daeho Jeong 2020-10-22 4004 F2FS_I(inode)->i_log_cluster_size = option.log_cluster_size; > d869d11ac39edb Daeho Jeong 2020-10-22 4005 F2FS_I(inode)->i_cluster_size = 1 << option.log_cluster_size; > d869d11ac39edb Daeho Jeong 2020-10-22 4006 f2fs_mark_inode_dirty_sync(inode, true); > > > "ret" is uninitialized on the success path. > > d869d11ac39edb Daeho Jeong 2020-10-22 4007 out: > d869d11ac39edb Daeho Jeong 2020-10-22 4008 inode_unlock(inode); > d869d11ac39edb Daeho Jeong 2020-10-22 4009 file_end_write(filp); > d869d11ac39edb Daeho Jeong 2020-10-22 4010 > d869d11ac39edb Daeho Jeong 2020-10-22 @4011 return ret; > d869d11ac39edb Daeho Jeong 2020-10-22 4012 } > > --- > 0-DAY CI Kernel Test Service, Intel Corporation > https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [f2fs-dev] [PATCH v2 1/2] f2fs: add F2FS_IOC_GET_COMPRESS_OPTION ioctl 2020-10-22 3:58 [PATCH v2 1/2] f2fs: add F2FS_IOC_GET_COMPRESS_OPTION ioctl Daeho Jeong 2020-10-22 3:58 ` [PATCH v2 2/2] f2fs: add F2FS_IOC_SET_COMPRESS_OPTION ioctl Daeho Jeong @ 2020-10-22 4:22 ` Eric Biggers 2020-10-22 4:30 ` Daeho Jeong 1 sibling, 1 reply; 10+ messages in thread From: Eric Biggers @ 2020-10-22 4:22 UTC (permalink / raw) To: Daeho Jeong; +Cc: linux-kernel, linux-f2fs-devel, kernel-team, Daeho Jeong On Thu, Oct 22, 2020 at 12:58:47PM +0900, Daeho Jeong wrote: > + if (!f2fs_compressed_file(inode)) { > + inode_unlock(inode); > + return -EINVAL; > + } How about using ENODATA here? EINVAL tends to be used for lots of different reasons, and it's not always clear what it means. Note that FS_IOC_GET_ENCRYPTION_POLICY fails with ENODATA when called on an unencrypted file, which is a similar case. - Eric ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [f2fs-dev] [PATCH v2 1/2] f2fs: add F2FS_IOC_GET_COMPRESS_OPTION ioctl 2020-10-22 4:22 ` [f2fs-dev] [PATCH v2 1/2] f2fs: add F2FS_IOC_GET_COMPRESS_OPTION ioctl Eric Biggers @ 2020-10-22 4:30 ` Daeho Jeong 0 siblings, 0 replies; 10+ messages in thread From: Daeho Jeong @ 2020-10-22 4:30 UTC (permalink / raw) To: Eric Biggers; +Cc: linux-kernel, linux-f2fs-devel, kernel-team, Daeho Jeong Yep, it sounds more clear~ 2020년 10월 22일 (목) 오후 1:22, Eric Biggers <ebiggers@kernel.org>님이 작성: > > On Thu, Oct 22, 2020 at 12:58:47PM +0900, Daeho Jeong wrote: > > + if (!f2fs_compressed_file(inode)) { > > + inode_unlock(inode); > > + return -EINVAL; > > + } > > How about using ENODATA here? EINVAL tends to be used for lots of different > reasons, and it's not always clear what it means. > > Note that FS_IOC_GET_ENCRYPTION_POLICY fails with ENODATA when called on an > unencrypted file, which is a similar case. > > - Eric ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-10-23 12:03 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-10-22 3:58 [PATCH v2 1/2] f2fs: add F2FS_IOC_GET_COMPRESS_OPTION ioctl Daeho Jeong 2020-10-22 3:58 ` [PATCH v2 2/2] f2fs: add F2FS_IOC_SET_COMPRESS_OPTION ioctl Daeho Jeong 2020-10-22 4:26 ` [f2fs-dev] " Eric Biggers 2020-10-22 4:36 ` Daeho Jeong 2020-10-22 6:50 ` kernel test robot 2020-10-22 10:14 ` kernel test robot 2020-10-23 12:00 ` Dan Carpenter 2020-10-23 12:03 ` Daeho Jeong 2020-10-22 4:22 ` [f2fs-dev] [PATCH v2 1/2] f2fs: add F2FS_IOC_GET_COMPRESS_OPTION ioctl Eric Biggers 2020-10-22 4:30 ` Daeho Jeong
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox