linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4] f2fs: compress: support chksum
@ 2020-12-08  3:14 Chao Yu
  2020-12-09  1:37 ` Chao Yu
  0 siblings, 1 reply; 10+ messages in thread
From: Chao Yu @ 2020-12-08  3:14 UTC (permalink / raw)
  To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, chao, Chao Yu

This patch supports to store chksum value with compressed
data, and verify the integrality of compressed data while
reading the data.

The feature can be enabled through specifying mount option
'compress_chksum'.

Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
v4:
- enhance readability
- remove WARN_ON_ONCE()
 Documentation/filesystems/f2fs.rst |  1 +
 fs/f2fs/compress.c                 | 22 ++++++++++++++++++++++
 fs/f2fs/f2fs.h                     | 16 ++++++++++++++--
 fs/f2fs/inode.c                    |  3 +++
 fs/f2fs/super.c                    |  9 +++++++++
 include/linux/f2fs_fs.h            |  2 +-
 6 files changed, 50 insertions(+), 3 deletions(-)

diff --git a/Documentation/filesystems/f2fs.rst b/Documentation/filesystems/f2fs.rst
index b8ee761c9922..985ae7d35066 100644
--- a/Documentation/filesystems/f2fs.rst
+++ b/Documentation/filesystems/f2fs.rst
@@ -260,6 +260,7 @@ compress_extension=%s	 Support adding specified extension, so that f2fs can enab
 			 For other files, we can still enable compression via ioctl.
 			 Note that, there is one reserved special extension '*', it
 			 can be set to enable compression for all files.
+compress_chksum		 Support verifying chksum of raw data in compressed cluster.
 inlinecrypt		 When possible, encrypt/decrypt the contents of encrypted
 			 files using the blk-crypto framework rather than
 			 filesystem-layer encryption. This allows the use of
diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
index 14262e0f1cd6..9313c8695855 100644
--- a/fs/f2fs/compress.c
+++ b/fs/f2fs/compress.c
@@ -602,6 +602,7 @@ static int f2fs_compress_pages(struct compress_ctx *cc)
 				f2fs_cops[fi->i_compress_algorithm];
 	unsigned int max_len, new_nr_cpages;
 	struct page **new_cpages;
+	u32 chksum = 0;
 	int i, ret;
 
 	trace_f2fs_compress_pages_start(cc->inode, cc->cluster_idx,
@@ -655,6 +656,11 @@ static int f2fs_compress_pages(struct compress_ctx *cc)
 
 	cc->cbuf->clen = cpu_to_le32(cc->clen);
 
+	if (fi->i_compress_flag & 1 << COMPRESS_CHKSUM)
+		chksum = f2fs_crc32(F2FS_I_SB(cc->inode),
+					cc->cbuf->cdata, cc->clen);
+	cc->cbuf->chksum = cpu_to_le32(chksum);
+
 	for (i = 0; i < COMPRESS_DATA_RESERVED_SIZE; i++)
 		cc->cbuf->reserved[i] = cpu_to_le32(0);
 
@@ -790,6 +796,22 @@ void f2fs_decompress_pages(struct bio *bio, struct page *page, bool verity)
 
 	ret = cops->decompress_pages(dic);
 
+	if (!ret && (fi->i_compress_flag & 1 << COMPRESS_CHKSUM)) {
+		u32 provided = le32_to_cpu(dic->cbuf->chksum);
+		u32 calculated = f2fs_crc32(sbi, dic->cbuf->cdata, dic->clen);
+
+		if (provided != calculated) {
+			if (!is_inode_flag_set(dic->inode, FI_COMPRESS_CORRUPT)) {
+				set_inode_flag(dic->inode, FI_COMPRESS_CORRUPT);
+				printk_ratelimited(
+					"%sF2FS-fs (%s): checksum invalid, nid = %lu, %x vs %x",
+					KERN_INFO, sbi->sb->s_id, dic->inode->i_ino,
+					provided, calculated);
+			}
+			set_sbi_flag(sbi, SBI_NEED_FSCK);
+		}
+	}
+
 out_vunmap_cbuf:
 	vm_unmap_ram(dic->cbuf, dic->nr_cpages);
 out_vunmap_rbuf:
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 0d25f5ca5618..0b314b2034d8 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -147,7 +147,8 @@ struct f2fs_mount_info {
 
 	/* For compression */
 	unsigned char compress_algorithm;	/* algorithm type */
-	unsigned compress_log_size;		/* cluster log size */
+	unsigned char compress_log_size;	/* cluster log size */
+	bool compress_chksum;			/* compressed data chksum */
 	unsigned char compress_ext_cnt;		/* extension count */
 	unsigned char extensions[COMPRESS_EXT_NUM][F2FS_EXTENSION_LEN];	/* extensions */
 };
@@ -676,6 +677,7 @@ enum {
 	FI_ATOMIC_REVOKE_REQUEST, /* request to drop atomic data */
 	FI_VERITY_IN_PROGRESS,	/* building fs-verity Merkle tree */
 	FI_COMPRESSED_FILE,	/* indicate file's data can be compressed */
+	FI_COMPRESS_CORRUPT,	/* indicate compressed cluster is corrupted */
 	FI_MMAP_FILE,		/* indicate file was mmapped */
 	FI_MAX,			/* max flag, never be used */
 };
@@ -733,6 +735,7 @@ struct f2fs_inode_info {
 	atomic_t i_compr_blocks;		/* # of compressed blocks */
 	unsigned char i_compress_algorithm;	/* algorithm type */
 	unsigned char i_log_cluster_size;	/* log of cluster size */
+	unsigned short i_compress_flag;		/* compress flag */
 	unsigned int i_cluster_size;		/* cluster size */
 };
 
@@ -1272,9 +1275,15 @@ enum compress_algorithm_type {
 	COMPRESS_MAX,
 };
 
-#define COMPRESS_DATA_RESERVED_SIZE		5
+enum compress_flag {
+	COMPRESS_CHKSUM,
+	COMPRESS_MAX_FLAG,
+};
+
+#define COMPRESS_DATA_RESERVED_SIZE		4
 struct compress_data {
 	__le32 clen;			/* compressed data size */
+	__le32 chksum;			/* compressed data chksum */
 	__le32 reserved[COMPRESS_DATA_RESERVED_SIZE];	/* reserved */
 	u8 cdata[];			/* compressed data */
 };
@@ -3888,6 +3897,9 @@ static inline void set_compress_context(struct inode *inode)
 			F2FS_OPTION(sbi).compress_algorithm;
 	F2FS_I(inode)->i_log_cluster_size =
 			F2FS_OPTION(sbi).compress_log_size;
+	F2FS_I(inode)->i_compress_flag =
+			F2FS_OPTION(sbi).compress_chksum ?
+				1 << COMPRESS_CHKSUM : 0;
 	F2FS_I(inode)->i_cluster_size =
 			1 << F2FS_I(inode)->i_log_cluster_size;
 	F2FS_I(inode)->i_flags |= F2FS_COMPR_FL;
diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
index 657db2fb6739..349d9cb933ee 100644
--- a/fs/f2fs/inode.c
+++ b/fs/f2fs/inode.c
@@ -456,6 +456,7 @@ static int do_read_inode(struct inode *inode)
 					le64_to_cpu(ri->i_compr_blocks));
 			fi->i_compress_algorithm = ri->i_compress_algorithm;
 			fi->i_log_cluster_size = ri->i_log_cluster_size;
+			fi->i_compress_flag = le16_to_cpu(ri->i_compress_flag);
 			fi->i_cluster_size = 1 << fi->i_log_cluster_size;
 			set_inode_flag(inode, FI_COMPRESSED_FILE);
 		}
@@ -634,6 +635,8 @@ void f2fs_update_inode(struct inode *inode, struct page *node_page)
 					&F2FS_I(inode)->i_compr_blocks));
 			ri->i_compress_algorithm =
 				F2FS_I(inode)->i_compress_algorithm;
+			ri->i_compress_flag =
+				cpu_to_le16(F2FS_I(inode)->i_compress_flag);
 			ri->i_log_cluster_size =
 				F2FS_I(inode)->i_log_cluster_size;
 		}
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 82baaa89c893..f3d919ee4dee 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -146,6 +146,7 @@ enum {
 	Opt_compress_algorithm,
 	Opt_compress_log_size,
 	Opt_compress_extension,
+	Opt_compress_chksum,
 	Opt_atgc,
 	Opt_err,
 };
@@ -214,6 +215,7 @@ static match_table_t f2fs_tokens = {
 	{Opt_compress_algorithm, "compress_algorithm=%s"},
 	{Opt_compress_log_size, "compress_log_size=%u"},
 	{Opt_compress_extension, "compress_extension=%s"},
+	{Opt_compress_chksum, "compress_chksum"},
 	{Opt_atgc, "atgc"},
 	{Opt_err, NULL},
 };
@@ -934,10 +936,14 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
 			F2FS_OPTION(sbi).compress_ext_cnt++;
 			kfree(name);
 			break;
+		case Opt_compress_chksum:
+			F2FS_OPTION(sbi).compress_chksum = true;
+			break;
 #else
 		case Opt_compress_algorithm:
 		case Opt_compress_log_size:
 		case Opt_compress_extension:
+		case Opt_compress_chksum:
 			f2fs_info(sbi, "compression options not supported");
 			break;
 #endif
@@ -1523,6 +1529,9 @@ static inline void f2fs_show_compress_options(struct seq_file *seq,
 		seq_printf(seq, ",compress_extension=%s",
 			F2FS_OPTION(sbi).extensions[i]);
 	}
+
+	if (F2FS_OPTION(sbi).compress_chksum)
+		seq_puts(seq, ",compress_chksum");
 }
 
 static int f2fs_show_options(struct seq_file *seq, struct dentry *root)
diff --git a/include/linux/f2fs_fs.h b/include/linux/f2fs_fs.h
index a5dbb57a687f..7dc2a06cf19a 100644
--- a/include/linux/f2fs_fs.h
+++ b/include/linux/f2fs_fs.h
@@ -273,7 +273,7 @@ struct f2fs_inode {
 			__le64 i_compr_blocks;	/* # of compressed blocks */
 			__u8 i_compress_algorithm;	/* compress algorithm */
 			__u8 i_log_cluster_size;	/* log of cluster size */
-			__le16 i_padding;		/* padding */
+			__le16 i_compress_flag;		/* compress flag */
 			__le32 i_extra_end[0];	/* for attribute size calculation */
 		} __packed;
 		__le32 i_addr[DEF_ADDRS_PER_INODE];	/* Pointers to data blocks */
-- 
2.29.2


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

* Re: [PATCH v4] f2fs: compress: support chksum
  2020-12-08  3:14 [PATCH v4] f2fs: compress: support chksum Chao Yu
@ 2020-12-09  1:37 ` Chao Yu
  2020-12-09  2:49   ` Jaegeuk Kim
  0 siblings, 1 reply; 10+ messages in thread
From: Chao Yu @ 2020-12-09  1:37 UTC (permalink / raw)
  To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, chao

Hello guys,

Any further comments on compress related patches?

Jaegeuk, could you please queue these patches in dev-test? let me know
if there is any problem.

On 2020/12/8 11:14, Chao Yu wrote:
> This patch supports to store chksum value with compressed
> data, and verify the integrality of compressed data while
> reading the data.
> 
> The feature can be enabled through specifying mount option
> 'compress_chksum'.
> 
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> ---
> v4:
> - enhance readability
> - remove WARN_ON_ONCE()
>   Documentation/filesystems/f2fs.rst |  1 +
>   fs/f2fs/compress.c                 | 22 ++++++++++++++++++++++
>   fs/f2fs/f2fs.h                     | 16 ++++++++++++++--
>   fs/f2fs/inode.c                    |  3 +++
>   fs/f2fs/super.c                    |  9 +++++++++
>   include/linux/f2fs_fs.h            |  2 +-
>   6 files changed, 50 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/filesystems/f2fs.rst b/Documentation/filesystems/f2fs.rst
> index b8ee761c9922..985ae7d35066 100644
> --- a/Documentation/filesystems/f2fs.rst
> +++ b/Documentation/filesystems/f2fs.rst
> @@ -260,6 +260,7 @@ compress_extension=%s	 Support adding specified extension, so that f2fs can enab
>   			 For other files, we can still enable compression via ioctl.
>   			 Note that, there is one reserved special extension '*', it
>   			 can be set to enable compression for all files.
> +compress_chksum		 Support verifying chksum of raw data in compressed cluster.
>   inlinecrypt		 When possible, encrypt/decrypt the contents of encrypted
>   			 files using the blk-crypto framework rather than
>   			 filesystem-layer encryption. This allows the use of
> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> index 14262e0f1cd6..9313c8695855 100644
> --- a/fs/f2fs/compress.c
> +++ b/fs/f2fs/compress.c
> @@ -602,6 +602,7 @@ static int f2fs_compress_pages(struct compress_ctx *cc)
>   				f2fs_cops[fi->i_compress_algorithm];
>   	unsigned int max_len, new_nr_cpages;
>   	struct page **new_cpages;
> +	u32 chksum = 0;
>   	int i, ret;
>   
>   	trace_f2fs_compress_pages_start(cc->inode, cc->cluster_idx,
> @@ -655,6 +656,11 @@ static int f2fs_compress_pages(struct compress_ctx *cc)
>   
>   	cc->cbuf->clen = cpu_to_le32(cc->clen);
>   
> +	if (fi->i_compress_flag & 1 << COMPRESS_CHKSUM)
> +		chksum = f2fs_crc32(F2FS_I_SB(cc->inode),
> +					cc->cbuf->cdata, cc->clen);
> +	cc->cbuf->chksum = cpu_to_le32(chksum);
> +
>   	for (i = 0; i < COMPRESS_DATA_RESERVED_SIZE; i++)
>   		cc->cbuf->reserved[i] = cpu_to_le32(0);
>   
> @@ -790,6 +796,22 @@ void f2fs_decompress_pages(struct bio *bio, struct page *page, bool verity)
>   
>   	ret = cops->decompress_pages(dic);
>   
> +	if (!ret && (fi->i_compress_flag & 1 << COMPRESS_CHKSUM)) {
> +		u32 provided = le32_to_cpu(dic->cbuf->chksum);
> +		u32 calculated = f2fs_crc32(sbi, dic->cbuf->cdata, dic->clen);
> +
> +		if (provided != calculated) {
> +			if (!is_inode_flag_set(dic->inode, FI_COMPRESS_CORRUPT)) {
> +				set_inode_flag(dic->inode, FI_COMPRESS_CORRUPT);
> +				printk_ratelimited(
> +					"%sF2FS-fs (%s): checksum invalid, nid = %lu, %x vs %x",
> +					KERN_INFO, sbi->sb->s_id, dic->inode->i_ino,
> +					provided, calculated);
> +			}
> +			set_sbi_flag(sbi, SBI_NEED_FSCK);
> +		}
> +	}
> +
>   out_vunmap_cbuf:
>   	vm_unmap_ram(dic->cbuf, dic->nr_cpages);
>   out_vunmap_rbuf:
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 0d25f5ca5618..0b314b2034d8 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -147,7 +147,8 @@ struct f2fs_mount_info {
>   
>   	/* For compression */
>   	unsigned char compress_algorithm;	/* algorithm type */
> -	unsigned compress_log_size;		/* cluster log size */
> +	unsigned char compress_log_size;	/* cluster log size */
> +	bool compress_chksum;			/* compressed data chksum */
>   	unsigned char compress_ext_cnt;		/* extension count */
>   	unsigned char extensions[COMPRESS_EXT_NUM][F2FS_EXTENSION_LEN];	/* extensions */
>   };
> @@ -676,6 +677,7 @@ enum {
>   	FI_ATOMIC_REVOKE_REQUEST, /* request to drop atomic data */
>   	FI_VERITY_IN_PROGRESS,	/* building fs-verity Merkle tree */
>   	FI_COMPRESSED_FILE,	/* indicate file's data can be compressed */
> +	FI_COMPRESS_CORRUPT,	/* indicate compressed cluster is corrupted */
>   	FI_MMAP_FILE,		/* indicate file was mmapped */
>   	FI_MAX,			/* max flag, never be used */
>   };
> @@ -733,6 +735,7 @@ struct f2fs_inode_info {
>   	atomic_t i_compr_blocks;		/* # of compressed blocks */
>   	unsigned char i_compress_algorithm;	/* algorithm type */
>   	unsigned char i_log_cluster_size;	/* log of cluster size */
> +	unsigned short i_compress_flag;		/* compress flag */
>   	unsigned int i_cluster_size;		/* cluster size */
>   };
>   
> @@ -1272,9 +1275,15 @@ enum compress_algorithm_type {
>   	COMPRESS_MAX,
>   };
>   
> -#define COMPRESS_DATA_RESERVED_SIZE		5
> +enum compress_flag {
> +	COMPRESS_CHKSUM,
> +	COMPRESS_MAX_FLAG,
> +};
> +
> +#define COMPRESS_DATA_RESERVED_SIZE		4
>   struct compress_data {
>   	__le32 clen;			/* compressed data size */
> +	__le32 chksum;			/* compressed data chksum */
>   	__le32 reserved[COMPRESS_DATA_RESERVED_SIZE];	/* reserved */
>   	u8 cdata[];			/* compressed data */
>   };
> @@ -3888,6 +3897,9 @@ static inline void set_compress_context(struct inode *inode)
>   			F2FS_OPTION(sbi).compress_algorithm;
>   	F2FS_I(inode)->i_log_cluster_size =
>   			F2FS_OPTION(sbi).compress_log_size;
> +	F2FS_I(inode)->i_compress_flag =
> +			F2FS_OPTION(sbi).compress_chksum ?
> +				1 << COMPRESS_CHKSUM : 0;
>   	F2FS_I(inode)->i_cluster_size =
>   			1 << F2FS_I(inode)->i_log_cluster_size;
>   	F2FS_I(inode)->i_flags |= F2FS_COMPR_FL;
> diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
> index 657db2fb6739..349d9cb933ee 100644
> --- a/fs/f2fs/inode.c
> +++ b/fs/f2fs/inode.c
> @@ -456,6 +456,7 @@ static int do_read_inode(struct inode *inode)
>   					le64_to_cpu(ri->i_compr_blocks));
>   			fi->i_compress_algorithm = ri->i_compress_algorithm;
>   			fi->i_log_cluster_size = ri->i_log_cluster_size;
> +			fi->i_compress_flag = le16_to_cpu(ri->i_compress_flag);
>   			fi->i_cluster_size = 1 << fi->i_log_cluster_size;
>   			set_inode_flag(inode, FI_COMPRESSED_FILE);
>   		}
> @@ -634,6 +635,8 @@ void f2fs_update_inode(struct inode *inode, struct page *node_page)
>   					&F2FS_I(inode)->i_compr_blocks));
>   			ri->i_compress_algorithm =
>   				F2FS_I(inode)->i_compress_algorithm;
> +			ri->i_compress_flag =
> +				cpu_to_le16(F2FS_I(inode)->i_compress_flag);
>   			ri->i_log_cluster_size =
>   				F2FS_I(inode)->i_log_cluster_size;
>   		}
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 82baaa89c893..f3d919ee4dee 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -146,6 +146,7 @@ enum {
>   	Opt_compress_algorithm,
>   	Opt_compress_log_size,
>   	Opt_compress_extension,
> +	Opt_compress_chksum,
>   	Opt_atgc,
>   	Opt_err,
>   };
> @@ -214,6 +215,7 @@ static match_table_t f2fs_tokens = {
>   	{Opt_compress_algorithm, "compress_algorithm=%s"},
>   	{Opt_compress_log_size, "compress_log_size=%u"},
>   	{Opt_compress_extension, "compress_extension=%s"},
> +	{Opt_compress_chksum, "compress_chksum"},
>   	{Opt_atgc, "atgc"},
>   	{Opt_err, NULL},
>   };
> @@ -934,10 +936,14 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
>   			F2FS_OPTION(sbi).compress_ext_cnt++;
>   			kfree(name);
>   			break;
> +		case Opt_compress_chksum:
> +			F2FS_OPTION(sbi).compress_chksum = true;
> +			break;
>   #else
>   		case Opt_compress_algorithm:
>   		case Opt_compress_log_size:
>   		case Opt_compress_extension:
> +		case Opt_compress_chksum:
>   			f2fs_info(sbi, "compression options not supported");
>   			break;
>   #endif
> @@ -1523,6 +1529,9 @@ static inline void f2fs_show_compress_options(struct seq_file *seq,
>   		seq_printf(seq, ",compress_extension=%s",
>   			F2FS_OPTION(sbi).extensions[i]);
>   	}
> +
> +	if (F2FS_OPTION(sbi).compress_chksum)
> +		seq_puts(seq, ",compress_chksum");
>   }
>   
>   static int f2fs_show_options(struct seq_file *seq, struct dentry *root)
> diff --git a/include/linux/f2fs_fs.h b/include/linux/f2fs_fs.h
> index a5dbb57a687f..7dc2a06cf19a 100644
> --- a/include/linux/f2fs_fs.h
> +++ b/include/linux/f2fs_fs.h
> @@ -273,7 +273,7 @@ struct f2fs_inode {
>   			__le64 i_compr_blocks;	/* # of compressed blocks */
>   			__u8 i_compress_algorithm;	/* compress algorithm */
>   			__u8 i_log_cluster_size;	/* log of cluster size */
> -			__le16 i_padding;		/* padding */
> +			__le16 i_compress_flag;		/* compress flag */
>   			__le32 i_extra_end[0];	/* for attribute size calculation */
>   		} __packed;
>   		__le32 i_addr[DEF_ADDRS_PER_INODE];	/* Pointers to data blocks */
> 

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

* Re: [PATCH v4] f2fs: compress: support chksum
  2020-12-09  1:37 ` Chao Yu
@ 2020-12-09  2:49   ` Jaegeuk Kim
  2020-12-09  3:05     ` Chao Yu
  0 siblings, 1 reply; 10+ messages in thread
From: Jaegeuk Kim @ 2020-12-09  2:49 UTC (permalink / raw)
  To: Chao Yu; +Cc: linux-f2fs-devel, linux-kernel, chao

On 12/09, Chao Yu wrote:
> Hello guys,
> 
> Any further comments on compress related patches?
> 
> Jaegeuk, could you please queue these patches in dev-test? let me know
> if there is any problem.

Chao, could you please rebase and post them in a series? I lost the patch order
and got some conflicts when trying to merge.

> 
> On 2020/12/8 11:14, Chao Yu wrote:
> > This patch supports to store chksum value with compressed
> > data, and verify the integrality of compressed data while
> > reading the data.
> > 
> > The feature can be enabled through specifying mount option
> > 'compress_chksum'.
> > 
> > Signed-off-by: Chao Yu <yuchao0@huawei.com>
> > ---
> > v4:
> > - enhance readability
> > - remove WARN_ON_ONCE()
> >   Documentation/filesystems/f2fs.rst |  1 +
> >   fs/f2fs/compress.c                 | 22 ++++++++++++++++++++++
> >   fs/f2fs/f2fs.h                     | 16 ++++++++++++++--
> >   fs/f2fs/inode.c                    |  3 +++
> >   fs/f2fs/super.c                    |  9 +++++++++
> >   include/linux/f2fs_fs.h            |  2 +-
> >   6 files changed, 50 insertions(+), 3 deletions(-)
> > 
> > diff --git a/Documentation/filesystems/f2fs.rst b/Documentation/filesystems/f2fs.rst
> > index b8ee761c9922..985ae7d35066 100644
> > --- a/Documentation/filesystems/f2fs.rst
> > +++ b/Documentation/filesystems/f2fs.rst
> > @@ -260,6 +260,7 @@ compress_extension=%s	 Support adding specified extension, so that f2fs can enab
> >   			 For other files, we can still enable compression via ioctl.
> >   			 Note that, there is one reserved special extension '*', it
> >   			 can be set to enable compression for all files.
> > +compress_chksum		 Support verifying chksum of raw data in compressed cluster.
> >   inlinecrypt		 When possible, encrypt/decrypt the contents of encrypted
> >   			 files using the blk-crypto framework rather than
> >   			 filesystem-layer encryption. This allows the use of
> > diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> > index 14262e0f1cd6..9313c8695855 100644
> > --- a/fs/f2fs/compress.c
> > +++ b/fs/f2fs/compress.c
> > @@ -602,6 +602,7 @@ static int f2fs_compress_pages(struct compress_ctx *cc)
> >   				f2fs_cops[fi->i_compress_algorithm];
> >   	unsigned int max_len, new_nr_cpages;
> >   	struct page **new_cpages;
> > +	u32 chksum = 0;
> >   	int i, ret;
> >   	trace_f2fs_compress_pages_start(cc->inode, cc->cluster_idx,
> > @@ -655,6 +656,11 @@ static int f2fs_compress_pages(struct compress_ctx *cc)
> >   	cc->cbuf->clen = cpu_to_le32(cc->clen);
> > +	if (fi->i_compress_flag & 1 << COMPRESS_CHKSUM)
> > +		chksum = f2fs_crc32(F2FS_I_SB(cc->inode),
> > +					cc->cbuf->cdata, cc->clen);
> > +	cc->cbuf->chksum = cpu_to_le32(chksum);
> > +
> >   	for (i = 0; i < COMPRESS_DATA_RESERVED_SIZE; i++)
> >   		cc->cbuf->reserved[i] = cpu_to_le32(0);
> > @@ -790,6 +796,22 @@ void f2fs_decompress_pages(struct bio *bio, struct page *page, bool verity)
> >   	ret = cops->decompress_pages(dic);
> > +	if (!ret && (fi->i_compress_flag & 1 << COMPRESS_CHKSUM)) {
> > +		u32 provided = le32_to_cpu(dic->cbuf->chksum);
> > +		u32 calculated = f2fs_crc32(sbi, dic->cbuf->cdata, dic->clen);
> > +
> > +		if (provided != calculated) {
> > +			if (!is_inode_flag_set(dic->inode, FI_COMPRESS_CORRUPT)) {
> > +				set_inode_flag(dic->inode, FI_COMPRESS_CORRUPT);
> > +				printk_ratelimited(
> > +					"%sF2FS-fs (%s): checksum invalid, nid = %lu, %x vs %x",
> > +					KERN_INFO, sbi->sb->s_id, dic->inode->i_ino,
> > +					provided, calculated);
> > +			}
> > +			set_sbi_flag(sbi, SBI_NEED_FSCK);
> > +		}
> > +	}
> > +
> >   out_vunmap_cbuf:
> >   	vm_unmap_ram(dic->cbuf, dic->nr_cpages);
> >   out_vunmap_rbuf:
> > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > index 0d25f5ca5618..0b314b2034d8 100644
> > --- a/fs/f2fs/f2fs.h
> > +++ b/fs/f2fs/f2fs.h
> > @@ -147,7 +147,8 @@ struct f2fs_mount_info {
> >   	/* For compression */
> >   	unsigned char compress_algorithm;	/* algorithm type */
> > -	unsigned compress_log_size;		/* cluster log size */
> > +	unsigned char compress_log_size;	/* cluster log size */
> > +	bool compress_chksum;			/* compressed data chksum */
> >   	unsigned char compress_ext_cnt;		/* extension count */
> >   	unsigned char extensions[COMPRESS_EXT_NUM][F2FS_EXTENSION_LEN];	/* extensions */
> >   };
> > @@ -676,6 +677,7 @@ enum {
> >   	FI_ATOMIC_REVOKE_REQUEST, /* request to drop atomic data */
> >   	FI_VERITY_IN_PROGRESS,	/* building fs-verity Merkle tree */
> >   	FI_COMPRESSED_FILE,	/* indicate file's data can be compressed */
> > +	FI_COMPRESS_CORRUPT,	/* indicate compressed cluster is corrupted */
> >   	FI_MMAP_FILE,		/* indicate file was mmapped */
> >   	FI_MAX,			/* max flag, never be used */
> >   };
> > @@ -733,6 +735,7 @@ struct f2fs_inode_info {
> >   	atomic_t i_compr_blocks;		/* # of compressed blocks */
> >   	unsigned char i_compress_algorithm;	/* algorithm type */
> >   	unsigned char i_log_cluster_size;	/* log of cluster size */
> > +	unsigned short i_compress_flag;		/* compress flag */
> >   	unsigned int i_cluster_size;		/* cluster size */
> >   };
> > @@ -1272,9 +1275,15 @@ enum compress_algorithm_type {
> >   	COMPRESS_MAX,
> >   };
> > -#define COMPRESS_DATA_RESERVED_SIZE		5
> > +enum compress_flag {
> > +	COMPRESS_CHKSUM,
> > +	COMPRESS_MAX_FLAG,
> > +};
> > +
> > +#define COMPRESS_DATA_RESERVED_SIZE		4
> >   struct compress_data {
> >   	__le32 clen;			/* compressed data size */
> > +	__le32 chksum;			/* compressed data chksum */
> >   	__le32 reserved[COMPRESS_DATA_RESERVED_SIZE];	/* reserved */
> >   	u8 cdata[];			/* compressed data */
> >   };
> > @@ -3888,6 +3897,9 @@ static inline void set_compress_context(struct inode *inode)
> >   			F2FS_OPTION(sbi).compress_algorithm;
> >   	F2FS_I(inode)->i_log_cluster_size =
> >   			F2FS_OPTION(sbi).compress_log_size;
> > +	F2FS_I(inode)->i_compress_flag =
> > +			F2FS_OPTION(sbi).compress_chksum ?
> > +				1 << COMPRESS_CHKSUM : 0;
> >   	F2FS_I(inode)->i_cluster_size =
> >   			1 << F2FS_I(inode)->i_log_cluster_size;
> >   	F2FS_I(inode)->i_flags |= F2FS_COMPR_FL;
> > diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
> > index 657db2fb6739..349d9cb933ee 100644
> > --- a/fs/f2fs/inode.c
> > +++ b/fs/f2fs/inode.c
> > @@ -456,6 +456,7 @@ static int do_read_inode(struct inode *inode)
> >   					le64_to_cpu(ri->i_compr_blocks));
> >   			fi->i_compress_algorithm = ri->i_compress_algorithm;
> >   			fi->i_log_cluster_size = ri->i_log_cluster_size;
> > +			fi->i_compress_flag = le16_to_cpu(ri->i_compress_flag);
> >   			fi->i_cluster_size = 1 << fi->i_log_cluster_size;
> >   			set_inode_flag(inode, FI_COMPRESSED_FILE);
> >   		}
> > @@ -634,6 +635,8 @@ void f2fs_update_inode(struct inode *inode, struct page *node_page)
> >   					&F2FS_I(inode)->i_compr_blocks));
> >   			ri->i_compress_algorithm =
> >   				F2FS_I(inode)->i_compress_algorithm;
> > +			ri->i_compress_flag =
> > +				cpu_to_le16(F2FS_I(inode)->i_compress_flag);
> >   			ri->i_log_cluster_size =
> >   				F2FS_I(inode)->i_log_cluster_size;
> >   		}
> > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> > index 82baaa89c893..f3d919ee4dee 100644
> > --- a/fs/f2fs/super.c
> > +++ b/fs/f2fs/super.c
> > @@ -146,6 +146,7 @@ enum {
> >   	Opt_compress_algorithm,
> >   	Opt_compress_log_size,
> >   	Opt_compress_extension,
> > +	Opt_compress_chksum,
> >   	Opt_atgc,
> >   	Opt_err,
> >   };
> > @@ -214,6 +215,7 @@ static match_table_t f2fs_tokens = {
> >   	{Opt_compress_algorithm, "compress_algorithm=%s"},
> >   	{Opt_compress_log_size, "compress_log_size=%u"},
> >   	{Opt_compress_extension, "compress_extension=%s"},
> > +	{Opt_compress_chksum, "compress_chksum"},
> >   	{Opt_atgc, "atgc"},
> >   	{Opt_err, NULL},
> >   };
> > @@ -934,10 +936,14 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
> >   			F2FS_OPTION(sbi).compress_ext_cnt++;
> >   			kfree(name);
> >   			break;
> > +		case Opt_compress_chksum:
> > +			F2FS_OPTION(sbi).compress_chksum = true;
> > +			break;
> >   #else
> >   		case Opt_compress_algorithm:
> >   		case Opt_compress_log_size:
> >   		case Opt_compress_extension:
> > +		case Opt_compress_chksum:
> >   			f2fs_info(sbi, "compression options not supported");
> >   			break;
> >   #endif
> > @@ -1523,6 +1529,9 @@ static inline void f2fs_show_compress_options(struct seq_file *seq,
> >   		seq_printf(seq, ",compress_extension=%s",
> >   			F2FS_OPTION(sbi).extensions[i]);
> >   	}
> > +
> > +	if (F2FS_OPTION(sbi).compress_chksum)
> > +		seq_puts(seq, ",compress_chksum");
> >   }
> >   static int f2fs_show_options(struct seq_file *seq, struct dentry *root)
> > diff --git a/include/linux/f2fs_fs.h b/include/linux/f2fs_fs.h
> > index a5dbb57a687f..7dc2a06cf19a 100644
> > --- a/include/linux/f2fs_fs.h
> > +++ b/include/linux/f2fs_fs.h
> > @@ -273,7 +273,7 @@ struct f2fs_inode {
> >   			__le64 i_compr_blocks;	/* # of compressed blocks */
> >   			__u8 i_compress_algorithm;	/* compress algorithm */
> >   			__u8 i_log_cluster_size;	/* log of cluster size */
> > -			__le16 i_padding;		/* padding */
> > +			__le16 i_compress_flag;		/* compress flag */
> >   			__le32 i_extra_end[0];	/* for attribute size calculation */
> >   		} __packed;
> >   		__le32 i_addr[DEF_ADDRS_PER_INODE];	/* Pointers to data blocks */
> > 

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

* Re: [PATCH v4] f2fs: compress: support chksum
  2020-12-09  2:49   ` Jaegeuk Kim
@ 2020-12-09  3:05     ` Chao Yu
  2020-12-09  3:54       ` Jaegeuk Kim
  0 siblings, 1 reply; 10+ messages in thread
From: Chao Yu @ 2020-12-09  3:05 UTC (permalink / raw)
  To: Jaegeuk Kim; +Cc: linux-f2fs-devel, linux-kernel, chao

On 2020/12/9 10:49, Jaegeuk Kim wrote:
> On 12/09, Chao Yu wrote:
>> Hello guys,
>>
>> Any further comments on compress related patches?
>>
>> Jaegeuk, could you please queue these patches in dev-test? let me know
>> if there is any problem.
> 
> Chao, could you please rebase and post them in a series? I lost the patch order
> and got some conflicts when trying to merge.

I can't rebase on your dev branch, because in your branch, there is old version
of "f2fs: compress:support chksum", could you please drop old one and apply last
v4 one in the same place? and push to kernel.org.

Then I can rebase and resend patches.

Thanks,

> 
>>
>> On 2020/12/8 11:14, Chao Yu wrote:
>>> This patch supports to store chksum value with compressed
>>> data, and verify the integrality of compressed data while
>>> reading the data.
>>>
>>> The feature can be enabled through specifying mount option
>>> 'compress_chksum'.
>>>
>>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
>>> ---
>>> v4:
>>> - enhance readability
>>> - remove WARN_ON_ONCE()
>>>    Documentation/filesystems/f2fs.rst |  1 +
>>>    fs/f2fs/compress.c                 | 22 ++++++++++++++++++++++
>>>    fs/f2fs/f2fs.h                     | 16 ++++++++++++++--
>>>    fs/f2fs/inode.c                    |  3 +++
>>>    fs/f2fs/super.c                    |  9 +++++++++
>>>    include/linux/f2fs_fs.h            |  2 +-
>>>    6 files changed, 50 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/Documentation/filesystems/f2fs.rst b/Documentation/filesystems/f2fs.rst
>>> index b8ee761c9922..985ae7d35066 100644
>>> --- a/Documentation/filesystems/f2fs.rst
>>> +++ b/Documentation/filesystems/f2fs.rst
>>> @@ -260,6 +260,7 @@ compress_extension=%s	 Support adding specified extension, so that f2fs can enab
>>>    			 For other files, we can still enable compression via ioctl.
>>>    			 Note that, there is one reserved special extension '*', it
>>>    			 can be set to enable compression for all files.
>>> +compress_chksum		 Support verifying chksum of raw data in compressed cluster.
>>>    inlinecrypt		 When possible, encrypt/decrypt the contents of encrypted
>>>    			 files using the blk-crypto framework rather than
>>>    			 filesystem-layer encryption. This allows the use of
>>> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
>>> index 14262e0f1cd6..9313c8695855 100644
>>> --- a/fs/f2fs/compress.c
>>> +++ b/fs/f2fs/compress.c
>>> @@ -602,6 +602,7 @@ static int f2fs_compress_pages(struct compress_ctx *cc)
>>>    				f2fs_cops[fi->i_compress_algorithm];
>>>    	unsigned int max_len, new_nr_cpages;
>>>    	struct page **new_cpages;
>>> +	u32 chksum = 0;
>>>    	int i, ret;
>>>    	trace_f2fs_compress_pages_start(cc->inode, cc->cluster_idx,
>>> @@ -655,6 +656,11 @@ static int f2fs_compress_pages(struct compress_ctx *cc)
>>>    	cc->cbuf->clen = cpu_to_le32(cc->clen);
>>> +	if (fi->i_compress_flag & 1 << COMPRESS_CHKSUM)
>>> +		chksum = f2fs_crc32(F2FS_I_SB(cc->inode),
>>> +					cc->cbuf->cdata, cc->clen);
>>> +	cc->cbuf->chksum = cpu_to_le32(chksum);
>>> +
>>>    	for (i = 0; i < COMPRESS_DATA_RESERVED_SIZE; i++)
>>>    		cc->cbuf->reserved[i] = cpu_to_le32(0);
>>> @@ -790,6 +796,22 @@ void f2fs_decompress_pages(struct bio *bio, struct page *page, bool verity)
>>>    	ret = cops->decompress_pages(dic);
>>> +	if (!ret && (fi->i_compress_flag & 1 << COMPRESS_CHKSUM)) {
>>> +		u32 provided = le32_to_cpu(dic->cbuf->chksum);
>>> +		u32 calculated = f2fs_crc32(sbi, dic->cbuf->cdata, dic->clen);
>>> +
>>> +		if (provided != calculated) {
>>> +			if (!is_inode_flag_set(dic->inode, FI_COMPRESS_CORRUPT)) {
>>> +				set_inode_flag(dic->inode, FI_COMPRESS_CORRUPT);
>>> +				printk_ratelimited(
>>> +					"%sF2FS-fs (%s): checksum invalid, nid = %lu, %x vs %x",
>>> +					KERN_INFO, sbi->sb->s_id, dic->inode->i_ino,
>>> +					provided, calculated);
>>> +			}
>>> +			set_sbi_flag(sbi, SBI_NEED_FSCK);
>>> +		}
>>> +	}
>>> +
>>>    out_vunmap_cbuf:
>>>    	vm_unmap_ram(dic->cbuf, dic->nr_cpages);
>>>    out_vunmap_rbuf:
>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>>> index 0d25f5ca5618..0b314b2034d8 100644
>>> --- a/fs/f2fs/f2fs.h
>>> +++ b/fs/f2fs/f2fs.h
>>> @@ -147,7 +147,8 @@ struct f2fs_mount_info {
>>>    	/* For compression */
>>>    	unsigned char compress_algorithm;	/* algorithm type */
>>> -	unsigned compress_log_size;		/* cluster log size */
>>> +	unsigned char compress_log_size;	/* cluster log size */
>>> +	bool compress_chksum;			/* compressed data chksum */
>>>    	unsigned char compress_ext_cnt;		/* extension count */
>>>    	unsigned char extensions[COMPRESS_EXT_NUM][F2FS_EXTENSION_LEN];	/* extensions */
>>>    };
>>> @@ -676,6 +677,7 @@ enum {
>>>    	FI_ATOMIC_REVOKE_REQUEST, /* request to drop atomic data */
>>>    	FI_VERITY_IN_PROGRESS,	/* building fs-verity Merkle tree */
>>>    	FI_COMPRESSED_FILE,	/* indicate file's data can be compressed */
>>> +	FI_COMPRESS_CORRUPT,	/* indicate compressed cluster is corrupted */
>>>    	FI_MMAP_FILE,		/* indicate file was mmapped */
>>>    	FI_MAX,			/* max flag, never be used */
>>>    };
>>> @@ -733,6 +735,7 @@ struct f2fs_inode_info {
>>>    	atomic_t i_compr_blocks;		/* # of compressed blocks */
>>>    	unsigned char i_compress_algorithm;	/* algorithm type */
>>>    	unsigned char i_log_cluster_size;	/* log of cluster size */
>>> +	unsigned short i_compress_flag;		/* compress flag */
>>>    	unsigned int i_cluster_size;		/* cluster size */
>>>    };
>>> @@ -1272,9 +1275,15 @@ enum compress_algorithm_type {
>>>    	COMPRESS_MAX,
>>>    };
>>> -#define COMPRESS_DATA_RESERVED_SIZE		5
>>> +enum compress_flag {
>>> +	COMPRESS_CHKSUM,
>>> +	COMPRESS_MAX_FLAG,
>>> +};
>>> +
>>> +#define COMPRESS_DATA_RESERVED_SIZE		4
>>>    struct compress_data {
>>>    	__le32 clen;			/* compressed data size */
>>> +	__le32 chksum;			/* compressed data chksum */
>>>    	__le32 reserved[COMPRESS_DATA_RESERVED_SIZE];	/* reserved */
>>>    	u8 cdata[];			/* compressed data */
>>>    };
>>> @@ -3888,6 +3897,9 @@ static inline void set_compress_context(struct inode *inode)
>>>    			F2FS_OPTION(sbi).compress_algorithm;
>>>    	F2FS_I(inode)->i_log_cluster_size =
>>>    			F2FS_OPTION(sbi).compress_log_size;
>>> +	F2FS_I(inode)->i_compress_flag =
>>> +			F2FS_OPTION(sbi).compress_chksum ?
>>> +				1 << COMPRESS_CHKSUM : 0;
>>>    	F2FS_I(inode)->i_cluster_size =
>>>    			1 << F2FS_I(inode)->i_log_cluster_size;
>>>    	F2FS_I(inode)->i_flags |= F2FS_COMPR_FL;
>>> diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
>>> index 657db2fb6739..349d9cb933ee 100644
>>> --- a/fs/f2fs/inode.c
>>> +++ b/fs/f2fs/inode.c
>>> @@ -456,6 +456,7 @@ static int do_read_inode(struct inode *inode)
>>>    					le64_to_cpu(ri->i_compr_blocks));
>>>    			fi->i_compress_algorithm = ri->i_compress_algorithm;
>>>    			fi->i_log_cluster_size = ri->i_log_cluster_size;
>>> +			fi->i_compress_flag = le16_to_cpu(ri->i_compress_flag);
>>>    			fi->i_cluster_size = 1 << fi->i_log_cluster_size;
>>>    			set_inode_flag(inode, FI_COMPRESSED_FILE);
>>>    		}
>>> @@ -634,6 +635,8 @@ void f2fs_update_inode(struct inode *inode, struct page *node_page)
>>>    					&F2FS_I(inode)->i_compr_blocks));
>>>    			ri->i_compress_algorithm =
>>>    				F2FS_I(inode)->i_compress_algorithm;
>>> +			ri->i_compress_flag =
>>> +				cpu_to_le16(F2FS_I(inode)->i_compress_flag);
>>>    			ri->i_log_cluster_size =
>>>    				F2FS_I(inode)->i_log_cluster_size;
>>>    		}
>>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>>> index 82baaa89c893..f3d919ee4dee 100644
>>> --- a/fs/f2fs/super.c
>>> +++ b/fs/f2fs/super.c
>>> @@ -146,6 +146,7 @@ enum {
>>>    	Opt_compress_algorithm,
>>>    	Opt_compress_log_size,
>>>    	Opt_compress_extension,
>>> +	Opt_compress_chksum,
>>>    	Opt_atgc,
>>>    	Opt_err,
>>>    };
>>> @@ -214,6 +215,7 @@ static match_table_t f2fs_tokens = {
>>>    	{Opt_compress_algorithm, "compress_algorithm=%s"},
>>>    	{Opt_compress_log_size, "compress_log_size=%u"},
>>>    	{Opt_compress_extension, "compress_extension=%s"},
>>> +	{Opt_compress_chksum, "compress_chksum"},
>>>    	{Opt_atgc, "atgc"},
>>>    	{Opt_err, NULL},
>>>    };
>>> @@ -934,10 +936,14 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
>>>    			F2FS_OPTION(sbi).compress_ext_cnt++;
>>>    			kfree(name);
>>>    			break;
>>> +		case Opt_compress_chksum:
>>> +			F2FS_OPTION(sbi).compress_chksum = true;
>>> +			break;
>>>    #else
>>>    		case Opt_compress_algorithm:
>>>    		case Opt_compress_log_size:
>>>    		case Opt_compress_extension:
>>> +		case Opt_compress_chksum:
>>>    			f2fs_info(sbi, "compression options not supported");
>>>    			break;
>>>    #endif
>>> @@ -1523,6 +1529,9 @@ static inline void f2fs_show_compress_options(struct seq_file *seq,
>>>    		seq_printf(seq, ",compress_extension=%s",
>>>    			F2FS_OPTION(sbi).extensions[i]);
>>>    	}
>>> +
>>> +	if (F2FS_OPTION(sbi).compress_chksum)
>>> +		seq_puts(seq, ",compress_chksum");
>>>    }
>>>    static int f2fs_show_options(struct seq_file *seq, struct dentry *root)
>>> diff --git a/include/linux/f2fs_fs.h b/include/linux/f2fs_fs.h
>>> index a5dbb57a687f..7dc2a06cf19a 100644
>>> --- a/include/linux/f2fs_fs.h
>>> +++ b/include/linux/f2fs_fs.h
>>> @@ -273,7 +273,7 @@ struct f2fs_inode {
>>>    			__le64 i_compr_blocks;	/* # of compressed blocks */
>>>    			__u8 i_compress_algorithm;	/* compress algorithm */
>>>    			__u8 i_log_cluster_size;	/* log of cluster size */
>>> -			__le16 i_padding;		/* padding */
>>> +			__le16 i_compress_flag;		/* compress flag */
>>>    			__le32 i_extra_end[0];	/* for attribute size calculation */
>>>    		} __packed;
>>>    		__le32 i_addr[DEF_ADDRS_PER_INODE];	/* Pointers to data blocks */
>>>
> .
> 

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

* Re: [PATCH v4] f2fs: compress: support chksum
  2020-12-09  3:05     ` Chao Yu
