LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH 1/5] f2fs: change default compression algorithm
@ 2020-03-10 12:50 Chao Yu
  2020-03-10 12:50 ` [PATCH 2/5] f2fs: force compressed data into warm area Chao Yu
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Chao Yu @ 2020-03-10 12:50 UTC (permalink / raw)
  To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, chao, Chao Yu

Use LZ4 as default compression algorithm, as compared to LZO, it shows
almost the same compression ratio and much better decompression speed.

Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fs/f2fs/super.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index db3a63f7c769..ebffe7aa08ee 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -1577,7 +1577,7 @@ static void default_options(struct f2fs_sb_info *sbi)
 	F2FS_OPTION(sbi).test_dummy_encryption = false;
 	F2FS_OPTION(sbi).s_resuid = make_kuid(&init_user_ns, F2FS_DEF_RESUID);
 	F2FS_OPTION(sbi).s_resgid = make_kgid(&init_user_ns, F2FS_DEF_RESGID);
-	F2FS_OPTION(sbi).compress_algorithm = COMPRESS_LZO;
+	F2FS_OPTION(sbi).compress_algorithm = COMPRESS_LZ4;
 	F2FS_OPTION(sbi).compress_log_size = MIN_COMPRESS_LOG_SIZE;
 	F2FS_OPTION(sbi).compress_ext_cnt = 0;
 	F2FS_OPTION(sbi).bggc_mode = BGGC_MODE_ON;
-- 
2.18.0.rc1


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

* [PATCH 2/5] f2fs: force compressed data into warm area
  2020-03-10 12:50 [PATCH 1/5] f2fs: change default compression algorithm Chao Yu
@ 2020-03-10 12:50 ` Chao Yu
  2020-03-10 16:32   ` Jaegeuk Kim
  2020-03-10 12:50 ` [PATCH 3/5] f2fs: fix to account compressed inode correctly Chao Yu
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Chao Yu @ 2020-03-10 12:50 UTC (permalink / raw)
  To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, chao, Chao Yu

