[PATCH 1/3] erofs: get rid of erofs_prepare_dio() helper

[PATCH 1/3] erofs: get rid of erofs_prepare_dio() helper

[Gao Xiang]

Fold in erofs_prepare_dio() and it seems much clearer.

Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
---
 fs/erofs/data.c | 40 +++++++++++++++-------------------------
 1 file changed, 15 insertions(+), 25 deletions(-)

diff --git a/fs/erofs/data.c b/fs/erofs/data.c
index bb9c1fd48c19..fa9fdc6a5966 100644
--- a/fs/erofs/data.c
+++ b/fs/erofs/data.c
@@ -366,42 +366,32 @@ static sector_t erofs_bmap(struct address_space *mapping, sector_t block)
 	return iomap_bmap(mapping, block, &erofs_iomap_ops);
 }
 
-static int erofs_prepare_dio(struct kiocb *iocb, struct iov_iter *to)
+static ssize_t erofs_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
 {
 	struct inode *inode = file_inode(iocb->ki_filp);
-	loff_t align = iocb->ki_pos | iov_iter_count(to) |
-		iov_iter_alignment(to);
-	struct block_device *bdev = inode->i_sb->s_bdev;
-	unsigned int blksize_mask;
-
-	if (bdev)
-		blksize_mask = (1 << ilog2(bdev_logical_block_size(bdev))) - 1;
-	else
-		blksize_mask = (1 << inode->i_blkbits) - 1;
-
-	if (align & blksize_mask)
-		return -EINVAL;
-	return 0;
-}
 
-static ssize_t erofs_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
-{
 	/* no need taking (shared) inode lock since it's a ro filesystem */
 	if (!iov_iter_count(to))
 		return 0;
 
 #ifdef CONFIG_FS_DAX
-	if (IS_DAX(iocb->ki_filp->f_mapping->host))
+	if (IS_DAX(inode))
 		return dax_iomap_rw(iocb, to, &erofs_iomap_ops);
 #endif
 	if (iocb->ki_flags & IOCB_DIRECT) {
-		int err = erofs_prepare_dio(iocb, to);
-
-		if (!err)
-			return iomap_dio_rw(iocb, to, &erofs_iomap_ops,
-					    NULL, 0, 0);
-		if (err < 0)
-			return err;
+		loff_t align = iocb->ki_pos | iov_iter_count(to) |
+			iov_iter_alignment(to);
+		struct block_device *bdev = inode->i_sb->s_bdev;
+		unsigned int blksize_mask;
+
+		if (bdev)
+			blksize_mask = bdev_logical_block_size(bdev) - 1;
+		else
+			blksize_mask = (1 << inode->i_blkbits) - 1;
+
+		if (align & blksize_mask)
+			return -EINVAL;
+		return iomap_dio_rw(iocb, to, &erofs_iomap_ops, NULL, 0, 0);
 	}
 	return filemap_read(iocb, to, 0);
 }
-- 
2.24.4

[PATCH 1/3] erofs: get rid of erofs_prepare_dio() helper

[Gao Xiang]

Fold in erofs_prepare_dio() and it seems much clearer.

Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
---
 fs/erofs/data.c | 40 +++++++++++++++-------------------------
 1 file changed, 15 insertions(+), 25 deletions(-)

diff --git a/fs/erofs/data.c b/fs/erofs/data.c
index bb9c1fd48c19..fa9fdc6a5966 100644
--- a/fs/erofs/data.c
+++ b/fs/erofs/data.c
@@ -366,42 +366,32 @@ static sector_t erofs_bmap(struct address_space *mapping, sector_t block)
 	return iomap_bmap(mapping, block, &erofs_iomap_ops);
 }
 
-static int erofs_prepare_dio(struct kiocb *iocb, struct iov_iter *to)
+static ssize_t erofs_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
 {
 	struct inode *inode = file_inode(iocb->ki_filp);
-	loff_t align = iocb->ki_pos | iov_iter_count(to) |
-		iov_iter_alignment(to);
-	struct block_device *bdev = inode->i_sb->s_bdev;
-	unsigned int blksize_mask;
-
-	if (bdev)
-		blksize_mask = (1 << ilog2(bdev_logical_block_size(bdev))) - 1;
-	else
-		blksize_mask = (1 << inode->i_blkbits) - 1;
-
-	if (align & blksize_mask)
-		return -EINVAL;
-	return 0;
-}
 