@ 2020-12-09  3:54       ` Jaegeuk Kim
  2020-12-09  4:28         ` Chao Yu
  0 siblings, 1 reply; 10+ messages in thread
From: Jaegeuk Kim @ 2020-12-09  3:54 UTC (permalink / raw)
  To: Chao Yu; +Cc: linux-f2fs-devel, linux-kernel, chao

On 12/09, Chao Yu wrote:
> On 2020/12/9 10:49, Jaegeuk Kim wrote:
> > On 12/09, Chao Yu wrote:
> > > Hello guys,
> > > 
> > > Any further comments on compress related patches?
> > > 
> > > Jaegeuk, could you please queue these patches in dev-test? let me know
> > > if there is any problem.
> > 
> > Chao, could you please rebase and post them in a series? I lost the patch order
> > and got some conflicts when trying to merge.
> 
> I can't rebase on your dev branch, because in your branch, there is old version
> of "f2fs: compress:support chksum", could you please drop old one and apply last
> v4 one in the same place? and push to kernel.org.

Ah, could you please write another patch to adjust the new changes?

> 
> Then I can rebase and resend patches.
> 
> Thanks,
> 
> > 
> > > 
> > > On 2020/12/8 11:14, Chao Yu wrote:
> > > > This patch supports to store chksum value with compressed
> > > > data, and verify the integrality of compressed data while
> > > > reading the data.
> > > > 
> > > > The feature can be enabled through specifying mount option
> > > > 'compress_chksum'.
> > > > 
> > > > Signed-off-by: Chao Yu <yuchao0@huawei.com>
> > > > ---
> > > > v4:
> > > > - enhance readability
> > > > - remove WARN_ON_ONCE()
> > > >    Documentation/filesystems/f2fs.rst |  1 +
> > > >    fs/f2fs/compress.c                 | 22 ++++++++++++++++++++++
> > > >    fs/f2fs/f2fs.h                     | 16 ++++++++++++++--
> > > >    fs/f2fs/inode.c                    |  3 +++
> > > >    fs/f2fs/super.c                    |  9 +++++++++
> > > >    include/linux/f2fs_fs.h            |  2 +-
> > > >    6 files changed, 50 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/Documentation/filesystems/f2fs.rst b/Documentation/filesystems/f2fs.rst
> > > > index b8ee761c9922..985ae7d35066 100644
> > > > --- a/Documentation/filesystems/f2fs.rst
> > > > +++ b/Documentation/filesystems/f2fs.rst
> > > > @@ -260,6 +260,7 @@ compress_extension=%s	 Support adding specified extension, so that f2fs can enab
> > > >    			 For other files, we can still enable compression via ioctl.
> > > >    			 Note that, there is one reserved special extension '*', it
> > > >    			 can be set to enable compression for all files.
> > > > +compress_chksum		 Support verifying chksum of raw data in compressed cluster.
> > > >    inlinecrypt		 When possible, encrypt/decrypt the contents of encrypted
> > > >    			 files using the blk-crypto framework rather than
> > > >    			 filesystem-layer encryption. This allows the use of
> > > > diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> > > > index 14262e0f1cd6..9313c8695855 100644
> > > > --- a/fs/f2fs/compress.c
> > > > +++ b/fs/f2fs/compress.c
> > > > @@ -602,6 +602,7 @@ static int f2fs_compress_pages(struct compress_ctx *cc)
> > > >    				f2fs_cops[fi->i_compress_algorithm];
> > > >    	unsigned int max_len, new_nr_cpages;
> > > >    	struct page **new_cpages;
> > > > +	u32 chksum = 0;
> > > >    	int i, ret;
> > > >    	trace_f2fs_compress_pages_start(cc->inode, cc->cluster_idx,
> > > > @@ -655,6 +656,11 @@ static int f2fs_compress_pages(struct compress_ctx *cc)
> > > >    	cc->cbuf->clen = cpu_to_le32(cc->clen);
> > > > +	if (fi->i_compress_flag & 1 << COMPRESS_CHKSUM)
> > > > +		chksum = f2fs_crc32(F2FS_I_SB(cc->inode),
> > > > +					cc->cbuf->cdata, cc->clen);
> > > > +	cc->cbuf->chksum = cpu_to_le32(chksum);
> > > > +
> > > >    	for (i = 0; i < COMPRESS_DATA_RESERVED_SIZE; i++)
> > > >    		cc->cbuf->reserved[i] = cpu_to_le32(0);
> > > > @@ -790,6 +796,22 @@ void f2fs_decompress_pages(struct bio *bio, struct page *page, bool verity)
> > > >    	ret = cops->decompress_pages(dic);
> > > > +	if (!ret && (fi->i_compress_flag & 1 << COMPRESS_CHKSUM)) {
> > > > +		u32 provided = le32_to_cpu(dic->cbuf->chksum);
> > > > +		u32 calculated = f2fs_crc32(sbi, dic->cbuf->cdata, dic->clen);
> > > > +
> > > > +		if (provided != calculated) {
> > > > +			if (!is_inode_flag_set(dic->inode, FI_COMPRESS_CORRUPT)) {
> > > > +				set_inode_flag(dic->inode, FI_COMPRESS_CORRUPT);
> > > > +				printk_ratelimited(
> > > > +					"%sF2FS-fs (%s): checksum invalid, nid = %lu, %x vs %x",
> > > > +					KERN_INFO, sbi->sb->s_id, dic->inode->i_ino,
> > > > +					provided, calculated);
> > > > +			}
> > > > +			set_sbi_flag(sbi, SBI_NEED_FSCK);
> > > > +		}
> > > > +	}
> > > > +
> > > >    out_vunmap_cbuf:
> > > >    	vm_unmap_ram(dic->cbuf, dic->nr_cpages);
> > > >    out_vunmap_rbuf:
> > > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > > > index 0d25f5ca5618..0b314b2034d8 100644
> > > > --- a/fs/f2fs/f2fs.h
> > > > +++ b/fs/f2fs/f2fs.h
> > > > @@ -147,7 +147,8 @@ struct f2fs_mount_info {
> > > >    	/* For compression */
> > > >    	unsigned char compress_algorithm;	/* algorithm type */
> > > > -	unsigned compress_log_size;		/* cluster log size */
> > > > +	unsigned char compress_log_size;	/* cluster log size */
> > > > +	bool compress_chksum;			/* compressed data chksum */
> > > >    	unsigned char compress_ext_cnt;		/* extension count */
> > > >    	unsigned char extensions[COMPRESS_EXT_NUM][F2FS_EXTENSION_LEN];	/* extensions */
> > > >    };
> > > > @@ -676,6 +677,7 @@ enum {
> > > >    	FI_ATOMIC_REVOKE_REQUEST, /* request to drop atomic data */
> > > >    	FI_VERITY_IN_PROGRESS,	/* building fs-verity Merkle tree */
> > > >    	FI_COMPRESSED_FILE,	/* indicate file's data can be compressed */
> > > > +	FI_COMPRESS_CORRUPT,	/* indicate compressed cluster is corrupted */
> > > >    	FI_MMAP_FILE,		/* indicate file was mmapped */
> > > >    	FI_MAX,			/* max flag, never be used */
> > > >    };
> > > > @@ -733,6 +735,7 @@ struct f2fs_inode_info {
> > > >    	atomic_t i_compr_blocks;		/* # of compressed blocks */
> > > >    	unsigned char i_compress_algorithm;	/* algorithm type */
> > > >    	unsigned char i_log_cluster_size;	/* log of cluster size */
> > > > +	unsigned short i_compress_flag;		/* compress flag */
> > > >    	unsigned int i_cluster_size;		/* cluster size */
> > > >    };
> > > > @@ -1272,9 +1275,15 @@ enum compress_algorithm_type {
> > > >    	COMPRESS_MAX,
> > > >    };
> > > > -#define COMPRESS_DATA_RESERVED_SIZE		5
> > > > +enum compress_flag {
> > > > +	COMPRESS_CHKSUM,
> > > > +	COMPRESS_MAX_FLAG,
> > > > +};
> > > > +
> > > > +#define COMPRESS_DATA_RESERVED_SIZE		4
> > > >    struct compress_data {
> > > >    	__le32 clen;			/* compressed data size */
> > > > +	__le32 chksum;			/* compressed data chksum */
> > > >    	__le32 reserved[COMPRESS_DATA_RESERVED_SIZE];	/* reserved */
> > > >    	u8 cdata[];			/* compressed data */
> > > >    };
> > > > @@ -3888,6 +3897,9 @@ static inline void set_compress_context(struct inode *inode)
> > > >    			F2FS_OPTION(sbi).compress_algorithm;
> > > >    	F2FS_I(inode)->i_log_cluster_size =
> > > >    			F2FS_OPTION(sbi).compress_log_size;
> > > > +	F2FS_I(inode)->i_compress_flag =
> > > > +			F2FS_OPTION(sbi).compress_chksum ?
> > > > +				1 << COMPRESS_CHKSUM : 0;
> > > >    	F2FS_I(inode)->i_cluster_size =
> > > >    			1 << F2FS_I(inode)->i_log_cluster_size;
> > > >    	F2FS_I(inode)->i_flags |= F2FS_COMPR_FL;
> > > > diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
> > > > index 657db2fb6739..349d9cb933ee 100644
> > > > --- a/fs/f2fs/inode.c
> > > > +++ b/fs/f2fs/inode.c
> > > > @@ -456,6 +456,7 @@ static int do_read_inode(struct inode *inode)
> > > >    					le64_to_cpu(ri->i_compr_blocks));
> > > >    			fi->i_compress_algorithm = ri->i_compress_algorithm;
> > > >    			fi->i_log_cluster_size = ri->i_log_cluster_size;
> > > > +			fi->i_compress_flag = le16_to_cpu(ri->i_compress_flag);
> > > >    			fi->i_cluster_size = 1 << fi->i_log_cluster_size;
> > > >    			set_inode_flag(inode, FI_COMPRESSED_FILE);
> > > >    		}
> > > > @@ -634,6 +635,8 @@ void f2fs_update_inode(struct inode *inode, struct page *node_page)
> > > >    					&F2FS_I(inode)->i_compr_blocks));
> > > >    			ri->i_compress_algorithm =
> > > >    				F2FS_I(inode)->i_compress_algorithm;
> > > > +			ri->i_compress_flag =
> > > > +				cpu_to_le16(F2FS_I(inode)->i_compress_flag);
> > > >    			ri->i_log_cluster_size =
> > > >    				F2FS_I(inode)->i_log_cluster_size;
> > > >    		}
> > > > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> > > > index 82baaa89c893..f3d919ee4dee 100644
> > > > --- a/fs/f2fs/super.c
> > > > +++ b/fs/f2fs/super.c
> > > > @@ -146,6 +146,7 @@ enum {
> > > >    	Opt_compress_algorithm,
> > > >    	Opt_compress_log_size,
> > > >    	Opt_compress_extension,
> > > > +	Opt_compress_chksum,
> > > >    	Opt_atgc,
> > > >    	Opt_err,
> > > >    };
> > > > @@ -214,6 +215,7 @@ static match_table_t f2fs_tokens = {
> > > >    	{Opt_compress_algorithm, "compress_algorithm=%s"},
> > > >    	{Opt_compress_log_size, "compress_log_size=%u"},
> > > >    	{Opt_compress_extension, "compress_extension=%s"},
> > > > +	{Opt_compress_chksum, "compress_chksum"},
> > > >    	{Opt_atgc, "atgc"},
> > > >    	{Opt_err, NULL},
> > > >    };
> > > > @@ -934,10 +936,14 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
> > > >    			F2FS_OPTION(sbi).compress_ext_cnt++;
> > > >    			kfree(name);
> > > >    			break;
> > > > +		case Opt_compress_chksum:
> > > > +			F2FS_OPTION(sbi).compress_chksum = true;
> > > > +			break;
> > > >    #else
> > > >    		case Opt_compress_algorithm:
> > > >    		case Opt_compress_log_size:
> > > >    		case Opt_compress_extension:
> > > > +		case Opt_compress_chksum:
> > > >    			f2fs_info(sbi, "compression options not supported");
> > > >    			break;
> > > >    #endif
> > > > @@ -1523,6 +1529,9 @@ static inline void f2fs_show_compress_options(struct seq_file *seq,
> > > >    		seq_printf(seq, ",compress_extension=%s",
> > > >    			F2FS_OPTION(sbi).extensions[i]);
> > > >    	}
> > > > +
> > > > +	if (F2FS_OPTION(sbi).compress_chksum)
> > > > +		seq_puts(seq, ",compress_chksum");
> > > >    }
> > > >    static int f2fs_show_options(struct seq_file *seq, struct dentry *root)
> > > > diff --git a/include/linux/f2fs_fs.h b/include/linux/f2fs_fs.h
> > > > index a5dbb57a687f..7dc2a06cf19a 100644
> > > > --- a/include/linux/f2fs_fs.h
> > > > +++ b/include/linux/f2fs_fs.h
> > > > @@ -273,7 +273,7 @@ struct f2fs_inode {
> > > >    			__le64 i_compr_blocks;	/* # of compressed blocks */
> > > >    			__u8 i_compress_algorithm;	/* compress algorithm */
> > > >    			__u8 i_log_cluster_size;	/* log of cluster size */
> > > > -			__le16 i_padding;		/* padding */
> > > > +			__le16 i_compress_flag;		/* compress flag */
> > > >    			__le32 i_extra_end[0];	/* for attribute size calculation */
> > > >    		} __packed;
> > > >    		__le32 i_addr[DEF_ADDRS_PER_INODE];	/* Pointers to data blocks */
> > > > 
> > .
> > 

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

