[PATCH] f2fs: compress: allow lz4 to compress data partially

[PATCH] f2fs: compress: allow lz4 to compress data partially

[Chao Yu]

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@google.com>
Signed-off-by: Chao Yu <yuchao0@huawei.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.
+	 */
+	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

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

[Chao Yu]

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@google.com>
Signed-off-by: Chao Yu <yuchao0@huawei.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.
+	 */
+	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



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

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

[Gao Xiang]

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@google.com>
> Signed-off-by: Chao Yu <yuchao0@huawei.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

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

[Gao Xiang via Linux-f2fs-devel]

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@google.com>
> Signed-off-by: Chao Yu <yuchao0@huawei.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


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

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

[Chao Yu]

Hi Xiang,

On 2020/5/8 18:23, Gao Xiang wrote:
> 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@google.com>
>> Signed-off-by: Chao Yu <yuchao0@huawei.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.

Yes, comments could be improved to avoid confusing.

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

It's better, let me change to use this, thanks for the advice.

Thanks,

> 
> 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
> .
> 

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

[Chao Yu]

Hi Xiang,

On 2020/5/8 18:23, Gao Xiang wrote:
> 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@google.com>
>> Signed-off-by: Chao Yu <yuchao0@huawei.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.

Yes, comments could be improved to avoid confusing.

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

It's better, let me change to use this, thanks for the advice.

Thanks,

> 
> 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
> .
> 


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel