Linux-f2fs-devel Archive on lore.kernel.org
 help / color / Atom feed
* [f2fs-dev] [PATCH] f2fs: compress: allow lz4 to compress data partially
@ 2020-05-08  9:47 Chao Yu
  2020-05-08 10:23 ` Gao Xiang via Linux-f2fs-devel
  0 siblings, 1 reply; 3+ messages in thread
From: Chao Yu @ 2020-05-08  9:47 UTC (permalink / raw)
  To: jaegeuk; +Cc: linux-kernel, linux-f2fs-devel

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [f2fs-dev] [PATCH] f2fs: compress: allow lz4 to compress data partially
  2020-05-08  9:47 [f2fs-dev] [PATCH] f2fs: compress: allow lz4 to compress data partially Chao Yu
@ 2020-05-08 10:23 ` Gao Xiang via Linux-f2fs-devel
  2020-05-09  1:56   ` Chao Yu
  0 siblings, 1 reply; 3+ messages in thread
From: Gao Xiang via Linux-f2fs-devel @ 2020-05-08 10:23 UTC (permalink / raw)
  To: Chao Yu; +Cc: jaegeuk, linux-kernel, 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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [f2fs-dev] [PATCH] f2fs: compress: allow lz4 to compress data partially
  2020-05-08 10:23 ` Gao Xiang via Linux-f2fs-devel
@ 2020-05-09  1:56   ` Chao Yu
  0 siblings, 0 replies; 3+ messages in thread
From: Chao Yu @ 2020-05-09  1:56 UTC (permalink / raw)
  To: Gao Xiang; +Cc: jaegeuk, linux-kernel, linux-f2fs-devel

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-08  9:47 [f2fs-dev] [PATCH] f2fs: compress: allow lz4 to compress data partially Chao Yu
2020-05-08 10:23 ` Gao Xiang via Linux-f2fs-devel
2020-05-09  1:56   ` Chao Yu

Linux-f2fs-devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-f2fs-devel/0 linux-f2fs-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-f2fs-devel linux-f2fs-devel/ https://lore.kernel.org/linux-f2fs-devel \
		linux-f2fs-devel@lists.sourceforge.net
	public-inbox-index linux-f2fs-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/net.sourceforge.lists.linux-f2fs-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git