* Re: [PATCH v4] f2fs: compress: support chksum
  2020-12-09  3:54       ` Jaegeuk Kim
@ 2020-12-09  4:28         ` Chao Yu
  2020-12-09  6:29           ` [f2fs-dev] " Chao Yu
  0 siblings, 1 reply; 10+ messages in thread
From: Chao Yu @ 2020-12-09  4:28 UTC (permalink / raw)
  To: Jaegeuk Kim; +Cc: linux-f2fs-devel, linux-kernel, chao

On 2020/12/9 11:54, Jaegeuk Kim wrote:
> On 12/09, Chao Yu wrote:
>> On 2020/12/9 10:49, Jaegeuk Kim wrote:
>>> On 12/09, Chao Yu wrote:
>>>> Hello guys,
>>>>
>>>> Any further comments on compress related patches?
>>>>
>>>> Jaegeuk, could you please queue these patches in dev-test? let me know
>>>> if there is any problem.
>>>
>>> Chao, could you please rebase and post them in a series? I lost the patch order
>>> and got some conflicts when trying to merge.
>>
>> I can't rebase on your dev branch, because in your branch, there is old version
>> of "f2fs: compress:support chksum", could you please drop old one and apply last
>> v4 one in the same place? and push to kernel.org.
> 
> Ah, could you please write another patch to adjust the new changes?

No problem, will drop "f2fs: compress:support chksum" based on your dev branch, and
apply all compress related patches on top of dev branch.

Thanks,

> 
>>
>> Then I can rebase and resend patches.
>>
>> Thanks,
>>
>>>
>>>>
>>>> On 2020/12/8 11:14, Chao Yu wrote:
>>>>> This patch supports to store chksum value with compressed
>>>>> data, and verify the integrality of compressed data while
>>>>> reading the data.
>>>>>
>>>>> The feature can be enabled through specifying mount option
>>>>> 'compress_chksum'.
>>>>>
>>>>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
>>>>> ---
>>>>> v4:
>>>>> - enhance readability
>>>>> - remove WARN_ON_ONCE()
>>>>>     Documentation/filesystems/f2fs.rst |  1 +
>>>>>     fs/f2fs/compress.c                 | 22 ++++++++++++++++++++++
>>>>>     fs/f2fs/f2fs.h                     | 16 ++++++++++++++--
>>>>>     fs/f2fs/inode.c                    |  3 +++
>>>>>     fs/f2fs/super.c                    |  9 +++++++++
>>>>>     include/linux/f2fs_fs.h            |  2 +-
>>>>>     6 files changed, 50 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/Documentation/filesystems/f2fs.rst b/Documentation/filesystems/f2fs.rst
>>>>> index b8ee761c9922..985ae7d35066 100644
>>>>> --- a/Documentation/filesystems/f2fs.rst
>>>>> +++ b/Documentation/filesystems/f2fs.rst
>>>>> @@ -260,6 +260,7 @@ compress_extension=%s	 Support adding specified extension, so that f2fs can enab
>>>>>     			 For other files, we can still enable compression via ioctl.
>>>>>     			 Note that, there is one reserved special extension '*', it
>>>>>     			 can be set to enable compression for all files.
>>>>> +compress_chksum		 Support verifying chksum of raw data in compressed cluster.
>>>>>     inlinecrypt		 When possible, encrypt/decrypt the contents of encrypted
>>>>>     			 files using the blk-crypto framework rather than
>>>>>     			 filesystem-layer encryption. This allows the use of
>>>>> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
>>>>> index 14262e0f1cd6..9313c8695855 100644
>>>>> --- a/fs/f2fs/compress.c
>>>>> +++ b/fs/f2fs/compress.c
>>>>> @@ -602,6 +602,7 @@ static int f2fs_compress_pages(struct compress_ctx *cc)
>>>>>     				f2fs_cops[fi->i_compress_algorithm];
>>>>>     	unsigned int max_len, new_nr_cpages;
>>>>>     	struct page **new_cpages;
>>>>> +	u32 chksum = 0;
>>>>>     	int i, ret;
>>>>>     	trace_f2fs_compress_pages_start(cc->inode, cc->cluster_idx,
>>>>> @@ -655,6 +656,11 @@ static int f2fs_compress_pages(struct compress_ctx *cc)
>>>>>     	cc->cbuf->clen = cpu_to_le32(cc->clen);
>>>>> +	if (fi->i_compress_flag & 1 << COMPRESS_CHKSUM)
>>>>> +		chksum = f2fs_crc32(F2FS_I_SB(cc->inode),
>>>>> +					cc->cbuf->cdata, cc->clen);
>>>>> +	cc->cbuf->chksum = cpu_to_le32(chksum);
>>>>> +
>>>>>     	for (i = 0; i < COMPRESS_DATA_RESERVED_SIZE; i++)
>>>>>     		cc->cbuf->reserved[i] = cpu_to_le32(0);
>>>>> @@ -790,6 +796,22 @@ void f2fs_decompress_pages(struct bio *bio, struct page *page, bool verity)
>>>>>     	ret = cops->decompress_pages(dic);
>>>>> +	if (!ret && (fi->i_compress_flag & 1 << COMPRESS_CHKSUM)) {
>>>>> +		u32 provided = le32_to_cpu(dic->cbuf->chksum);
>>>>> +		u32 calculated = f2fs_crc32(sbi, dic->cbuf->cdata, dic->clen);
>>>>> +
>>>>> +		if (provided != calculated) {
>>>>> +			if (!is_inode_flag_set(dic->inode, FI_COMPRESS_CORRUPT)) {
>>>>> +				set_inode_flag(dic->inode, FI_COMPRESS_CORRUPT);
>>>>> +				printk_ratelimited(
>>>>> +					"%sF2FS-fs (%s): checksum invalid, nid = %lu, %x vs %x",
>>>>> +					KERN_INFO, sbi->sb->s_id, dic->inode->i_ino,
>>>>> +					provided, calculated);
>>>>> +			}
>>>>> +			set_sbi_flag(sbi, SBI_NEED_FSCK);
>>>>> +		}
>>>>> +	}
>>>>> +
>>>>>     out_vunmap_cbuf:
>>>>>     	vm_unmap_ram(dic->cbuf, dic->nr_cpages);
>>>>>     out_vunmap_rbuf:
>>>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>>>>> index 0d25f5ca5618..0b314b2034d8 100644
>>>>> --- a/fs/f2fs/f2fs.h
>>>>> +++ b/fs/f2fs/f2fs.h
>>>>> @@ -147,7 +147,8 @@ struct f2fs_mount_info {
>>>>>     	/* For compression */
>>>>>     	unsigned char compress_algorithm;	/* algorithm type */
>>>>> -	unsigned compress_log_size;		/* cluster log size */
>>>>> +	unsigned char compress_log_size;	/* cluster log size */
>>>>> +	bool compress_chksum;			/* compressed data chksum */
>>>>>     	unsigned char compress_ext_cnt;		/* extension count */
>>>>>     	unsigned char extensions[COMPRESS_EXT_NUM][F2FS_EXTENSION_LEN];	/* extensions */
>>>>>     };
>>>>> @@ -676,6 +677,7 @@ enum {
>>>>>     	FI_ATOMIC_REVOKE_REQUEST, /* request to drop atomic data */
>>>>>     	FI_VERITY_IN_PROGRESS,	/* building fs-verity Merkle tree */
>>>>>     	FI_COMPRESSED_FILE,	/* indicate file's data can be compressed */
>>>>> +	FI_COMPRESS_CORRUPT,	/* indicate compressed cluster is corrupted */
>>>>>     	FI_MMAP_FILE,		/* indicate file was mmapped */
>>>>>     	FI_MAX,			/* max flag, never be used */
>>>>>     };
>>>>> @@ -733,6 +735,7 @@ struct f2fs_inode_info {
>>>>>     	atomic_t i_compr_blocks;		/* # of compressed blocks */
>>>>>     	unsigned char i_compress_algorithm;	/* algorithm type */
>>>>>     	unsigned char i_log_cluster_size;	/* log of cluster size */
>>>>> +	unsigned short i_compress_flag;		/* compress flag */
>>>>>     	unsigned int i_cluster_size;		/* cluster size */
>>>>>     };
>>>>> @@ -1272,9 +1275,15 @@ enum compress_algorithm_type {
>>>>>     	COMPRESS_MAX,
>>>>>     };
>>>>> -#define COMPRESS_DATA_RESERVED_SIZE		5
>>>>> +enum compress_flag {
>>>>> +	COMPRESS_CHKSUM,
>>>>> +	COMPRESS_MAX_FLAG,
>>>>> +};
>>>>> +
>>>>> +#define COMPRESS_DATA_RESERVED_SIZE		4
>>>>>     struct compress_data {
>>>>>     	__le32 clen;			/* compressed data size */
>>>>> +	__le32 chksum;			/* compressed data chksum */
>>>>>     	__le32 reserved[COMPRESS_DATA_RESERVED_SIZE];	/* reserved */
>>>>>     	u8 cdata[];			/* compressed data */
>>>>>     };
>>>>> @@ -3888,6 +3897,9 @@ static inline void set_compress_context(struct inode *inode)
>>>>>     			F2FS_OPTION(sbi).compress_algorithm;
>>>>>     	F2FS_I(inode)->i_log_cluster_size =
>>>>>     			F2FS_OPTION(sbi).compress_log_size;
>>>>> +	F2FS_I(inode)->i_compress_flag =
>>>>> +			F2FS_OPTION(sbi).compress_chksum ?
>>>>> +				1 << COMPRESS_CHKSUM : 0;
>>>>>     	F2FS_I(inode)->i_cluster_size =
>>>>>     			1 << F2FS_I(inode)->i_log_cluster_size;
>>>>>     	F2FS_I(inode)->i_flags |= F2FS_COMPR_FL;
>>>>> diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
>>>>> index 657db2fb6739..349d9cb933ee 100644
>>>>> --- a/fs/f2fs/inode.c
>>>>> +++ b/fs/f2fs/inode.c
>>>>> @@ -456,6 +456,7 @@ static int do_read_inode(struct inode *inode)
>>>>>     					le64_to_cpu(ri->i_compr_blocks));
>>>>>     			fi->i_compress_algorithm = ri->i_compress_algorithm;
>>>>>     			fi->i_log_cluster_size = ri->i_log_cluster_size;
>>>>> +			fi->i_compress_flag = le16_to_cpu(ri->i_compress_flag);
>>>>>     			fi->i_cluster_size = 1 << fi->i_log_cluster_size;
>>>>>     			set_inode_flag(inode, FI_COMPRESSED_FILE);
>>>>>     		}
>>>>> @@ -634,6 +635,8 @@ void f2fs_update_inode(struct inode *inode, struct page *node_page)
>>>>>     					&F2FS_I(inode)->i_compr_blocks));
>>>>>     			ri->i_compress_algorithm =
>>>>>     				F2FS_I(inode)->i_compress_algorithm;
>>>>> +			ri->i_compress_flag =
>>>>> +				cpu_to_le16(F2FS_I(inode)->i_compress_flag);
>>>>>     			ri->i_log_cluster_size =
>>>>>     				F2FS_I(inode)->i_log_cluster_size;
>>>>>     		}
>>>>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>>>>> index 82baaa89c893..f3d919ee4dee 100644
>>>>> --- a/fs/f2fs/super.c
>>>>> +++ b/fs/f2fs/super.c
>>>>> @@ -146,6 +146,7 @@ enum {
>>>>>     	Opt_compress_algorithm,
>>>>>     	Opt_compress_log_size,
>>>>>     	Opt_compress_extension,
>>>>> +	Opt_compress_chksum,
>>>>>     	Opt_atgc,
>>>>>     	Opt_err,
>>>>>     };
>>>>> @@ -214,6 +215,7 @@ static match_table_t f2fs_tokens = {
>>>>>     	{Opt_compress_algorithm, "compress_algorithm=%s"},
>>>>>     	{Opt_compress_log_size, "compress_log_size=%u"},
>>>>>     	{Opt_compress_extension, "compress_extension=%s"},
>>>>> +	{Opt_compress_chksum, "compress_chksum"},
>>>>>     	{Opt_atgc, "atgc"},
>>>>>     	{Opt_err, NULL},
>>>>>     };
>>>>> @@ -934,10 +936,14 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
>>>>>     			F2FS_OPTION(sbi).compress_ext_cnt++;
>>>>>     			kfree(name);
>>>>>     			break;
>>>>> +		case Opt_compress_chksum:
>>>>> +			F2FS_OPTION(sbi).compress_chksum = true;
>>>>> +			break;
>>>>>     #else
>>>>>     		case Opt_compress_algorithm:
>>>>>     		case Opt_compress_log_size:
>>>>>     		case Opt_compress_extension:
>>>>> +		case Opt_compress_chksum:
>>>>>     			f2fs_info(sbi, "compression options not supported");
>>>>>     			break;
>>>>>     #endif
>>>>> @@ -1523,6 +1529,9 @@ static inline void f2fs_show_compress_options(struct seq_file *seq,
>>>>>     		seq_printf(seq, ",compress_extension=%s",
>>>>>     			F2FS_OPTION(sbi).extensions[i]);
>>>>>     	}
>>>>> +
>>>>> +	if (F2FS_OPTION(sbi).compress_chksum)
>>>>> +		seq_puts(seq, ",compress_chksum");
>>>>>     }
>>>>>     static int f2fs_show_options(struct seq_file *seq, struct dentry *root)
>>>>> diff --git a/include/linux/f2fs_fs.h b/include/linux/f2fs_fs.h
>>>>> index a5dbb57a687f..7dc2a06cf19a 100644
>>>>> --- a/include/linux/f2fs_fs.h
>>>>> +++ b/include/linux/f2fs_fs.h
>>>>> @@ -273,7 +273,7 @@ struct f2fs_inode {
>>>>>     			__le64 i_compr_blocks;	/* # of compressed blocks */
>>>>>     			__u8 i_compress_algorithm;	/* compress algorithm */
>>>>>     			__u8 i_log_cluster_size;	/* log of cluster size */
>>>>> -			__le16 i_padding;		/* padding */
>>>>> +			__le16 i_compress_flag;		/* compress flag */
>>>>>     			__le32 i_extra_end[0];	/* for attribute size calculation */
>>>>>     		} __packed;
>>>>>     		__le32 i_addr[DEF_ADDRS_PER_INODE];	/* Pointers to data blocks */
>>>>>
>>> .
>>>
> .
> 

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

* Re: [f2fs-dev] [PATCH v4] f2fs: compress: support chksum
  2020-12-09  4:28         ` Chao Yu