-static ssize_t erofs_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
-{
 	/* no need taking (shared) inode lock since it's a ro filesystem */
 	if (!iov_iter_count(to))
 		return 0;
 
 #ifdef CONFIG_FS_DAX
-	if (IS_DAX(iocb->ki_filp->f_mapping->host))
+	if (IS_DAX(inode))
 		return dax_iomap_rw(iocb, to, &erofs_iomap_ops);
 #endif
 	if (iocb->ki_flags & IOCB_DIRECT) {
-		int err = erofs_prepare_dio(iocb, to);
-
-		if (!err)
-			return iomap_dio_rw(iocb, to, &erofs_iomap_ops,
-					    NULL, 0, 0);
-		if (err < 0)
-			return err;
+		loff_t align = iocb->ki_pos | iov_iter_count(to) |
+			iov_iter_alignment(to);
+		struct block_device *bdev = inode->i_sb->s_bdev;
+		unsigned int blksize_mask;
+
+		if (bdev)
+			blksize_mask = bdev_logical_block_size(bdev) - 1;
+		else
+			blksize_mask = (1 << inode->i_blkbits) - 1;
+
+		if (align & blksize_mask)
+			return -EINVAL;
+		return iomap_dio_rw(iocb, to, &erofs_iomap_ops, NULL, 0, 0);
 	}
 	return filemap_read(iocb, to, 0);
 }
-- 
2.24.4

[PATCH 2/3] erofs: remove obsoluted comments

[Gao Xiang]

Some comments haven't been useful anymore since the code updated.
Let's drop them instead.

Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
---
 fs/erofs/inode.c    |  5 -----
 fs/erofs/internal.h | 25 -------------------------
 2 files changed, 30 deletions(-)

diff --git a/fs/erofs/inode.c b/fs/erofs/inode.c
index 8d3f56c6469b..8b18d57ec18f 100644
--- a/fs/erofs/inode.c
+++ b/fs/erofs/inode.c
@@ -8,11 +8,6 @@
 
 #include <trace/events/erofs.h>
 
