[PATCH 1/5] f2fs: change default compression algorithm

[PATCH 1/5] f2fs: change default compression algorithm

[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

[PATCH 2/5] f2fs: force compressed data into warm area

[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

[PATCH 3/5] f2fs: fix to account compressed inode correctly

[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

[PATCH 4/5] f2fs: fix to check dirty pages during compressed inode conversion

[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

[PATCH 5/5] f2fs: allow to clear F2FS_COMPR_FL flag

[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

Re: [f2fs-dev] [PATCH 1/5] f2fs: change default compression algorithm

[Eric Biggers]

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

Re: [PATCH 2/5] f2fs: force compressed data into warm area

[Jaegeuk Kim]

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

Re: [f2fs-dev] [PATCH 1/5] f2fs: change default compression algorithm

[Chao Yu]

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

Re: [f2fs-dev] [PATCH 1/5] f2fs: change default compression algorithm

[Jaegeuk Kim]

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

Re: [PATCH 2/5] f2fs: force compressed data into warm area

[Chao Yu]

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