@ 2020-12-09  6:29           ` Chao Yu
  2020-12-09  8:23             ` Jaegeuk Kim
  0 siblings, 1 reply; 10+ messages in thread
From: Chao Yu @ 2020-12-09  6:29 UTC (permalink / raw)
  To: Jaegeuk Kim; +Cc: linux-kernel, linux-f2fs-devel

On 2020/12/9 12:28, Chao Yu wrote:
> On 2020/12/9 11:54, Jaegeuk Kim wrote:
>> Ah, could you please write another patch to adjust the new changes?
> 
> No problem, will drop "f2fs: compress:support chksum" based on your dev branch, and
> apply all compress related patches on top of dev branch.

Jaegeuk, could you please
- drop "f2fs: compress:support chksum",
- manually fix conflict when applying "f2fs: add compress_mode mount option"
- and then apply last my resent patches.

Thanks,

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

* Re: [f2fs-dev] [PATCH v4] f2fs: compress: support chksum
  2020-12-09  6:29           ` [f2fs-dev] " Chao Yu
@ 2020-12-09  8:23             ` Jaegeuk Kim
  2020-12-09  8:31               ` Chao Yu
  0 siblings, 1 reply; 10+ messages in thread
From: Jaegeuk Kim @ 2020-12-09  8:23 UTC (permalink / raw)
  To: Chao Yu; +Cc: linux-kernel, linux-f2fs-devel

On 12/09, Chao Yu wrote:
> On 2020/12/9 12:28, Chao Yu wrote:
> > On 2020/12/9 11:54, Jaegeuk Kim wrote:
> > > Ah, could you please write another patch to adjust the new changes?
> > 
> > No problem, will drop "f2fs: compress:support chksum" based on your dev branch, and
> > apply all compress related patches on top of dev branch.
> 
> Jaegeuk, could you please
> - drop "f2fs: compress:support chksum",

What I mean is keeping the old version in dev branch as is, since it gives
another conflicts when dropping it. That can add another bug at this point.
Can I get a separate patch to fix any issues in that original patch?

> - manually fix conflict when applying "f2fs: add compress_mode mount option"
> - and then apply last my resent patches.
> 
> Thanks,

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

* Re: [f2fs-dev] [PATCH v4] f2fs: compress: support chksum
  2020-12-09  8:23             ` Jaegeuk Kim