-/*
- * if inode is successfully read, return its inode page (or sometimes
- * the inode payload page if it's an extended inode) in order to fill
- * inline data if possible.
- */
 static void *erofs_read_inode(struct erofs_buf *buf,
 			      struct inode *inode, unsigned int *ofs)
 {
diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
index ce2a04836cd2..cfee49d33b95 100644
--- a/fs/erofs/internal.h
+++ b/fs/erofs/internal.h
@@ -397,31 +397,6 @@ extern const struct super_operations erofs_sops;
 extern const struct address_space_operations erofs_raw_access_aops;
 extern const struct address_space_operations z_erofs_aops;
 
-/*
- * Logical to physical block mapping
- *
- * Different with other file systems, it is used for 2 access modes:
- *
- * 1) RAW access mode:
- *
- * Users pass a valid (m_lblk, m_lofs -- usually 0) pair,
- * and get the valid m_pblk, m_pofs and the longest m_len(in bytes).
- *
- * Note that m_lblk in the RAW access mode refers to the number of
- * the compressed ondisk block rather than the uncompressed
- * in-memory block for the compressed file.
- *
- * m_pofs equals to m_lofs except for the inline data page.
- *
- * 2) Normal access mode:
- *
- * If the inode is not compressed, it has no difference with
- * the RAW access mode. However, if the inode is compressed,
- * users should pass a valid (m_lblk, m_lofs) pair, and get
- * the needed m_pblk, m_pofs, m_len to get the compressed data
- * and the updated m_lblk, m_lofs which indicates the start
- * of the corresponding uncompressed data in the file.
- */
 enum {
 	BH_Encoded = BH_PrivateStart,
 	BH_FullMapped,
-- 
2.24.4

[PATCH 2/3] erofs: remove obsoluted comments

[Gao Xiang]

Some comments haven't been useful anymore since the code updated.
Let's drop them instead.

Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
---
 fs/erofs/inode.c    |  5 -----
 fs/erofs/internal.h | 25 -------------------------
 2 files changed, 30 deletions(-)

diff --git a/fs/erofs/inode.c b/fs/erofs/inode.c
index 8d3f56c6469b..8b18d57ec18f 100644
--- a/fs/erofs/inode.c
+++ b/fs/erofs/inode.c
@@ -8,11 +8,6 @@
 
 #include <trace/events/erofs.h>
 
-/*
- * if inode is successfully read, return its inode page (or sometimes
- * the inode payload page if it's an extended inode) in order to fill
- * inline data if possible.
- */
 static void *erofs_read_inode(struct erofs_buf *buf,
 			      struct inode *inode, unsigned int *ofs)
 {
diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
index ce2a04836cd2..cfee49d33b95 100644
--- a/fs/erofs/internal.h
+++ b/fs/erofs/internal.h
@@ -397,31 +397,6 @@ extern const struct super_operations erofs_sops;
 extern const struct address_space_operations erofs_raw_access_aops;
 extern const struct address_space_operations z_erofs_aops;
 
-/*
- * Logical to physical block mapping
- *
- * Different with other file systems, it is used for 2 access modes:
- *
- * 1) RAW access mode:
- *
- * Users pass a valid (m_lblk, m_lofs -- usually 0) pair,
- * and get the valid m_pblk, m_pofs and the longest m_len(in bytes).
- *
- * Note that m_lblk in the RAW access mode refers to the number of
- * the compressed ondisk block rather than the uncompressed
- * in-memory block for the compressed file.
- *
- * m_pofs equals to m_lofs except for the inline data page.
- *
- * 2) Normal access mode:
- *
- * If the inode is not compressed, it has no difference with
- * the RAW access mode. However, if the inode is compressed,
- * users should pass a valid (m_lblk, m_lofs) pair, and get
- * the needed m_pblk, m_pofs, m_len to get the compressed data
- * and the updated m_lblk, m_lofs which indicates the start
- * of the corresponding uncompressed data in the file.
- */
 enum {
 	BH_Encoded = BH_PrivateStart,
 	BH_FullMapped,
-- 
2.24.4

[PATCH 3/3] erofs: refine on-disk definition comments

[Gao Xiang]

Fix some outdated comments and typos, hopefully helpful.

Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
---
 fs/erofs/erofs_fs.h | 37 +++++++++++++++++++------------------
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/fs/erofs/erofs_fs.h b/fs/erofs/erofs_fs.h
index 1238ca104f09..000fa2738974 100644
--- a/fs/erofs/erofs_fs.h
+++ b/fs/erofs/erofs_fs.h
@@ -79,15 +79,15 @@ struct erofs_super_block {
 
 /*
  * erofs inode datalayout (i_format in on-disk inode):
- * 0 - inode plain without inline data A:
+ * 0 - uncompressed flat inode without tail-packing inline data:
  * inode, [xattrs], ... | ... | no-holed data
- * 1 - inode VLE compression B (legacy):
- * inode, [xattrs], extents ... | ...
- * 2 - inode plain with inline data C:
- * inode, [xattrs], last_inline_data, ... | ... | no-holed data
- * 3 - inode compression D:
+ * 1 - compressed inode with non-compact indexes:
+ * inode, [xattrs], [map_header], extents ... | ...
+ * 2 - uncompressed flat inode with tail-packing inline data:
+ * inode, [xattrs], tailpacking data, ... | ... | no-holed data
+ * 3 - compressed inode with compact indexes:
  * inode, [xattrs], map_header, extents ... | ...
- * 4 - inode chunk-based E:
+ * 4 - chunk-based inode with (optional) multi-device support:
  * inode, [xattrs], chunk indexes ... | ...
  * 5~7 - reserved
  */
@@ -106,7 +106,7 @@ static inline bool erofs_inode_is_data_compressed(unsigned int datamode)
 		datamode == EROFS_INODE_FLAT_COMPRESSION_LEGACY;
 }
 
-/* bit definitions of inode i_advise */
+/* bit definitions of inode i_format */
 #define EROFS_I_VERSION_BITS            1
 #define EROFS_I_DATALAYOUT_BITS         3
 
@@ -140,8 +140,9 @@ struct erofs_inode_compact {
 	__le32 i_size;
 	__le32 i_reserved;
 	union {
-		/* file total compressed blocks for data mapping 1 */
+		/* total compressed blocks for compressed inodes */
 		__le32 compressed_blocks;
+		/* block address for uncompressed flat inodes */
 		__le32 raw_blkaddr;
 
 		/* for device files, used to indicate old/new device # */
@@ -156,9 +157,9 @@ struct erofs_inode_compact {
 	__le32 i_reserved2;
 };
 
-/* 32 bytes on-disk inode */
+/* 32-byte on-disk inode */
 #define EROFS_INODE_LAYOUT_COMPACT	0
-/* 64 bytes on-disk inode */
+/* 64-byte on-disk inode */
 #define EROFS_INODE_LAYOUT_EXTENDED	1
 
 /* 64-byte complete form of an ondisk inode */
@@ -171,8 +172,9 @@ struct erofs_inode_extended {
 	__le16 i_reserved;
 	__le64 i_size;
 	union {
-		/* file total compressed blocks for data mapping 1 */
+		/* total compressed blocks for compressed inodes */
 		__le32 compressed_blocks;
+		/* block address for uncompressed flat inodes */
 		__le32 raw_blkaddr;
 
 		/* for device files, used to indicate old/new device # */
@@ -365,17 +367,16 @@ enum {
 
 struct z_erofs_vle_decompressed_index {
 	__le16 di_advise;
-	/* where to decompress in the head cluster */
+	/* where to decompress in the head lcluster */
 	__le16 di_clusterofs;
 
 	union {
-		/* for the head cluster */
+		/* for the HEAD lclusters */
 		__le32 blkaddr;
 		/*
-		 * for the rest clusters
-		 * eg. for 4k page-sized cluster, maximum 4K*64k = 256M)
-		 * [0] - pointing to the head cluster
-		 * [1] - pointing to the tail cluster
+		 * for the NONHEAD lclusters
+		 * [0] - distance to its HEAD lcluster
+		 * [1] - distance to the next HEAD lcluster
 		 */
 		__le16 delta[2];
 	} di_u;
-- 
2.24.4

[PATCH 3/3] erofs: refine on-disk definition comments

[Gao Xiang]

Fix some outdated comments and typos, hopefully helpful.

Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
---
 fs/erofs/erofs_fs.h | 37 +++++++++++++++++++------------------
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/fs/erofs/erofs_fs.h b/fs/erofs/erofs_fs.h
index 1238ca104f09..000fa2738974 100644
--- a/fs/erofs/erofs_fs.h
+++ b/fs/erofs/erofs_fs.h
@@ -79,15 +79,15 @@ struct erofs_super_block {
 
 /*
  * erofs inode datalayout (i_format in on-disk inode):
- * 0 - inode plain without inline data A:
+ * 0 - uncompressed flat inode without tail-packing inline data:
  * inode, [xattrs], ... | ... | no-holed data
- * 1 - inode VLE compression B (legacy):
- * inode, [xattrs], extents ... | ...
- * 2 - inode plain with inline data C:
- * inode, [xattrs], last_inline_data, ... | ... | no-holed data
- * 3 - inode compression D:
+ * 1 - compressed inode with non-compact indexes:
+ * inode, [xattrs], [map_header], extents ... | ...
+ * 2 - uncompressed flat inode with tail-packing inline data:
+ * inode, [xattrs], tailpacking data, ... | ... | no-holed data
+ * 3 - compressed inode with compact indexes:
  * inode, [xattrs], map_header, extents ... | ...
- * 4 - inode chunk-based E:
+ * 4 - chunk-based inode with (optional) multi-device support:
  * inode, [xattrs], chunk indexes ... | ...
  * 5~7 - reserved
  */
@@ -106,7 +106,7 @@ static inline bool erofs_inode_is_data_compressed(unsigned int datamode)
 		datamode == EROFS_INODE_FLAT_COMPRESSION_LEGACY;
 }
 
-/* bit definitions of inode i_advise */
+/* bit definitions of inode i_format */
 #define EROFS_I_VERSION_BITS            1
 #define EROFS_I_DATALAYOUT_BITS         3
 
@@ -140,8 +140,9 @@ struct erofs_inode_compact {
 	__le32 i_size;
 	__le32 i_reserved;
 	union {
-		/* file total compressed blocks for data mapping 1 */
+		/* total compressed blocks for compressed inodes */
 		__le32 compressed_blocks;
+		/* block address for uncompressed flat inodes */
 		__le32 raw_blkaddr;
 
 		/* for device files, used to indicate old/new device # */
@@ -156,9 +157,9 @@ struct erofs_inode_compact {
 	__le32 i_reserved2;
 };
 
-/* 32 bytes on-disk inode */
+/* 32-byte on-disk inode */
 #define EROFS_INODE_LAYOUT_COMPACT	0
-/* 64 bytes on-disk inode */
+/* 64-byte on-disk inode */
 #define EROFS_INODE_LAYOUT_EXTENDED	1
 
 /* 64-byte complete form of an ondisk inode */
@@ -171,8 +172,9 @@ struct erofs_inode_extended {
 	__le16 i_reserved;
 	__le64 i_size;
 	union {
-		/* file total compressed blocks for data mapping 1 */
+		/* total compressed blocks for compressed inodes */
 		__le32 compressed_blocks;
+		/* block address for uncompressed flat inodes */
 		__le32 raw_blkaddr;
 
 		/* for device files, used to indicate old/new device # */
@@ -365,17 +367,16 @@ enum {
 
 struct z_erofs_vle_decompressed_index {
 	__le16 di_advise;
-	/* where to decompress in the head cluster */
+	/* where to decompress in the head lcluster */
 	__le16 di_clusterofs;
 
 	union {
-		/* for the head cluster */
+		/* for the HEAD lclusters */
 		__le32 blkaddr;
 		/*
-		 * for the rest clusters
-		 * eg. for 4k page-sized cluster, maximum 4K*64k = 256M)
-		 * [0] - pointing to the head cluster
-		 * [1] - pointing to the tail cluster
+		 * for the NONHEAD lclusters
+		 * [0] - distance to its HEAD lcluster
+		 * [1] - distance to the next HEAD lcluster
 		 */
 		__le16 delta[2];
 	} di_u;
-- 
2.24.4

Re: [PATCH 2/3] erofs: remove obsoluted comments

[Yue Hu]

On Sat,  7 May 2022 03:46:11 +0800
Gao Xiang <hsiangkao@linux.alibaba.com> wrote:

> Some comments haven't been useful anymore since the code updated.
> Let's drop them instead.
> 
> Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
> ---
>  fs/erofs/inode.c    |  5 -----
>  fs/erofs/internal.h | 25 -------------------------
>  2 files changed, 30 deletions(-)
> 
> diff --git a/fs/erofs/inode.c b/fs/erofs/inode.c
> index 8d3f56c6469b..8b18d57ec18f 100644
> --- a/fs/erofs/inode.c
> +++ b/fs/erofs/inode.c
> @@ -8,11 +8,6 @@
>  
>  #include <trace/events/erofs.h>
>  
> -/*
> - * if inode is successfully read, return its inode page (or sometimes
> - * the inode payload page if it's an extended inode) in order to fill
> - * inline data if possible.
> - */
>  static void *erofs_read_inode(struct erofs_buf *buf,
>  			      struct inode *inode, unsigned int *ofs)
>  {
> diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
> index ce2a04836cd2..cfee49d33b95 100644
> --- a/fs/erofs/internal.h
> +++ b/fs/erofs/internal.h
> @@ -397,31 +397,6 @@ extern const struct super_operations erofs_sops;
>  extern const struct address_space_operations erofs_raw_access_aops;
>  extern const struct address_space_operations z_erofs_aops;
>  
> -/*
> - * Logical to physical block mapping
> - *
> - * Different with other file systems, it is used for 2 access modes:
> - *
> - * 1) RAW access mode:
> - *
> - * Users pass a valid (m_lblk, m_lofs -- usually 0) pair,
> - * and get the valid m_pblk, m_pofs and the longest m_len(in bytes).
> - *
> - * Note that m_lblk in the RAW access mode refers to the number of
> - * the compressed ondisk block rather than the uncompressed
> - * in-memory block for the compressed file.
> - *
> - * m_pofs equals to m_lofs except for the inline data page.
> - *
> - * 2) Normal access mode:
> - *
> - * If the inode is not compressed, it has no difference with
> - * the RAW access mode. However, if the inode is compressed,
> - * users should pass a valid (m_lblk, m_lofs) pair, and get
> - * the needed m_pblk, m_pofs, m_len to get the compressed data
> - * and the updated m_lblk, m_lofs which indicates the start
> - * of the corresponding uncompressed data in the file.
> - */
>  enum {
>  	BH_Encoded = BH_PrivateStart,
>  	BH_FullMapped,

Reviewed-by: Yue Hu <huyue2@coolpad.com>

Re: [PATCH 2/3] erofs: remove obsoluted comments

[Yue Hu]

On Sat,  7 May 2022 03:46:11 +0800
Gao Xiang <hsiangkao@linux.alibaba.com> wrote:

> Some comments haven't been useful anymore since the code updated.
> Let's drop them instead.
> 
> Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
> ---
>  fs/erofs/inode.c    |  5 -----
>  fs/erofs/internal.h | 25 -------------------------
>  2 files changed, 30 deletions(-)
> 
> diff --git a/fs/erofs/inode.c b/fs/erofs/inode.c
> index 8d3f56c6469b..8b18d57ec18f 100644
> --- a/fs/erofs/inode.c
> +++ b/fs/erofs/inode.c
> @@ -8,11 +8,6 @@
>  
>  #include <trace/events/erofs.h>
>  
> -/*
> - * if inode is successfully read, return its inode page (or sometimes
> - * the inode payload page if it's an extended inode) in order to fill
> - * inline data if possible.
> - */
>  static void *erofs_read_inode(struct erofs_buf *buf,
>  			      struct inode *inode, unsigned int *ofs)
>  {
> diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
> index ce2a04836cd2..cfee49d33b95 100644
> --- a/fs/erofs/internal.h
> +++ b/fs/erofs/internal.h
> @@ -397,31 +397,6 @@ extern const struct super_operations erofs_sops;
>  extern const struct address_space_operations erofs_raw_access_aops;
>  extern const struct address_space_operations z_erofs_aops;
>  
> -/*
> - * Logical to physical block mapping
> - *
> - * Different with other file systems, it is used for 2 access modes:
> - *
> - * 1) RAW access mode:
> - *
> - * Users pass a valid (m_lblk, m_lofs -- usually 0) pair,
> - * and get the valid m_pblk, m_pofs and the longest m_len(in bytes).
> - *
> - * Note that m_lblk in the RAW access mode refers to the number of
> - * the compressed ondisk block rather than the uncompressed
> - * in-memory block for the compressed file.
> - *
> - * m_pofs equals to m_lofs except for the inline data page.
> - *
> - * 2) Normal access mode:
> - *
> - * If the inode is not compressed, it has no difference with
> - * the RAW access mode. However, if the inode is compressed,
> - * users should pass a valid (m_lblk, m_lofs) pair, and get
> - * the needed m_pblk, m_pofs, m_len to get the compressed data
> - * and the updated m_lblk, m_lofs which indicates the start
> - * of the corresponding uncompressed data in the file.
> - */
>  enum {
>  	BH_Encoded = BH_PrivateStart,
>  	BH_FullMapped,

Reviewed-by: Yue Hu <huyue2@coolpad.com>

Re: [PATCH 3/3] erofs: refine on-disk definition comments

[Yue Hu]

On Sat,  7 May 2022 03:46:12 +0800
Gao Xiang <hsiangkao@linux.alibaba.com> wrote:

> Fix some outdated comments and typos, hopefully helpful.
> 
> Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
> ---
>  fs/erofs/erofs_fs.h | 37 +++++++++++++++++++------------------
>  1 file changed, 19 insertions(+), 18 deletions(-)
> 
> diff --git a/fs/erofs/erofs_fs.h b/fs/erofs/erofs_fs.h
> index 1238ca104f09..000fa2738974 100644
> --- a/fs/erofs/erofs_fs.h
> +++ b/fs/erofs/erofs_fs.h
> @@ -79,15 +79,15 @@ struct erofs_super_block {
>  
>  /*
>   * erofs inode datalayout (i_format in on-disk inode):
> - * 0 - inode plain without inline data A:
> + * 0 - uncompressed flat inode without tail-packing inline data:
>   * inode, [xattrs], ... | ... | no-holed data
> - * 1 - inode VLE compression B (legacy):
> - * inode, [xattrs], extents ... | ...
> - * 2 - inode plain with inline data C:
> - * inode, [xattrs], last_inline_data, ... | ... | no-holed data
> - * 3 - inode compression D:
> + * 1 - compressed inode with non-compact indexes:
> + * inode, [xattrs], [map_header], extents ... | ...
> + * 2 - uncompressed flat inode with tail-packing inline data:
> + * inode, [xattrs], tailpacking data, ... | ... | no-holed data
> + * 3 - compressed inode with compact indexes:
>   * inode, [xattrs], map_header, extents ... | ...
> - * 4 - inode chunk-based E:
> + * 4 - chunk-based inode with (optional) multi-device support:
>   * inode, [xattrs], chunk indexes ... | ...
>   * 5~7 - reserved
>   */
> @@ -106,7 +106,7 @@ static inline bool erofs_inode_is_data_compressed(unsigned int datamode)
>  		datamode == EROFS_INODE_FLAT_COMPRESSION_LEGACY;
>  }
>  
> -/* bit definitions of inode i_advise */
> +/* bit definitions of inode i_format */
>  #define EROFS_I_VERSION_BITS            1
>  #define EROFS_I_DATALAYOUT_BITS         3
>  
> @@ -140,8 +140,9 @@ struct erofs_inode_compact {
>  	__le32 i_size;
>  	__le32 i_reserved;
>  	union {
> -		/* file total compressed blocks for data mapping 1 */
> +		/* total compressed blocks for compressed inodes */
>  		__le32 compressed_blocks;
> +		/* block address for uncompressed flat inodes */
>  		__le32 raw_blkaddr;
>  
>  		/* for device files, used to indicate old/new device # */
> @@ -156,9 +157,9 @@ struct erofs_inode_compact {
>  	__le32 i_reserved2;
>  };
>  
> -/* 32 bytes on-disk inode */
> +/* 32-byte on-disk inode */
>  #define EROFS_INODE_LAYOUT_COMPACT	0
> -/* 64 bytes on-disk inode */
> +/* 64-byte on-disk inode */
>  #define EROFS_INODE_LAYOUT_EXTENDED	1
>  
>  /* 64-byte complete form of an ondisk inode */
> @@ -171,8 +172,9 @@ struct erofs_inode_extended {
>  	__le16 i_reserved;
>  	__le64 i_size;
>  	union {
> -		/* file total compressed blocks for data mapping 1 */
> +		/* total compressed blocks for compressed inodes */
>  		__le32 compressed_blocks;
> +		/* block address for uncompressed flat inodes */
>  		__le32 raw_blkaddr;
>  
>  		/* for device files, used to indicate old/new device # */
> @@ -365,17 +367,16 @@ enum {
>  
>  struct z_erofs_vle_decompressed_index {
>  	__le16 di_advise;
> -	/* where to decompress in the head cluster */
> +	/* where to decompress in the head lcluster */
>  	__le16 di_clusterofs;
>  
>  	union {
> -		/* for the head cluster */
> +		/* for the HEAD lclusters */
>  		__le32 blkaddr;
>  		/*
> -		 * for the rest clusters
> -		 * eg. for 4k page-sized cluster, maximum 4K*64k = 256M)
> -		 * [0] - pointing to the head cluster
> -		 * [1] - pointing to the tail cluster
> +		 * for the NONHEAD lclusters
> +		 * [0] - distance to its HEAD lcluster
> +		 * [1] - distance to the next HEAD lcluster
>  		 */
>  		__le16 delta[2];
>  	} di_u;

Reviewed-by: Yue Hu <huyue2@coolpad.com>

Re: [PATCH 3/3] erofs: refine on-disk definition comments

[Yue Hu]

On Sat,  7 May 2022 03:46:12 +0800
Gao Xiang <hsiangkao@linux.alibaba.com> wrote:

> Fix some outdated comments and typos, hopefully helpful.
> 
> Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
> ---
>  fs/erofs/erofs_fs.h | 37 +++++++++++++++++++------------------
>  1 file changed, 19 insertions(+), 18 deletions(-)
> 
> diff --git a/fs/erofs/erofs_fs.h b/fs/erofs/erofs_fs.h
> index 1238ca104f09..000fa2738974 100644
> --- a/fs/erofs/erofs_fs.h
> +++ b/fs/erofs/erofs_fs.h
> @@ -79,15 +79,15 @@ struct erofs_super_block {
>  
>  /*
>   * erofs inode datalayout (i_format in on-disk inode):
> - * 0 - inode plain without inline data A:
> + * 0 - uncompressed flat inode without tail-packing inline data:
>   * inode, [xattrs], ... | ... | no-holed data
> - * 1 - inode VLE compression B (legacy):
> - * inode, [xattrs], extents ... | ...
> - * 2 - inode plain with inline data C:
> - * inode, [xattrs], last_inline_data, ... | ... | no-holed data
> - * 3 - inode compression D:
> + * 1 - compressed inode with non-compact indexes:
> + * inode, [xattrs], [map_header], extents ... | ...
> + * 2 - uncompressed flat inode with tail-packing inline data:
> + * inode, [xattrs], tailpacking data, ... | ... | no-holed data
> + * 3 - compressed inode with compact indexes:
>   * inode, [xattrs], map_header, extents ... | ...
> - * 4 - inode chunk-based E:
> + * 4 - chunk-based inode with (optional) multi-device support:
>   * inode, [xattrs], chunk indexes ... | ...
>   * 5~7 - reserved
>   */
> @@ -106,7 +106,7 @@ static inline bool erofs_inode_is_data_compressed(unsigned int datamode)
>  		datamode == EROFS_INODE_FLAT_COMPRESSION_LEGACY;
>  }
>  
> -/* bit definitions of inode i_advise */
> +/* bit definitions of inode i_format */
>  #define EROFS_I_VERSION_BITS            1
>  #define EROFS_I_DATALAYOUT_BITS         3
>  
> @@ -140,8 +140,9 @@ struct erofs_inode_compact {
>  	__le32 i_size;
>  	__le32 i_reserved;
>  	union {
> -		/* file total compressed blocks for data mapping 1 */
> +		/* total compressed blocks for compressed inodes */
>  		__le32 compressed_blocks;
> +		/* block address for uncompressed flat inodes */
>  		__le32 raw_blkaddr;
>  
>  		/* for device files, used to indicate old/new device # */
> @@ -156,9 +157,9 @@ struct erofs_inode_compact {
>  	__le32 i_reserved2;
>  };
>  
> -/* 32 bytes on-disk inode */
> +/* 32-byte on-disk inode */
>  #define EROFS_INODE_LAYOUT_COMPACT	0
> -/* 64 bytes on-disk inode */
> +/* 64-byte on-disk inode */
>  #define EROFS_INODE_LAYOUT_EXTENDED	1
>  
>  /* 64-byte complete form of an ondisk inode */
> @@ -171,8 +172,9 @@ struct erofs_inode_extended {
>  	__le16 i_reserved;
>  	__le64 i_size;
>  	union {
> -		/* file total compressed blocks for data mapping 1 */
> +		/* total compressed blocks for compressed inodes */
>  		__le32 compressed_blocks;
> +		/* block address for uncompressed flat inodes */
>  		__le32 raw_blkaddr;
>  
>  		/* for device files, used to indicate old/new device # */
> @@ -365,17 +367,16 @@ enum {
>  
>  struct z_erofs_vle_decompressed_index {
>  	__le16 di_advise;
> -	/* where to decompress in the head cluster */
> +	/* where to decompress in the head lcluster */
>  	__le16 di_clusterofs;
>  
>  	union {
> -		/* for the head cluster */
> +		/* for the HEAD lclusters */
>  		__le32 blkaddr;
>  		/*
> -		 * for the rest clusters
> -		 * eg. for 4k page-sized cluster, maximum 4K*64k = 256M)
> -		 * [0] - pointing to the head cluster
> -		 * [1] - pointing to the tail cluster
> +		 * for the NONHEAD lclusters
> +		 * [0] - distance to its HEAD lcluster
> +		 * [1] - distance to the next HEAD lcluster
>  		 */
>  		__le16 delta[2];
>  	} di_u;

Reviewed-by: Yue Hu <huyue2@coolpad.com>

Re: [PATCH 2/3] erofs: remove obsoluted comments

[Chao Yu]

On 2022/5/7 3:46, Gao Xiang wrote:
> Some comments haven't been useful anymore since the code updated.
> Let's drop them instead.
> 
> Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>

Reviewed-by: Chao Yu <chao@kernel.org>

Thanks,

Re: [PATCH 3/3] erofs: refine on-disk definition comments

[Chao Yu]

On 2022/5/7 3:46, Gao Xiang wrote:
> Fix some outdated comments and typos, hopefully helpful.
> 
> Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>

Reviewed-by: Chao Yu <chao@kernel.org>

Thanks,