Chao, got it on f2fs_is_compress_algorithm_valid(). I also agree on Eric's opinion on that error value.
2020년 10월 30일 (금) 오전 12:54, Eric Biggers <ebigg...@kernel.org>님이 작성: > > On Thu, Oct 29, 2020 at 03:29:17PM +0800, Chao Yu wrote: > > > +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 = 0; > > > + > > > + if (!f2fs_sb_has_compression(sbi)) > > > + return -EOPNOTSUPP; > > > + > > > + if (!(filp->f_mode & FMODE_WRITE)) > > > + return -EBADF; > > > + > > > + if (copy_from_user(&option, (struct f2fs_comp_option __user *)arg, > > > + sizeof(option))) > > > + 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) > > > + return -EINVAL; > > > + > > > + file_start_write(filp); > > > + inode_lock(inode); > > > + > > > + if (f2fs_is_mmap_file(inode) || get_dirty_pages(inode)) { > > > + ret = -EBUSY; > > > + goto out; > > > + } > > > + > > > + if (inode->i_size != 0) { > > > + ret = -EFBIG; > > > + goto out; > > > + } > > > > Hmm... > > > > Shouldn't it be: > > > > if (algorithm >= COMPRESS_MAX) { > > ret = -ENOPKG; > > goto out; > > } > > > > if (!f2fs_cops[algorithm]) > > f2fs_warn(...); > > Note that my intent with recommending ENOPKG was for it to be returned in the > !f2fs_cops[algorithm] case, similar to how opening an encrypted file when the > encryption algorithm is recognized but not supported by the kernel returns > ENOPKG. For a truly unrecognized algorithm (algorithm >= COMPRESS_MAX), > EINVAL > would probably be more appropriate. So if !f2fs_cops[algorithm] is now > allowed, > then ENOPKG should no longer be among the error codes this ioctl returns. > > - Eric