@ 2020-12-09  8:31               ` Chao Yu
  2020-12-09  8:42                 ` Chao Yu
  0 siblings, 1 reply; 10+ messages in thread
From: Chao Yu @ 2020-12-09  8:31 UTC (permalink / raw)
  To: Jaegeuk Kim; +Cc: linux-kernel, linux-f2fs-devel

On 2020/12/9 16:23, Jaegeuk Kim wrote:
> On 12/09, Chao Yu wrote:
>> On 2020/12/9 12:28, Chao Yu wrote:
>>> On 2020/12/9 11:54, Jaegeuk Kim wrote:
>>>> Ah, could you please write another patch to adjust the new changes?
>>>
>>> No problem, will drop "f2fs: compress:support chksum" based on your dev branch, and
>>> apply all compress related patches on top of dev branch.
>>
>> Jaegeuk, could you please
>> - drop "f2fs: compress:support chksum",
> 
> What I mean is keeping the old version in dev branch as is, since it gives
> another conflicts when dropping it. That can add another bug at this point.
> Can I get a separate patch to fix any issues in that original patch?

Oops...

Thanks,

> 
>> - manually fix conflict when applying "f2fs: add compress_mode mount option"
>> - and then apply last my resent patches.
>>
>> Thanks,
> .
> 

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