Generally, data shows better continuity in warm data area as its
default allocation direction is right, in order to enhance
sequential read/write performance of compress inode, let's force
compress data into warm area.

Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fs/f2fs/segment.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 601d67e72c50..ab1bc750712a 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -3037,9 +3037,10 @@ static int __get_segment_type_6(struct f2fs_io_info *fio)
 	if (fio->type == DATA) {
 		struct inode *inode = fio->page->mapping->host;
 
-		if (is_cold_data(fio->page) || file_is_cold(inode) ||
-				f2fs_compressed_file(inode))
+		if (is_cold_data(fio->page) || file_is_cold(inode))
 			return CURSEG_COLD_DATA;
+		if (f2fs_compressed_file(inode))
+			return CURSEG_WARM_DATA;
 		if (file_is_hot(inode) ||
 				is_inode_flag_set(inode, FI_HOT_DATA) ||
 				f2fs_is_atomic_file(inode) ||
-- 
2.18.0.rc1


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

* [PATCH 3/5] f2fs: fix to account compressed inode correctly
  2020-03-10 12:50 [PATCH 1/5] f2fs: change default compression algorithm Chao Yu
  2020-03-10 12:50 ` [PATCH 2/5] f2fs: force compressed data into warm area Chao Yu
@ 2020-03-10 12:50 ` Chao Yu
  2020-03-10 12:50 ` [PATCH 4/5] f2fs: fix to check dirty pages during compressed inode conversion Chao Yu
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Chao Yu @ 2020-03-10 12:50 UTC (permalink / raw)
  To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, chao, Chao Yu

stat_inc_compr_inode() needs to check FI_COMPRESSED_FILE flag, so
in f2fs_disable_compressed_file(), we should call stat_dec_compr_inode()
before clearing FI_COMPRESSED_FILE flag.

Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fs/f2fs/f2fs.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index bc45f738e3e0..5ba6c2382c32 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -3858,8 +3858,8 @@ static inline u64 f2fs_disable_compressed_file(struct inode *inode)
 		return fi->i_compr_blocks;
 
 	fi->i_flags &= ~F2FS_COMPR_FL;
-	clear_inode_flag(inode, FI_COMPRESSED_FILE);
 	stat_dec_compr_inode(inode);
+	clear_inode_flag(inode, FI_COMPRESSED_FILE);
 	return 0;
 }
 
-- 
2.18.0.rc1


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

* [PATCH 4/5] f2fs: fix to check dirty pages during compressed inode conversion
  2020-03-10 12:50 [PATCH 1/5] f2fs: change default compression algorithm Chao Yu
  2020-03-10 12:50 ` [PATCH 2/5] f2fs: force compressed data into warm area Chao Yu
  2020-03-10 12:50 ` [PATCH 3/5] f2fs: fix to account compressed inode correctly Chao Yu
@ 2020-03-10 12:50 ` Chao Yu
  2020-03-10 12:50 ` [PATCH 5/5] f2fs: allow to clear F2FS_COMPR_FL flag Chao Yu
  2020-03-10 16:15 ` [f2fs-dev] [PATCH 1/5] f2fs: change default compression algorithm Eric Biggers
  4 siblings, 0 replies; 10+ messages in thread
From: Chao Yu @ 2020-03-10 12:50 UTC (permalink / raw)
  To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, chao, Chao Yu

Compressed cluster can be generated during dirty data writeback,
if there is dirty pages on compressed inode, it needs to disable
converting compressed inode to non-compressed one.

Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fs/f2fs/f2fs.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 5ba6c2382c32..76d2a99520bf 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -3854,6 +3854,8 @@ static inline u64 f2fs_disable_compressed_file(struct inode *inode)
 
 	if (!f2fs_compressed_file(inode))
 		return 0;
+	if (get_dirty_pages(inode))
+		return 1;
 	if (fi->i_compr_blocks)
 		return fi->i_compr_blocks;
 
-- 
2.18.0.rc1


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

* [PATCH 5/5] f2fs: allow to clear F2FS_COMPR_FL flag
  2020-03-10 12:50 [PATCH 1/5] f2fs: change default compression algorithm Chao Yu
                   ` (2 preceding siblings ...)
  2020-03-10 12:50 ` [PATCH 4/5] f2fs: fix to check dirty pages during compressed inode conversion Chao Yu
@ 2020-03-10 12:50 ` Chao Yu
  2020-03-10 16:15 ` [f2fs-dev] [PATCH 1/5] f2fs: change default compression algorithm Eric Biggers
  4 siblings, 0 replies; 10+ messages in thread
From: Chao Yu @ 2020-03-10 12:50 UTC (permalink / raw)
  To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, chao, Chao Yu

If regular inode has no compressed cluster, allow using 'chattr -c'
to remove its compress flag, recovering it to a non-compressed file.

Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fs/f2fs/file.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 08c19ab81c80..e04a31e21448 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -1822,10 +1822,10 @@ static int f2fs_setflags_common(struct inode *inode, u32 iflags, u32 mask)
 	}
 
 	if ((iflags ^ masked_flags) & F2FS_COMPR_FL) {
-		if (S_ISREG(inode->i_mode) &&
-			(masked_flags & F2FS_COMPR_FL || i_size_read(inode) ||
-						F2FS_HAS_BLOCKS(inode)))
-			return -EINVAL;
+		if (S_ISREG(inode->i_mode) && (masked_flags & F2FS_COMPR_FL)) {
+			if (f2fs_disable_compressed_file(inode))
+				return -EINVAL;
+		}
 		if (iflags & F2FS_NOCOMP_FL)
 			return -EINVAL;
 		if (iflags & F2FS_COMPR_FL) {
-- 
2.18.0.rc1


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

* Re: [f2fs-dev] [PATCH 1/5] f2fs: change default compression algorithm
  2020-03-10 12:50 [PATCH 1/5] f2fs: change default compression algorithm Chao Yu
                   ` (3 preceding siblings ...)
  2020-03-10 12:50 ` [PATCH 5/5] f2fs: allow to clear F2FS_COMPR_FL flag Chao Yu
@ 2020-03-10 16:15 ` Eric Biggers
  2020-03-11  0:41   ` Chao Yu
  4 siblings, 1 reply; 10+ messages in thread
From: Eric Biggers @ 2020-03-10 16:15 UTC (permalink / raw)
  To: Chao Yu; +Cc: jaegeuk, linux-kernel, linux-f2fs-devel

On Tue, Mar 10, 2020 at 08:50:05PM +0800, Chao Yu wrote:
> Use LZ4 as default compression algorithm, as compared to LZO, it shows
> almost the same compression ratio and much better decompression speed.
> 
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> ---
>  fs/f2fs/super.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index db3a63f7c769..ebffe7aa08ee 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -1577,7 +1577,7 @@ static void default_options(struct f2fs_sb_info *sbi)
>  	F2FS_OPTION(sbi).test_dummy_encryption = false;
>  	F2FS_OPTION(sbi).s_resuid = make_kuid(&init_user_ns, F2FS_DEF_RESUID);
>  	F2FS_OPTION(sbi).s_resgid = make_kgid(&init_user_ns, F2FS_DEF_RESGID);
> -	F2FS_OPTION(sbi).compress_algorithm = COMPRESS_LZO;
> +	F2FS_OPTION(sbi).compress_algorithm = COMPRESS_LZ4;
>  	F2FS_OPTION(sbi).compress_log_size = MIN_COMPRESS_LOG_SIZE;
>  	F2FS_OPTION(sbi).compress_ext_cnt = 0;
>  	F2FS_OPTION(sbi).bggc_mode = BGGC_MODE_ON;

This makes sense, but it's unclear to me why comparing the different compression
algorithms is happening just now, after support for both LZO and LZ4 was already
merged into mainline and now has to be supported forever.  During review months
ago, multiple people suggested that LZ4 is better than LZO, so there's not much
reason to support LZO at all.

- Eric

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

* Re: [PATCH 2/5] f2fs: force compressed data into warm area
  2020-03-10 12:50 ` [PATCH 2/5] f2fs: force compressed data into warm area Chao Yu
@ 2020-03-10 16:32   ` Jaegeuk Kim
  2020-03-11  3:45     ` Chao Yu
  0 siblings, 1 reply; 10+ messages in thread
From: Jaegeuk Kim @ 2020-03-10 16:32 UTC (permalink / raw)
  To: Chao Yu; +Cc: linux-f2fs-devel, linux-kernel, chao

On 03/10, Chao Yu wrote:
> Generally, data shows better continuity in warm data area as its
> default allocation direction is right, in order to enhance
> sequential read/write performance of compress inode, let's force
> compress data into warm area.

Not quite sure tho, compressed blocks are logically cold, no?

> 
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> ---
>  fs/f2fs/segment.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 601d67e72c50..ab1bc750712a 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -3037,9 +3037,10 @@ static int __get_segment_type_6(struct f2fs_io_info *fio)
>  	if (fio->type == DATA) {
>  		struct inode *inode = fio->page->mapping->host;
>  
> -		if (is_cold_data(fio->page) || file_is_cold(inode) ||
> -				f2fs_compressed_file(inode))
> +		if (is_cold_data(fio->page) || file_is_cold(inode))
>  			return CURSEG_COLD_DATA;
> +		if (f2fs_compressed_file(inode))
> +			return CURSEG_WARM_DATA;
>  		if (file_is_hot(inode) ||
>  				is_inode_flag_set(inode, FI_HOT_DATA) ||
>  				f2fs_is_atomic_file(inode) ||
> -- 
> 2.18.0.rc1

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

* Re: [f2fs-dev] [PATCH 1/5] f2fs: change default compression algorithm
  2020-03-10 16:15 ` [f2fs-dev] [PATCH 1/5] f2fs: change default compression algorithm Eric Biggers
@ 2020-03-11  0:41   ` Chao Yu
  2020-03-11  3:07     ` Jaegeuk Kim
  0 siblings, 1 reply; 10+ messages in thread
From: Chao Yu @ 2020-03-11  0:41 UTC (permalink / raw)
  To: Eric Biggers; +Cc: jaegeuk, linux-kernel, linux-f2fs-devel

On 2020/3/11 0:15, Eric Biggers wrote:
> On Tue, Mar 10, 2020 at 08:50:05PM +0800, Chao Yu wrote:
>> Use LZ4 as default compression algorithm, as compared to LZO, it shows
>> almost the same compression ratio and much better decompression speed.
>>
>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
>> ---
>>  fs/f2fs/super.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>> index db3a63f7c769..ebffe7aa08ee 100644
>> --- a/fs/f2fs/super.c
>> +++ b/fs/f2fs/super.c
>> @@ -1577,7 +1577,7 @@ static void default_options(struct f2fs_sb_info *sbi)
>>  	F2FS_OPTION(sbi).test_dummy_encryption = false;
>>  	F2FS_OPTION(sbi).s_resuid = make_kuid(&init_user_ns, F2FS_DEF_RESUID);
>>  	F2FS_OPTION(sbi).s_resgid = make_kgid(&init_user_ns, F2FS_DEF_RESGID);
>> -	F2FS_OPTION(sbi).compress_algorithm = COMPRESS_LZO;
>> +	F2FS_OPTION(sbi).compress_algorithm = COMPRESS_LZ4;
>>  	F2FS_OPTION(sbi).compress_log_size = MIN_COMPRESS_LOG_SIZE;
>>  	F2FS_OPTION(sbi).compress_ext_cnt = 0;
>>  	F2FS_OPTION(sbi).bggc_mode = BGGC_MODE_ON;
> 
> This makes sense, but it's unclear to me why comparing the different compression
> algorithms is happening just now, after support for both LZO and LZ4 was already
> merged into mainline and now has to be supported forever.  During review months
> ago, multiple people suggested that LZ4 is better than LZO, so there's not much
> reason to support LZO at all.

Agreed,

Jaegeuk, thoughts?

Let me remove lzo if you have no objection on this.

Thanks,

> 
> - Eric
> .
> 

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

* Re: [f2fs-dev] [PATCH 1/5] f2fs: change default compression algorithm
  2020-03-11  0:41   ` Chao Yu
@ 2020-03-11  3:07     ` Jaegeuk Kim
  0 siblings, 0 replies; 10+ messages in thread
From: Jaegeuk Kim @ 2020-03-11  3:07 UTC (permalink / raw)
  To: Chao Yu; +Cc: Eric Biggers, linux-kernel, linux-f2fs-devel

On 03/11, Chao Yu wrote:
> On 2020/3/11 0:15, Eric Biggers wrote:
> > On Tue, Mar 10, 2020 at 08:50:05PM +0800, Chao Yu wrote:
> >> Use LZ4 as default compression algorithm, as compared to LZO, it shows
> >> almost the same compression ratio and much better decompression speed.
> >>
> >> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> >> ---
> >>  fs/f2fs/super.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> >> index db3a63f7c769..ebffe7aa08ee 100644
> >> --- a/fs/f2fs/super.c
> >> +++ b/fs/f2fs/super.c
> >> @@ -1577,7 +1577,7 @@ static void default_options(struct f2fs_sb_info *sbi)
> >>  	F2FS_OPTION(sbi).test_dummy_encryption = false;
> >>  	F2FS_OPTION(sbi).s_resuid = make_kuid(&init_user_ns, F2FS_DEF_RESUID);
> >>  	F2FS_OPTION(sbi).s_resgid = make_kgid(&init_user_ns, F2FS_DEF_RESGID);
> >> -	F2FS_OPTION(sbi).compress_algorithm = COMPRESS_LZO;
> >> +	F2FS_OPTION(sbi).compress_algorithm = COMPRESS_LZ4;
> >>  	F2FS_OPTION(sbi).compress_log_size = MIN_COMPRESS_LOG_SIZE;
> >>  	F2FS_OPTION(sbi).compress_ext_cnt = 0;
> >>  	F2FS_OPTION(sbi).bggc_mode = BGGC_MODE_ON;
> > 
> > This makes sense, but it's unclear to me why comparing the different compression
> > algorithms is happening just now, after support for both LZO and LZ4 was already
> > merged into mainline and now has to be supported forever.  During review months
> > ago, multiple people suggested that LZ4 is better than LZO, so there's not much
> > reason to support LZO at all.
> 
> Agreed,
> 
> Jaegeuk, thoughts?

Supporting LZO or whatever algorithms looks fine to me as long as kernel
supports without huge maintenance effort. I think it'd be good to provide
a way for users to compare several algorithms in whatever their ways.

And, unfortunately, I don't think we can remove LZO at this point, even for -rc.

Thanks,

> 
> Let me remove lzo if you have no objection on this.
> 
> Thanks,
> 
> > 
> > - Eric
> > .
> > 

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

* Re: [PATCH 2/5] f2fs: force compressed data into warm area
  2020-03-10 16:32   ` Jaegeuk Kim
@ 2020-03-11  3:45     ` Chao Yu
  0 siblings, 0 replies; 10+ messages in thread
From: Chao Yu @ 2020-03-11  3:45 UTC (permalink / raw)
  To: Jaegeuk Kim; +Cc: linux-f2fs-devel, linux-kernel, chao

On 2020/3/11 0:32, Jaegeuk Kim wrote:
> On 03/10, Chao Yu wrote:
>> Generally, data shows better continuity in warm data area as its
>> default allocation direction is right, in order to enhance
>> sequential read/write performance of compress inode, let's force
>> compress data into warm area.

It looks cold data's allocation direction is right as well, I missed
to notice that previously due to I tested in small-sized image, and
f2fs_tuning_parameters() enables ALLOC_MODE_REUSE, then cold data
allocation always searchs free segment from segno #0, it breaks
continuity of physical blocks.

> 
> Not quite sure tho, compressed blocks are logically cold, no?

Correct.

Please ignore this patch. :P

Thanks,

> 
>>
>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
>> ---
>>  fs/f2fs/segment.c | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>> index 601d67e72c50..ab1bc750712a 100644
>> --- a/fs/f2fs/segment.c
>> +++ b/fs/f2fs/segment.c
>> @@ -3037,9 +3037,10 @@ static int __get_segment_type_6(struct f2fs_io_info *fio)
>>  	if (fio->type == DATA) {
>>  		struct inode *inode = fio->page->mapping->host;
>>  
>> -		if (is_cold_data(fio->page) || file_is_cold(inode) ||
>> -				f2fs_compressed_file(inode))
>> +		if (is_cold_data(fio->page) || file_is_cold(inode))
>>  			return CURSEG_COLD_DATA;
>> +		if (f2fs_compressed_file(inode))
>> +			return CURSEG_WARM_DATA;
>>  		if (file_is_hot(inode) ||
>>  				is_inode_flag_set(inode, FI_HOT_DATA) ||
>>  				f2fs_is_atomic_file(inode) ||
>> -- 
>> 2.18.0.rc1
> .
> 

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

end of thread, other threads:[~2020-03-11  3:45 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-10 12:50 [PATCH 1/5] f2fs: change default compression algorithm Chao Yu
2020-03-10 12:50 ` [PATCH 2/5] f2fs: force compressed data into warm area Chao Yu
2020-03-10 16:32   ` Jaegeuk Kim
2020-03-11  3:45     ` Chao Yu
2020-03-10 12:50 ` [PATCH 3/5] f2fs: fix to account compressed inode correctly Chao Yu
2020-03-10 12:50 ` [PATCH 4/5] f2fs: fix to check dirty pages during compressed inode conversion Chao Yu
2020-03-10 12:50 ` [PATCH 5/5] f2fs: allow to clear F2FS_COMPR_FL flag Chao Yu
2020-03-10 16:15 ` [f2fs-dev] [PATCH 1/5] f2fs: change default compression algorithm Eric Biggers
2020-03-11  0:41   ` Chao Yu
2020-03-11  3:07     ` Jaegeuk Kim

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).