Re: [f2fs-dev] [PATCH 2/2] f2fs: support data compression

[Chao Yu <yuchao0@huawei.com>]

Hi Ju Hyung,

On 2019/10/23 1:53, Ju Hyung Park wrote:
> Hi Jaegeuk and Chao,
> 
> Nice to see this finally getting into shape :) Great work
> I'm excited to see possible use-cases for this in the future.
> 
> Would f2fs compress files automatically like how btrfs' "compress" option works?
> Or is it per-extension basis for now?

We support three ways to active file compression:

Quoted:

- To enable compression on regular inode, there are three ways:
* chattr +c file
* chattr +c dir; touch dir/file
* mount w/ -o compress_extension=ext; touch file.ext

> 
> On Wed, Oct 23, 2019 at 2:16 AM Jaegeuk Kim <jaegeuk@kernel.org> wrote:
>> +compress_algorithm=%s  Control compress algorithm, currently f2fs supports "lzo"
>> +                       and "lz4" algorithm.
> 
> I see absolutely no reason to support regular lzo variant at this time.
> Everyone should use lz4 instead of lzo. If one wants zlib-level
> compression, they should use zstd.
> 
> However, there's recent conversation on new lzo-rle and how it could
> be a better candidate than lz4.
> 
> Since the mainline now have lz4, zstd and lzo-rle, I don't think
> supporting lzo is a good idea.

This is just RFC version, we can change it anytime, let's decide whether
deleting it before final version.

> 
>> diff --git a/fs/f2fs/Kconfig b/fs/f2fs/Kconfig
>> index 652fd2e2b23d..c12854c3b1a1 100644
>> --- a/fs/f2fs/Kconfig
>> +++ b/fs/f2fs/Kconfig
>> @@ -6,6 +6,10 @@ config F2FS_FS
>>         select CRYPTO
>>         select CRYPTO_CRC32
>>         select F2FS_FS_XATTR if FS_ENCRYPTION
>> +       select LZO_COMPRESS
>> +       select LZO_DECOMPRESS
>> +       select LZ4_COMPRESS
>> +       select LZ4_DECOMPRESS
> 
> This is a bad idea.
> This unnecessarily increases kernel binary image when no the user
> intends to change the defaults.
> 
> For example, my Android kernel doesn't use lzo anywhere and this
> wouldn't be welcome.

Agreed, maybe we need a kconfig entry for compress.c as well.

> 
>> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
>> new file mode 100644
>> index 000000000000..f276d82a67aa
>> --- /dev/null
>> +++ b/fs/f2fs/compress.c
>> @@ -0,0 +1,1066 @@
>> +static unsigned int offset_in_cluster(struct compress_ctx *cc, pgoff_t index)
>> +static unsigned int cluster_idx(struct compress_ctx *cc, pgoff_t index)
>> +static unsigned int start_idx_of_cluster(struct compress_ctx *cc)
> 
> Looks like these would be better if they were explicitly marked as inline.
> 
>> +static void f2fs_init_compress_ops(struct f2fs_sb_info *sbi)
>> +{
>> +       sbi->cops[COMPRESS_LZO] = &f2fs_lzo_ops;
>> +       sbi->cops[COMPRESS_LZ4] = &f2fs_lz4_ops;
>> +}
> 
> Would it be possible for f2fs to use generic crypto compression APIs?
> Hardcoding for lzo/lz4 would make it harder to venture future different options.
> 
> Have a look at mm/zswap.c:__zswap_pool_create_fallback().

Not sure, I think I could look into it later, now Jaegeuk and I have to
stabilize codes first.

Thanks for your advice anyway.

> 
>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>> index c681f51e351b..775c96291490 100644
>> --- a/fs/f2fs/f2fs.h
>> +++ b/fs/f2fs/f2fs.h
>> @@ -155,6 +163,7 @@ struct f2fs_mount_info {
>>  #define F2FS_FEATURE_VERITY            0x0400
>>  #define F2FS_FEATURE_SB_CHKSUM         0x0800
>>  #define F2FS_FEATURE_CASEFOLD          0x1000
>> +#define F2FS_FEATURE_COMPRESSION       0x2000
> 
> How would older versions of f2fs behave if an image was used by the
> latest f2fs and have compressed pages?
> I hope fail-safes are in place.

That patch haven't merged yet, since there is detailed implementation which is
under discussion.

Thanks,

> 
> Thanks.
> 
>> --
>> 2.19.0.605.g01d371f741-goog
>>
>>
>>
>> _______________________________________________
>> Linux-f2fs-devel mailing list
>> Linux-f2fs-devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> .
> 


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