* Re: [f2fs-dev] [PATCH v4] f2fs: compress: support chksum
  2020-12-09  8:31               ` Chao Yu
@ 2020-12-09  8:42                 ` Chao Yu
  0 siblings, 0 replies; 10+ messages in thread
From: Chao Yu @ 2020-12-09  8:42 UTC (permalink / raw)
  To: Jaegeuk Kim; +Cc: linux-kernel, linux-f2fs-devel

On 2020/12/9 16:31, Chao Yu wrote:
> On 2020/12/9 16:23, Jaegeuk Kim wrote:
>> On 12/09, Chao Yu wrote:
>>> On 2020/12/9 12:28, Chao Yu wrote:
>>>> On 2020/12/9 11:54, Jaegeuk Kim wrote:
>>>>> Ah, could you please write another patch to adjust the new changes?
>>>>
>>>> No problem, will drop "f2fs: compress:support chksum" based on your dev branch, and
>>>> apply all compress related patches on top of dev branch.
>>>
>>> Jaegeuk, could you please
>>> - drop "f2fs: compress:support chksum",
>>
>> What I mean is keeping the old version in dev branch as is, since it gives
>> another conflicts when dropping it. That can add another bug at this point.
>> Can I get a separate patch to fix any issues in that original patch?
> 
> Oops...

The diff is as below:

 From 1ad86c640d3a295292960f6b90802cc5a9be7a0d Mon Sep 17 00:00:00 2001
From: Chao Yu <yuchao0@huawei.com>
Date: Wed, 9 Dec 2020 16:36:58 +0800
Subject: [PATCH] fix_chksum

Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
  fs/f2fs/compress.c | 3 +--
  fs/f2fs/compress.h | 0
  2 files changed, 1 insertion(+), 2 deletions(-)
  create mode 100644 fs/f2fs/compress.h

diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
index f05d409fd0ed..4bcbacfe3325 100644
--- a/fs/f2fs/compress.c
+++ b/fs/f2fs/compress.c
@@ -796,7 +796,7 @@ void f2fs_decompress_pages(struct bio *bio, struct page *page, bool verity)

  	ret = cops->decompress_pages(dic);

-	if (!ret && fi->i_compress_flag & 1 << COMPRESS_CHKSUM) {
+	if (!ret && (fi->i_compress_flag & 1 << COMPRESS_CHKSUM)) {
  		u32 provided = le32_to_cpu(dic->cbuf->chksum);
  		u32 calculated = f2fs_crc32(sbi, dic->cbuf->cdata, dic->clen);

@@ -809,7 +809,6 @@ void f2fs_decompress_pages(struct bio *bio, struct page *page, bool verity)
  					provided, calculated);
  			}
  			set_sbi_flag(sbi, SBI_NEED_FSCK);
-			WARN_ON_ONCE(1);
  		}
  	}

diff --git a/fs/f2fs/compress.h b/fs/f2fs/compress.h
new file mode 100644
index 000000000000..e69de29bb2d1
-- 
2.29.2

> 
> Thanks,
> 
>>
>>> - manually fix conflict when applying "f2fs: add compress_mode mount option"
>>> - and then apply last my resent patches.
>>>
>>> Thanks,
>> .
>>
> 
> 
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> .
> 

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

end of thread, other threads:[~2020-12-09  8:43 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-08  3:14 [PATCH v4] f2fs: compress: support chksum Chao Yu
2020-12-09  1:37 ` Chao Yu
2020-12-09  2:49   ` Jaegeuk Kim
2020-12-09  3:05     ` Chao Yu
2020-12-09  3:54       ` Jaegeuk Kim
2020-12-09  4:28         ` Chao Yu
2020-12-09  6:29           ` [f2fs-dev] " Chao Yu
2020-12-09  8:23             ` Jaegeuk Kim
2020-12-09  8:31               ` Chao Yu
2020-12-09  8:42                 ` Chao Yu

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