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
