linux-kernel - Re: [f2fs-dev] [PATCH] f2fs: compress: allow lz4 to compress data partially

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200508102306.GA18849@hsiangkao-HP-ZHAN-66-Pro-G1>
Date:   Fri, 8 May 2020 18:23:14 +0800
From:   Gao Xiang <hsiangkao@....com>
To:     Chao Yu <yuchao0@...wei.com>
Cc:     jaegeuk@...nel.org, linux-kernel@...r.kernel.org,
        linux-f2fs-devel@...ts.sourceforge.net, chao@...nel.org
Subject: Re: [f2fs-dev] [PATCH] f2fs: compress: allow lz4 to compress data
 partially

Hi Chao,

On Fri, May 08, 2020 at 05:47:09PM +0800, Chao Yu wrote:
> For lz4 worst compress case, caller should allocate buffer with size
> of LZ4_compressBound(inputsize) for target compressed data storing.
> 
> However lz4 supports partial data compression, so we can get rid of
> output buffer size limitation now, then we can avoid 2 * 4KB size
> intermediate buffer allocation when log_cluster_size is 2, and avoid
> unnecessary compressing work of compressor if we can not save at
> least 4KB space.
> 
> Suggested-by: Daeho Jeong <daehojeong@...gle.com>
> Signed-off-by: Chao Yu <yuchao0@...wei.com>
> ---
>  fs/f2fs/compress.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> index 5e4947250262..23825f559bcf 100644
> --- a/fs/f2fs/compress.c
> +++ b/fs/f2fs/compress.c
> @@ -228,7 +228,12 @@ static int lz4_init_compress_ctx(struct compress_ctx *cc)
>  	if (!cc->private)
>  		return -ENOMEM;
>  
> -	cc->clen = LZ4_compressBound(PAGE_SIZE << cc->log_cluster_size);
> +	/*
> +	 * we do not change cc->clen to LZ4_compressBound(inputsize) to
> +	 * adapt worst compress case, because lz4 algorithm supports partial
> +	 * compression.

Actually, in this case the lz4 compressed block is not valid (at least not ended
in a valid lz4 EOF), and AFAIK the current public lz4 API cannot keep on
compressing this block. so IMO "partial compression" for an invalid lz4
block may be confusing.

I think some words like "because lz4 implementation can handle output buffer
budget properly" or similar stuff could be better.

The same to the patch title and the commit message.

Thanks,
Gao Xiang


> +	 */
> +	cc->clen = cc->rlen - PAGE_SIZE - COMPRESS_HEADER_SIZE;
>  	return 0;
>  }
>  
> @@ -244,11 +249,9 @@ static int lz4_compress_pages(struct compress_ctx *cc)
>  
>  	len = LZ4_compress_default(cc->rbuf, cc->cbuf->cdata, cc->rlen,
>  						cc->clen, cc->private);
> -	if (!len) {
> -		printk_ratelimited("%sF2FS-fs (%s): lz4 compress failed\n",
> -				KERN_ERR, F2FS_I_SB(cc->inode)->sb->s_id);
> -		return -EIO;
> -	}
> +	if (!len)
> +		return -EAGAIN;
> +
>  	cc->clen = len;
>  	return 0;
>  }
> -- 
> 2.18.0.rc1