linux-erofs.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/4] erofs: introduce on-disk compression configurations
       [not found] <20210329012308.28743-1-hsiangkao.ref@aol.com>
@ 2021-03-29  1:23 ` Gao Xiang via Linux-erofs
  2021-03-29  1:23   ` [PATCH v2 1/4] erofs: introduce erofs_sb_has_xxx() helpers Gao Xiang via Linux-erofs
                     ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Gao Xiang via Linux-erofs @ 2021-03-29  1:23 UTC (permalink / raw)
  To: linux-erofs, Chao Yu, Chao Yu; +Cc: LKML

From: Gao Xiang <hsiangkao@redhat.com>

Hi folks,

When we provides support for different algorithms or big pcluster, it'd
be necessary to record some configuration in the per-fs basis.

For example, when big pcluster feature for lz4 is enabled, we need to
know the largest pclustersize in the whole fs instance to adjust per-CPU
buffers in advance, which will be used to handle in-place decompression
failure for inplace IO then. And we also need to record global arguments
(e.g. dict_size) for the upcoming LZMA algorithm support.

Therefore, this patchset introduces a new INCOMPAT feature called
COMPR_CFGS, which provides an available algorithm bitmap in the sb
and a variable array list to store corresponding arguments for each
available algorithms.

Since such a INCOMPAT sb feature will be introduced, it'd be better to
reuse such bit for BIGPCLUSTER feature as well since BIGPCLUSTER feature
depends on COMPR_CFGS and will be released together with COMPR_CFGS.

If COMPR_CFGS is disabled, the field of available algorithm bitmap would
become a lz4_max_distance (which is now reserved as 0), and max_distance
can be safely ignored for old kernels since 64k lz4 dictionary is always
enough even new images could reduce the sliding window.

Thanks,
Gao Xiang

Changes since v1:
 - [2/4] add a comment on lz4_max_distance (Chao);
 - [4/4] fix a misplaced label (Chao);
 - [4/4] enable COMPR_CFGS with BIG_PCLUSTER later since they share
         the same bit.

Gao Xiang (3):
  erofs: introduce erofs_sb_has_xxx() helpers
  erofs: introduce on-disk lz4 fs configurations
  erofs: add on-disk compression configurations

Huang Jianan (1):
  erofs: support adjust lz4 history window size

 fs/erofs/decompressor.c |  35 ++++++++--
 fs/erofs/erofs_fs.h     |  20 +++++-
 fs/erofs/internal.h     |  33 +++++++++
 fs/erofs/super.c        | 147 +++++++++++++++++++++++++++++++++++++++-
 4 files changed, 224 insertions(+), 11 deletions(-)

-- 
2.20.1


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

* [PATCH v2 1/4] erofs: introduce erofs_sb_has_xxx() helpers
  2021-03-29  1:23 ` [PATCH v2 0/4] erofs: introduce on-disk compression configurations Gao Xiang via Linux-erofs
@ 2021-03-29  1:23   ` Gao Xiang via Linux-erofs
  2021-03-29  1:23   ` [PATCH v2 2/4] erofs: support adjust lz4 history window size Gao Xiang via Linux-erofs
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 11+ messages in thread
From: Gao Xiang via Linux-erofs @ 2021-03-29  1:23 UTC (permalink / raw)
  To: linux-erofs, Chao Yu, Chao Yu; +Cc: LKML

From: Gao Xiang <hsiangkao@redhat.com>

Introduce erofs_sb_has_xxx() to make long checks short, especially
for later big pcluster & LZMA features.

Reviewed-by: Chao Yu <yuchao0@huawei.com>
Signed-off-by: Gao Xiang <hsiangkao@redhat.com>
---
 fs/erofs/decompressor.c | 3 +--
 fs/erofs/internal.h     | 9 +++++++++
 fs/erofs/super.c        | 2 +-
 3 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/fs/erofs/decompressor.c b/fs/erofs/decompressor.c
index 34e73ff76f89..80e8871aef71 100644
--- a/fs/erofs/decompressor.c
+++ b/fs/erofs/decompressor.c
@@ -124,8 +124,7 @@ static int z_erofs_lz4_decompress(struct z_erofs_decompress_req *rq, u8 *out)
 	support_0padding = false;
 
 	/* decompression inplace is only safe when 0padding is enabled */
-	if (EROFS_SB(rq->sb)->feature_incompat &
-	    EROFS_FEATURE_INCOMPAT_LZ4_0PADDING) {
+	if (erofs_sb_has_lz4_0padding(EROFS_SB(rq->sb))) {
 		support_0padding = true;
 
 		while (!src[inputmargin & ~PAGE_MASK])
diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
index 30e63b73a675..d29fc0c56032 100644
--- a/fs/erofs/internal.h
+++ b/fs/erofs/internal.h
@@ -218,6 +218,15 @@ static inline erofs_off_t iloc(struct erofs_sb_info *sbi, erofs_nid_t nid)
 	return blknr_to_addr(sbi->meta_blkaddr) + (nid << sbi->islotbits);
 }
 
+#define EROFS_FEATURE_FUNCS(name, compat, feature) \
+static inline bool erofs_sb_has_##name(struct erofs_sb_info *sbi) \
+{ \
+	return sbi->feature_##compat & EROFS_FEATURE_##feature; \
+}
+
+EROFS_FEATURE_FUNCS(lz4_0padding, incompat, INCOMPAT_LZ4_0PADDING)
+EROFS_FEATURE_FUNCS(sb_chksum, compat, COMPAT_SB_CHKSUM)
+
 /* atomic flag definitions */
 #define EROFS_I_EA_INITED_BIT	0
 #define EROFS_I_Z_INITED_BIT	1
diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index 0445d09b6331..991b99eaf22a 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -149,7 +149,7 @@ static int erofs_read_superblock(struct super_block *sb)
 	}
 
 	sbi->feature_compat = le32_to_cpu(dsb->feature_compat);
-	if (sbi->feature_compat & EROFS_FEATURE_COMPAT_SB_CHKSUM) {
+	if (erofs_sb_has_sb_chksum(sbi)) {
 		ret = erofs_superblock_csum_verify(sb, data);
 		if (ret)
 			goto out;
-- 
2.20.1


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

* [PATCH v2 2/4] erofs: support adjust lz4 history window size
  2021-03-29  1:23 ` [PATCH v2 0/4] erofs: introduce on-disk compression configurations Gao Xiang via Linux-erofs
  2021-03-29  1:23   ` [PATCH v2 1/4] erofs: introduce erofs_sb_has_xxx() helpers Gao Xiang via Linux-erofs
@ 2021-03-29  1:23   ` Gao Xiang via Linux-erofs
  2021-03-29  1:23   ` [PATCH v2 3/4] erofs: introduce on-disk lz4 fs configurations Gao Xiang via Linux-erofs
  2021-03-29  1:23   ` [PATCH v2 4/4] erofs: add on-disk compression configurations Gao Xiang via Linux-erofs
  3 siblings, 0 replies; 11+ messages in thread
From: Gao Xiang via Linux-erofs @ 2021-03-29  1:23 UTC (permalink / raw)
  To: linux-erofs, Chao Yu, Chao Yu; +Cc: Guo Weichao, LKML

From: Huang Jianan <huangjianan@oppo.com>

lz4 uses LZ4_DISTANCE_MAX to record history preservation. When
using rolling decompression, a block with a higher compression
ratio will cause a larger memory allocation (up to 64k). It may
cause a large resource burden in extreme cases on devices with
small memory and a large number of concurrent IOs. So appropriately
reducing this value can improve performance.

Decreasing this value will reduce the compression ratio (except
when input_size <LZ4_DISTANCE_MAX). But considering that erofs
currently only supports 4k output, reducing this value will not
significantly reduce the compression benefits.

The maximum value of LZ4_DISTANCE_MAX defined by lz4 is 64k, and
we can only reduce this value. For the old kernel, it just can't
reduce the memory allocation during rolling decompression without
affecting the decompression result.

Signed-off-by: Huang Jianan <huangjianan@oppo.com>
Signed-off-by: Guo Weichao <guoweichao@oppo.com>
[ Gao Xiang: introduce struct erofs_sb_lz4_info for configurations. ]
Reviewed-by: Chao Yu <yuchao0@huawei.com>
Signed-off-by: Gao Xiang <hsiangkao@redhat.com>
---
 fs/erofs/decompressor.c | 21 +++++++++++++++++----
 fs/erofs/erofs_fs.h     |  4 +++-
 fs/erofs/internal.h     | 19 +++++++++++++++++++
 fs/erofs/super.c        |  4 +++-
 4 files changed, 42 insertions(+), 6 deletions(-)

diff --git a/fs/erofs/decompressor.c b/fs/erofs/decompressor.c
index 80e8871aef71..93411e9df9b6 100644
--- a/fs/erofs/decompressor.c
+++ b/fs/erofs/decompressor.c
@@ -28,6 +28,17 @@ struct z_erofs_decompressor {
 	char *name;
 };
 
+int z_erofs_load_lz4_config(struct super_block *sb,
+			    struct erofs_super_block *dsb)
+{
+	u16 distance = le16_to_cpu(dsb->lz4_max_distance);
+
+	EROFS_SB(sb)->lz4.max_distance_pages = distance ?
+					DIV_ROUND_UP(distance, PAGE_SIZE) + 1 :
+					LZ4_MAX_DISTANCE_PAGES;
+	return 0;
+}
+
 static int z_erofs_lz4_prepare_destpages(struct z_erofs_decompress_req *rq,
 					 struct list_head *pagepool)
 {
@@ -36,6 +47,8 @@ static int z_erofs_lz4_prepare_destpages(struct z_erofs_decompress_req *rq,
 	struct page *availables[LZ4_MAX_DISTANCE_PAGES] = { NULL };
 	unsigned long bounced[DIV_ROUND_UP(LZ4_MAX_DISTANCE_PAGES,
 					   BITS_PER_LONG)] = { 0 };
+	unsigned int lz4_max_distance_pages =
+				EROFS_SB(rq->sb)->lz4.max_distance_pages;
 	void *kaddr = NULL;
 	unsigned int i, j, top;
 
@@ -44,14 +57,14 @@ static int z_erofs_lz4_prepare_destpages(struct z_erofs_decompress_req *rq,
 		struct page *const page = rq->out[i];
 		struct page *victim;
 
-		if (j >= LZ4_MAX_DISTANCE_PAGES)
+		if (j >= lz4_max_distance_pages)
 			j = 0;
 
 		/* 'valid' bounced can only be tested after a complete round */
 		if (test_bit(j, bounced)) {
-			DBG_BUGON(i < LZ4_MAX_DISTANCE_PAGES);
-			DBG_BUGON(top >= LZ4_MAX_DISTANCE_PAGES);
-			availables[top++] = rq->out[i - LZ4_MAX_DISTANCE_PAGES];
+			DBG_BUGON(i < lz4_max_distance_pages);
+			DBG_BUGON(top >= lz4_max_distance_pages);
+			availables[top++] = rq->out[i - lz4_max_distance_pages];
 		}
 
 		if (page) {
diff --git a/fs/erofs/erofs_fs.h b/fs/erofs/erofs_fs.h
index 9ad1615f4474..43467624ae3b 100644
--- a/fs/erofs/erofs_fs.h
+++ b/fs/erofs/erofs_fs.h
@@ -39,7 +39,9 @@ struct erofs_super_block {
 	__u8 uuid[16];          /* 128-bit uuid for volume */
 	__u8 volume_name[16];   /* volume name */
 	__le32 feature_incompat;
-	__u8 reserved2[44];
+	/* customized lz4 sliding window size instead of 64k by default */
+	__le16 lz4_max_distance;
+	__u8 reserved2[42];
 };
 
 /*
diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
index d29fc0c56032..1de60992c3dd 100644
--- a/fs/erofs/internal.h
+++ b/fs/erofs/internal.h
@@ -59,6 +59,12 @@ struct erofs_fs_context {
 	unsigned int mount_opt;
 };
 
+/* all filesystem-wide lz4 configurations */
+struct erofs_sb_lz4_info {
+	/* # of pages needed for EROFS lz4 rolling decompression */
+	u16 max_distance_pages;
+};
+
 struct erofs_sb_info {
 #ifdef CONFIG_EROFS_FS_ZIP
 	/* list for all registered superblocks, mainly for shrinker */
@@ -72,6 +78,8 @@ struct erofs_sb_info {
 
 	/* pseudo inode to manage cached pages */
 	struct inode *managed_cache;
+
+	struct erofs_sb_lz4_info lz4;
 #endif	/* CONFIG_EROFS_FS_ZIP */
 	u32 blocks;
 	u32 meta_blkaddr;
@@ -432,6 +440,8 @@ int erofs_try_to_free_all_cached_pages(struct erofs_sb_info *sbi,
 				       struct erofs_workgroup *egrp);
 int erofs_try_to_free_cached_page(struct address_space *mapping,
 				  struct page *page);
+int z_erofs_load_lz4_config(struct super_block *sb,
+			    struct erofs_super_block *dsb);
 #else
 static inline void erofs_shrinker_register(struct super_block *sb) {}
 static inline void erofs_shrinker_unregister(struct super_block *sb) {}
@@ -439,6 +449,15 @@ static inline int erofs_init_shrinker(void) { return 0; }
 static inline void erofs_exit_shrinker(void) {}
 static inline int z_erofs_init_zip_subsystem(void) { return 0; }
 static inline void z_erofs_exit_zip_subsystem(void) {}
+static inline int z_erofs_load_lz4_config(struct super_block *sb,
+				struct erofs_super_block *dsb)
+{
+	if (dsb->lz4_max_distance) {
+		erofs_err(sb, "lz4 algorithm isn't enabled");
+		return -EINVAL;
+	}
+	return 0;
+}
 #endif	/* !CONFIG_EROFS_FS_ZIP */
 
 #define EFSCORRUPTED    EUCLEAN         /* Filesystem is corrupted */
diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index 991b99eaf22a..3212e4f73f85 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -187,7 +187,9 @@ static int erofs_read_superblock(struct super_block *sb)
 		ret = -EFSCORRUPTED;
 		goto out;
 	}
-	ret = 0;
+
+	/* parse on-disk compression configurations */
+	ret = z_erofs_load_lz4_config(sb, dsb);
 out:
 	kunmap(page);
 	put_page(page);
-- 
2.20.1


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

* [PATCH v2 3/4] erofs: introduce on-disk lz4 fs configurations
  2021-03-29  1:23 ` [PATCH v2 0/4] erofs: introduce on-disk compression configurations Gao Xiang via Linux-erofs
  2021-03-29  1:23   ` [PATCH v2 1/4] erofs: introduce erofs_sb_has_xxx() helpers Gao Xiang via Linux-erofs
  2021-03-29  1:23   ` [PATCH v2 2/4] erofs: support adjust lz4 history window size Gao Xiang via Linux-erofs
@ 2021-03-29  1:23   ` Gao Xiang via Linux-erofs
  2021-03-29  1:23   ` [PATCH v2 4/4] erofs: add on-disk compression configurations Gao Xiang via Linux-erofs
  3 siblings, 0 replies; 11+ messages in thread
From: Gao Xiang via Linux-erofs @ 2021-03-29  1:23 UTC (permalink / raw)
  To: linux-erofs, Chao Yu, Chao Yu; +Cc: LKML

From: Gao Xiang <hsiangkao@redhat.com>

Introduce z_erofs_lz4_cfgs to store all lz4 configurations.
Currently it's only max_distance, but will be used for new
features later.

Reviewed-by: Chao Yu <yuchao0@huawei.com>
Signed-off-by: Gao Xiang <hsiangkao@redhat.com>
---
 fs/erofs/decompressor.c | 15 +++++++++++++--
 fs/erofs/erofs_fs.h     |  6 ++++++
 fs/erofs/internal.h     |  8 +++++---
 fs/erofs/super.c        |  2 +-
 4 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/fs/erofs/decompressor.c b/fs/erofs/decompressor.c
index 93411e9df9b6..97538ff24a19 100644
--- a/fs/erofs/decompressor.c
+++ b/fs/erofs/decompressor.c
@@ -29,9 +29,20 @@ struct z_erofs_decompressor {
 };
 
 int z_erofs_load_lz4_config(struct super_block *sb,
-			    struct erofs_super_block *dsb)
+			    struct erofs_super_block *dsb,
+			    struct z_erofs_lz4_cfgs *lz4, int size)
 {
-	u16 distance = le16_to_cpu(dsb->lz4_max_distance);
+	u16 distance;
+
+	if (lz4) {
+		if (size < sizeof(struct z_erofs_lz4_cfgs)) {
+			erofs_err(sb, "invalid lz4 cfgs, size=%u", size);
+			return -EINVAL;
+		}
+		distance = le16_to_cpu(lz4->max_distance);
+	} else {
+		distance = le16_to_cpu(dsb->lz4_max_distance);
+	}
 
 	EROFS_SB(sb)->lz4.max_distance_pages = distance ?
 					DIV_ROUND_UP(distance, PAGE_SIZE) + 1 :
diff --git a/fs/erofs/erofs_fs.h b/fs/erofs/erofs_fs.h
index 43467624ae3b..e0f3c0db1f82 100644
--- a/fs/erofs/erofs_fs.h
+++ b/fs/erofs/erofs_fs.h
@@ -197,6 +197,12 @@ enum {
 	Z_EROFS_COMPRESSION_MAX
 };
 
+/* 14 bytes (+ length field = 16 bytes) */
+struct z_erofs_lz4_cfgs {
+	__le16 max_distance;
+	u8 reserved[12];
+} __packed;
+
 /*
  * bit 0 : COMPACTED_2B indexes (0 - off; 1 - on)
  *  e.g. for 4k logical cluster size,      4B        if compacted 2B is off;
diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
index 1de60992c3dd..46b977f348eb 100644
--- a/fs/erofs/internal.h
+++ b/fs/erofs/internal.h
@@ -441,7 +441,8 @@ int erofs_try_to_free_all_cached_pages(struct erofs_sb_info *sbi,
 int erofs_try_to_free_cached_page(struct address_space *mapping,
 				  struct page *page);
 int z_erofs_load_lz4_config(struct super_block *sb,
-			    struct erofs_super_block *dsb);
+			    struct erofs_super_block *dsb,
+			    struct z_erofs_lz4_cfgs *lz4, int len);
 #else
 static inline void erofs_shrinker_register(struct super_block *sb) {}
 static inline void erofs_shrinker_unregister(struct super_block *sb) {}
@@ -450,9 +451,10 @@ static inline void erofs_exit_shrinker(void) {}
 static inline int z_erofs_init_zip_subsystem(void) { return 0; }
 static inline void z_erofs_exit_zip_subsystem(void) {}
 static inline int z_erofs_load_lz4_config(struct super_block *sb,
-				struct erofs_super_block *dsb)
+				  struct erofs_super_block *dsb,
+				  struct z_erofs_lz4_cfgs *lz4, int len)
 {
-	if (dsb->lz4_max_distance) {
+	if (lz4 || dsb->lz4_max_distance) {
 		erofs_err(sb, "lz4 algorithm isn't enabled");
 		return -EINVAL;
 	}
diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index 3212e4f73f85..1ca8da3f2125 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -189,7 +189,7 @@ static int erofs_read_superblock(struct super_block *sb)
 	}
 
 	/* parse on-disk compression configurations */
-	ret = z_erofs_load_lz4_config(sb, dsb);
+	ret = z_erofs_load_lz4_config(sb, dsb, NULL, 0);
 out:
 	kunmap(page);
 	put_page(page);
-- 
2.20.1


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

* [PATCH v2 4/4] erofs: add on-disk compression configurations
  2021-03-29  1:23 ` [PATCH v2 0/4] erofs: introduce on-disk compression configurations Gao Xiang via Linux-erofs
                     ` (2 preceding siblings ...)
  2021-03-29  1:23   ` [PATCH v2 3/4] erofs: introduce on-disk lz4 fs configurations Gao Xiang via Linux-erofs
@ 2021-03-29  1:23   ` Gao Xiang via Linux-erofs
  2021-03-29  6:26     ` Chao Yu
  2021-03-29 10:00     ` [PATCH v3 " Gao Xiang via Linux-erofs
  3 siblings, 2 replies; 11+ messages in thread
From: Gao Xiang via Linux-erofs @ 2021-03-29  1:23 UTC (permalink / raw)
  To: linux-erofs, Chao Yu, Chao Yu; +Cc: LKML

From: Gao Xiang <hsiangkao@redhat.com>

Add a bitmap for available compression algorithms and a variable-sized
on-disk table for compression options in preparation for upcoming big
pcluster and LZMA algorithm, which follows the end of super block.

To parse the compression options, the bitmap is scanned one by one.
For each available algorithm, there is data followed by 2-byte `length'
correspondingly (it's enough for most cases, or entire fs blocks should
be used.)

With such available algorithm bitmap, kernel itself can also refuse to
mount such filesystem if any unsupported compression algorithm exists.

Note that COMPR_CFGS feature will be enabled with BIG_PCLUSTER.

Signed-off-by: Gao Xiang <hsiangkao@redhat.com>
---
 fs/erofs/decompressor.c |   2 +-
 fs/erofs/erofs_fs.h     |  14 ++--
 fs/erofs/internal.h     |   5 +-
 fs/erofs/super.c        | 143 +++++++++++++++++++++++++++++++++++++++-
 4 files changed, 157 insertions(+), 7 deletions(-)

diff --git a/fs/erofs/decompressor.c b/fs/erofs/decompressor.c
index 97538ff24a19..27aa6a99b371 100644
--- a/fs/erofs/decompressor.c
+++ b/fs/erofs/decompressor.c
@@ -41,7 +41,7 @@ int z_erofs_load_lz4_config(struct super_block *sb,
 		}
 		distance = le16_to_cpu(lz4->max_distance);
 	} else {
-		distance = le16_to_cpu(dsb->lz4_max_distance);
+		distance = le16_to_cpu(dsb->u1.lz4_max_distance);
 	}
 
 	EROFS_SB(sb)->lz4.max_distance_pages = distance ?
diff --git a/fs/erofs/erofs_fs.h b/fs/erofs/erofs_fs.h
index e0f3c0db1f82..5a126493d4d9 100644
--- a/fs/erofs/erofs_fs.h
+++ b/fs/erofs/erofs_fs.h
@@ -18,15 +18,16 @@
  * be incompatible with this kernel version.
  */
 #define EROFS_FEATURE_INCOMPAT_LZ4_0PADDING	0x00000001
+#define EROFS_FEATURE_INCOMPAT_COMPR_CFGS	0x00000002
 #define EROFS_ALL_FEATURE_INCOMPAT		EROFS_FEATURE_INCOMPAT_LZ4_0PADDING
 
-/* 128-byte erofs on-disk super block */
+/* erofs on-disk super block (currently 128 bytes) */
 struct erofs_super_block {
 	__le32 magic;           /* file system magic number */
 	__le32 checksum;        /* crc32c(super_block) */
 	__le32 feature_compat;
 	__u8 blkszbits;         /* support block_size == PAGE_SIZE only */
-	__u8 reserved;
+	__u8 sb_extslots;	/* superblock size = 128 + sb_extslots * 16 */
 
 	__le16 root_nid;	/* nid of root directory */
 	__le64 inos;            /* total valid ino # (== f_files - f_favail) */
@@ -39,8 +40,12 @@ struct erofs_super_block {
 	__u8 uuid[16];          /* 128-bit uuid for volume */
 	__u8 volume_name[16];   /* volume name */
 	__le32 feature_incompat;
-	/* customized lz4 sliding window size instead of 64k by default */
-	__le16 lz4_max_distance;
+	union {
+		/* bitmap for available compression algorithms */
+		__le16 available_compr_algs;
+		/* customized sliding window size instead of 64k by default */
+		__le16 lz4_max_distance;
+	} __packed u1;
 	__u8 reserved2[42];
 };
 
@@ -196,6 +201,7 @@ enum {
 	Z_EROFS_COMPRESSION_LZ4	= 0,
 	Z_EROFS_COMPRESSION_MAX
 };
+#define Z_EROFS_ALL_COMPR_ALGS		(1 << (Z_EROFS_COMPRESSION_MAX - 1))
 
 /* 14 bytes (+ length field = 16 bytes) */
 struct z_erofs_lz4_cfgs {
diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
index 46b977f348eb..f3fa895d809f 100644
--- a/fs/erofs/internal.h
+++ b/fs/erofs/internal.h
@@ -75,6 +75,7 @@ struct erofs_sb_info {
 	struct xarray managed_pslots;
 
 	unsigned int shrinker_run_no;
+	u16 available_compr_algs;
 
 	/* pseudo inode to manage cached pages */
 	struct inode *managed_cache;
@@ -90,6 +91,7 @@ struct erofs_sb_info {
 	/* inode slot unit size in bit shift */
 	unsigned char islotbits;
 
+	u32 sb_size;			/* total superblock size */
 	u32 build_time_nsec;
 	u64 build_time;
 
@@ -233,6 +235,7 @@ static inline bool erofs_sb_has_##name(struct erofs_sb_info *sbi) \
 }
 
 EROFS_FEATURE_FUNCS(lz4_0padding, incompat, INCOMPAT_LZ4_0PADDING)
+EROFS_FEATURE_FUNCS(compr_cfgs, incompat, INCOMPAT_COMPR_CFGS)
 EROFS_FEATURE_FUNCS(sb_chksum, compat, COMPAT_SB_CHKSUM)
 
 /* atomic flag definitions */
@@ -454,7 +457,7 @@ static inline int z_erofs_load_lz4_config(struct super_block *sb,
 				  struct erofs_super_block *dsb,
 				  struct z_erofs_lz4_cfgs *lz4, int len)
 {
-	if (lz4 || dsb->lz4_max_distance) {
+	if (lz4 || dsb->u1.lz4_max_distance) {
 		erofs_err(sb, "lz4 algorithm isn't enabled");
 		return -EINVAL;
 	}
diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index 1ca8da3f2125..628c751634fe 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -122,6 +122,138 @@ static bool check_layout_compatibility(struct super_block *sb,
 	return true;
 }
 
+#ifdef CONFIG_EROFS_FS_ZIP
+/* read variable-sized metadata, offset will be aligned by 4-byte */
+static void *erofs_read_metadata(struct super_block *sb, struct page **pagep,
+				 erofs_off_t *offset, int *lengthp)
+{
+	struct page *page = *pagep;
+	u8 *buffer, *ptr;
+	int len, i, cnt;
+	erofs_blk_t blk;
+
+	*offset = round_up(*offset, 4);
+	blk = erofs_blknr(*offset);
+
+	if (!page || page->index != blk) {
+		if (page) {
+			unlock_page(page);
+			put_page(page);
+		}
+		page = erofs_get_meta_page(sb, blk);
+		if (IS_ERR(page))
+			goto err_nullpage;
+	}
+
+	ptr = kmap(page);
+	len = le16_to_cpu(*(__le16 *)&ptr[erofs_blkoff(*offset)]);
+	if (!len)
+		len = U16_MAX + 1;
+	buffer = kmalloc(len, GFP_KERNEL);
+	if (!buffer) {
+		buffer = ERR_PTR(-ENOMEM);
+		goto out;
+	}
+	*offset += sizeof(__le16);
+	*lengthp = len;
+
+	for (i = 0; i < len; i += cnt) {
+		cnt = min(EROFS_BLKSIZ - (int)erofs_blkoff(*offset), len - i);
+		blk = erofs_blknr(*offset);
+
+		if (!page || page->index != blk) {
+			if (page) {
+				kunmap(page);
+				unlock_page(page);
+				put_page(page);
+			}
+			page = erofs_get_meta_page(sb, blk);
+			if (IS_ERR(page)) {
+				kfree(buffer);
+				goto err_nullpage;
+			}
+			ptr = kmap(page);
+		}
+		memcpy(buffer + i, ptr + erofs_blkoff(*offset), cnt);
+		*offset += cnt;
+	}
+out:
+	kunmap(page);
+	*pagep = page;
+	return buffer;
+err_nullpage:
+	*pagep = NULL;
+	return page;
+}
+
+static int erofs_load_compr_cfgs(struct super_block *sb,
+				 struct erofs_super_block *dsb)
+{
+	struct erofs_sb_info *sbi;
+	struct page *page;
+	unsigned int algs, alg;
+	erofs_off_t offset;
+	int size, ret;
+
+	sbi = EROFS_SB(sb);
+	sbi->available_compr_algs = le16_to_cpu(dsb->u1.available_compr_algs);
+
+	if (sbi->available_compr_algs & ~Z_EROFS_ALL_COMPR_ALGS) {
+		erofs_err(sb,
+"try to load compressed image with unsupported algorithms %x",
+			  sbi->available_compr_algs & ~Z_EROFS_ALL_COMPR_ALGS);
+		return -EINVAL;
+	}
+
+	offset = EROFS_SUPER_OFFSET + sbi->sb_size;
+	page = NULL;
+	alg = 0;
+	ret = 0;
+
+	for (algs = sbi->available_compr_algs; algs; algs >>= 1, ++alg) {
+		void *data;
+
+		if (!(algs & 1))
+			continue;
+
+		data = erofs_read_metadata(sb, &page, &offset, &size);
+		if (IS_ERR(data)) {
+			ret = PTR_ERR(data);
+			goto err;
+		}
+
+		switch (alg) {
+		case Z_EROFS_COMPRESSION_LZ4:
+			ret = z_erofs_load_lz4_config(sb, dsb, data, size);
+			break;
+		default:
+			DBG_BUGON(1);
+			ret = -EFAULT;
+		}
+		kfree(data);
+		if (ret)
+			goto err;
+	}
+err:
+	if (page) {
+		unlock_page(page);
+		put_page(page);
+	}
+	return ret;
+}
+#else
+static int erofs_load_compr_cfgs(struct super_block *sb,
+				 struct erofs_super_block *dsb)
+{
+	if (dsb->u1.available_compr_algs) {
+		erofs_err(sb,
+"try to load compressed image when compression is disabled");
+		return -EINVAL;
+	}
+	return 0;
+}
+#endif
+
 static int erofs_read_superblock(struct super_block *sb)
 {
 	struct erofs_sb_info *sbi;
@@ -166,6 +298,12 @@ static int erofs_read_superblock(struct super_block *sb)
 	if (!check_layout_compatibility(sb, dsb))
 		goto out;
 
+	sbi->sb_size = 128 + dsb->sb_extslots * 16;
+	if (sbi->sb_size > EROFS_BLKSIZ) {
+		erofs_err(sb, "invalid sb_extslots %u (more than a fs block)",
+			  sbi->sb_size);
+		goto out;
+	}
 	sbi->blocks = le32_to_cpu(dsb->blocks);
 	sbi->meta_blkaddr = le32_to_cpu(dsb->meta_blkaddr);
 #ifdef CONFIG_EROFS_FS_XATTR
@@ -189,7 +327,10 @@ static int erofs_read_superblock(struct super_block *sb)
 	}
 
 	/* parse on-disk compression configurations */
-	ret = z_erofs_load_lz4_config(sb, dsb, NULL, 0);
+	if (erofs_sb_has_compr_cfgs(sbi))
+		ret = erofs_load_compr_cfgs(sb, dsb);
+	else
+		ret = z_erofs_load_lz4_config(sb, dsb, NULL, 0);
 out:
 	kunmap(page);
 	put_page(page);
-- 
2.20.1


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

* Re: [PATCH v2 4/4] erofs: add on-disk compression configurations
  2021-03-29  1:23   ` [PATCH v2 4/4] erofs: add on-disk compression configurations Gao Xiang via Linux-erofs
@ 2021-03-29  6:26     ` Chao Yu
  2021-03-29  6:36       ` Gao Xiang
  2021-03-29 10:00     ` [PATCH v3 " Gao Xiang via Linux-erofs
  1 sibling, 1 reply; 11+ messages in thread
From: Chao Yu @ 2021-03-29  6:26 UTC (permalink / raw)
  To: Gao Xiang, linux-erofs, Chao Yu; +Cc: LKML

On 2021/3/29 9:23, Gao Xiang wrote:
> From: Gao Xiang <hsiangkao@redhat.com>
> 
> Add a bitmap for available compression algorithms and a variable-sized
> on-disk table for compression options in preparation for upcoming big
> pcluster and LZMA algorithm, which follows the end of super block.
> 
> To parse the compression options, the bitmap is scanned one by one.
> For each available algorithm, there is data followed by 2-byte `length'
> correspondingly (it's enough for most cases, or entire fs blocks should
> be used.)
> 
> With such available algorithm bitmap, kernel itself can also refuse to
> mount such filesystem if any unsupported compression algorithm exists.
> 
> Note that COMPR_CFGS feature will be enabled with BIG_PCLUSTER.
> 
> Signed-off-by: Gao Xiang <hsiangkao@redhat.com>
> ---
>   fs/erofs/decompressor.c |   2 +-
>   fs/erofs/erofs_fs.h     |  14 ++--
>   fs/erofs/internal.h     |   5 +-
>   fs/erofs/super.c        | 143 +++++++++++++++++++++++++++++++++++++++-
>   4 files changed, 157 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/erofs/decompressor.c b/fs/erofs/decompressor.c
> index 97538ff24a19..27aa6a99b371 100644
> --- a/fs/erofs/decompressor.c
> +++ b/fs/erofs/decompressor.c
> @@ -41,7 +41,7 @@ int z_erofs_load_lz4_config(struct super_block *sb,
>   		}
>   		distance = le16_to_cpu(lz4->max_distance);
>   	} else {
> -		distance = le16_to_cpu(dsb->lz4_max_distance);
> +		distance = le16_to_cpu(dsb->u1.lz4_max_distance);
>   	}
>   
>   	EROFS_SB(sb)->lz4.max_distance_pages = distance ?
> diff --git a/fs/erofs/erofs_fs.h b/fs/erofs/erofs_fs.h
> index e0f3c0db1f82..5a126493d4d9 100644
> --- a/fs/erofs/erofs_fs.h
> +++ b/fs/erofs/erofs_fs.h
> @@ -18,15 +18,16 @@
>    * be incompatible with this kernel version.
>    */
>   #define EROFS_FEATURE_INCOMPAT_LZ4_0PADDING	0x00000001
> +#define EROFS_FEATURE_INCOMPAT_COMPR_CFGS	0x00000002
>   #define EROFS_ALL_FEATURE_INCOMPAT		EROFS_FEATURE_INCOMPAT_LZ4_0PADDING
>   
> -/* 128-byte erofs on-disk super block */
> +/* erofs on-disk super block (currently 128 bytes) */
>   struct erofs_super_block {
>   	__le32 magic;           /* file system magic number */
>   	__le32 checksum;        /* crc32c(super_block) */
>   	__le32 feature_compat;
>   	__u8 blkszbits;         /* support block_size == PAGE_SIZE only */
> -	__u8 reserved;
> +	__u8 sb_extslots;	/* superblock size = 128 + sb_extslots * 16 */
>   
>   	__le16 root_nid;	/* nid of root directory */
>   	__le64 inos;            /* total valid ino # (== f_files - f_favail) */
> @@ -39,8 +40,12 @@ struct erofs_super_block {
>   	__u8 uuid[16];          /* 128-bit uuid for volume */
>   	__u8 volume_name[16];   /* volume name */
>   	__le32 feature_incompat;
> -	/* customized lz4 sliding window size instead of 64k by default */
> -	__le16 lz4_max_distance;
> +	union {
> +		/* bitmap for available compression algorithms */
> +		__le16 available_compr_algs;
> +		/* customized sliding window size instead of 64k by default */
> +		__le16 lz4_max_distance;
> +	} __packed u1;
>   	__u8 reserved2[42];
>   };
>   
> @@ -196,6 +201,7 @@ enum {
>   	Z_EROFS_COMPRESSION_LZ4	= 0,
>   	Z_EROFS_COMPRESSION_MAX
>   };
> +#define Z_EROFS_ALL_COMPR_ALGS		(1 << (Z_EROFS_COMPRESSION_MAX - 1))
>   
>   /* 14 bytes (+ length field = 16 bytes) */
>   struct z_erofs_lz4_cfgs {
> diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
> index 46b977f348eb..f3fa895d809f 100644
> --- a/fs/erofs/internal.h
> +++ b/fs/erofs/internal.h
> @@ -75,6 +75,7 @@ struct erofs_sb_info {
>   	struct xarray managed_pslots;
>   
>   	unsigned int shrinker_run_no;
> +	u16 available_compr_algs;
>   
>   	/* pseudo inode to manage cached pages */
>   	struct inode *managed_cache;
> @@ -90,6 +91,7 @@ struct erofs_sb_info {
>   	/* inode slot unit size in bit shift */
>   	unsigned char islotbits;
>   
> +	u32 sb_size;			/* total superblock size */
>   	u32 build_time_nsec;
>   	u64 build_time;
>   
> @@ -233,6 +235,7 @@ static inline bool erofs_sb_has_##name(struct erofs_sb_info *sbi) \
>   }
>   
>   EROFS_FEATURE_FUNCS(lz4_0padding, incompat, INCOMPAT_LZ4_0PADDING)
> +EROFS_FEATURE_FUNCS(compr_cfgs, incompat, INCOMPAT_COMPR_CFGS)
>   EROFS_FEATURE_FUNCS(sb_chksum, compat, COMPAT_SB_CHKSUM)
>   
>   /* atomic flag definitions */
> @@ -454,7 +457,7 @@ static inline int z_erofs_load_lz4_config(struct super_block *sb,
>   				  struct erofs_super_block *dsb,
>   				  struct z_erofs_lz4_cfgs *lz4, int len)
>   {
> -	if (lz4 || dsb->lz4_max_distance) {
> +	if (lz4 || dsb->u1.lz4_max_distance) {
>   		erofs_err(sb, "lz4 algorithm isn't enabled");
>   		return -EINVAL;
>   	}
> diff --git a/fs/erofs/super.c b/fs/erofs/super.c
> index 1ca8da3f2125..628c751634fe 100644
> --- a/fs/erofs/super.c
> +++ b/fs/erofs/super.c
> @@ -122,6 +122,138 @@ static bool check_layout_compatibility(struct super_block *sb,
>   	return true;
>   }
>   
> +#ifdef CONFIG_EROFS_FS_ZIP
> +/* read variable-sized metadata, offset will be aligned by 4-byte */
> +static void *erofs_read_metadata(struct super_block *sb, struct page **pagep,
> +				 erofs_off_t *offset, int *lengthp)
> +{
> +	struct page *page = *pagep;
> +	u8 *buffer, *ptr;
> +	int len, i, cnt;
> +	erofs_blk_t blk;
> +
> +	*offset = round_up(*offset, 4);
> +	blk = erofs_blknr(*offset);
> +
> +	if (!page || page->index != blk) {
> +		if (page) {
> +			unlock_page(page);
> +			put_page(page);
> +		}
> +		page = erofs_get_meta_page(sb, blk);
> +		if (IS_ERR(page))
> +			goto err_nullpage;
> +	}
> +
> +	ptr = kmap(page);
> +	len = le16_to_cpu(*(__le16 *)&ptr[erofs_blkoff(*offset)]);
> +	if (!len)
> +		len = U16_MAX + 1;
> +	buffer = kmalloc(len, GFP_KERNEL);
> +	if (!buffer) {
> +		buffer = ERR_PTR(-ENOMEM);
> +		goto out;
> +	}
> +	*offset += sizeof(__le16);
> +	*lengthp = len;
> +
> +	for (i = 0; i < len; i += cnt) {
> +		cnt = min(EROFS_BLKSIZ - (int)erofs_blkoff(*offset), len - i);
> +		blk = erofs_blknr(*offset);
> +
> +		if (!page || page->index != blk) {
> +			if (page) {
> +				kunmap(page);
> +				unlock_page(page);
> +				put_page(page);
> +			}
> +			page = erofs_get_meta_page(sb, blk);
> +			if (IS_ERR(page)) {
> +				kfree(buffer);
> +				goto err_nullpage;
> +			}
> +			ptr = kmap(page);
> +		}
> +		memcpy(buffer + i, ptr + erofs_blkoff(*offset), cnt);
> +		*offset += cnt;
> +	}
> +out:
> +	kunmap(page);
> +	*pagep = page;
> +	return buffer;
> +err_nullpage:
> +	*pagep = NULL;
> +	return page;
> +}
> +
> +static int erofs_load_compr_cfgs(struct super_block *sb,
> +				 struct erofs_super_block *dsb)
> +{
> +	struct erofs_sb_info *sbi;
> +	struct page *page;
> +	unsigned int algs, alg;
> +	erofs_off_t offset;
> +	int size, ret;
> +
> +	sbi = EROFS_SB(sb);
> +	sbi->available_compr_algs = le16_to_cpu(dsb->u1.available_compr_algs);
> +
> +	if (sbi->available_compr_algs & ~Z_EROFS_ALL_COMPR_ALGS) {
> +		erofs_err(sb,
> +"try to load compressed image with unsupported algorithms %x",

Minor style issue:

			"try to load compressed image with unsupported "
			"algorithms %x",
			sbi->available_compr_algs & ~Z_EROFS_ALL_COMPR_ALGS);

> +			  sbi->available_compr_algs & ~Z_EROFS_ALL_COMPR_ALGS);
> +		return -EINVAL;
> +	}
> +
> +	offset = EROFS_SUPER_OFFSET + sbi->sb_size;
> +	page = NULL;
> +	alg = 0;
> +	ret = 0;
> +
> +	for (algs = sbi->available_compr_algs; algs; algs >>= 1, ++alg) {
> +		void *data;
> +
> +		if (!(algs & 1))
> +			continue;
> +
> +		data = erofs_read_metadata(sb, &page, &offset, &size);
> +		if (IS_ERR(data)) {
> +			ret = PTR_ERR(data);
> +			goto err;
> +		}
> +
> +		switch (alg) {
> +		case Z_EROFS_COMPRESSION_LZ4:
> +			ret = z_erofs_load_lz4_config(sb, dsb, data, size);
> +			break;
> +		default:
> +			DBG_BUGON(1);
> +			ret = -EFAULT;
> +		}
> +		kfree(data);
> +		if (ret)
> +			goto err;
> +	}
> +err:
> +	if (page) {
> +		unlock_page(page);
> +		put_page(page);
> +	}
> +	return ret;
> +}
> +#else
> +static int erofs_load_compr_cfgs(struct super_block *sb,
> +				 struct erofs_super_block *dsb)
> +{
> +	if (dsb->u1.available_compr_algs) {
> +		erofs_err(sb,
> +"try to load compressed image when compression is disabled");

Ditto,
		erofs_err(sb, "try to load compressed image when "
			  "compression is disabled");


> +		return -EINVAL;
> +	}
> +	return 0;
> +}
> +#endif
> +
>   static int erofs_read_superblock(struct super_block *sb)
>   {
>   	struct erofs_sb_info *sbi;
> @@ -166,6 +298,12 @@ static int erofs_read_superblock(struct super_block *sb)
>   	if (!check_layout_compatibility(sb, dsb))
>   		goto out;
>   
> +	sbi->sb_size = 128 + dsb->sb_extslots * 16;

	sbi->sb_size = sizeof(struct erofs_super_block) +
			dsb->sb_extslots * EROFS_EXTSLOT_SIZE;

Otherwise it looks good to me,

Reviewed-by: Chao Yu <yuchao0@huawei.com>

Thanks,

> +	if (sbi->sb_size > EROFS_BLKSIZ) {
> +		erofs_err(sb, "invalid sb_extslots %u (more than a fs block)",
> +			  sbi->sb_size);
> +		goto out;
> +	}
>   	sbi->blocks = le32_to_cpu(dsb->blocks);
>   	sbi->meta_blkaddr = le32_to_cpu(dsb->meta_blkaddr);
>   #ifdef CONFIG_EROFS_FS_XATTR
> @@ -189,7 +327,10 @@ static int erofs_read_superblock(struct super_block *sb)
>   	}
>   
>   	/* parse on-disk compression configurations */
> -	ret = z_erofs_load_lz4_config(sb, dsb, NULL, 0);
> +	if (erofs_sb_has_compr_cfgs(sbi))
> +		ret = erofs_load_compr_cfgs(sb, dsb);
> +	else
> +		ret = z_erofs_load_lz4_config(sb, dsb, NULL, 0);
>   out:
>   	kunmap(page);
>   	put_page(page);
> 

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

* Re: [PATCH v2 4/4] erofs: add on-disk compression configurations
  2021-03-29  6:26     ` Chao Yu
@ 2021-03-29  6:36       ` Gao Xiang
  2021-03-29  6:43         ` Chao Yu
  0 siblings, 1 reply; 11+ messages in thread
From: Gao Xiang @ 2021-03-29  6:36 UTC (permalink / raw)
  To: Chao Yu; +Cc: linux-erofs, LKML

Hi Chao,

On Mon, Mar 29, 2021 at 02:26:05PM +0800, Chao Yu wrote:
> On 2021/3/29 9:23, Gao Xiang wrote:
> > From: Gao Xiang <hsiangkao@redhat.com>
> > 
> > Add a bitmap for available compression algorithms and a variable-sized
> > on-disk table for compression options in preparation for upcoming big
> > pcluster and LZMA algorithm, which follows the end of super block.
> > 
> > To parse the compression options, the bitmap is scanned one by one.
> > For each available algorithm, there is data followed by 2-byte `length'
> > correspondingly (it's enough for most cases, or entire fs blocks should
> > be used.)
> > 
> > With such available algorithm bitmap, kernel itself can also refuse to
> > mount such filesystem if any unsupported compression algorithm exists.
> > 
> > Note that COMPR_CFGS feature will be enabled with BIG_PCLUSTER.
> > 
> > Signed-off-by: Gao Xiang <hsiangkao@redhat.com>
> > ---
> >   fs/erofs/decompressor.c |   2 +-
> >   fs/erofs/erofs_fs.h     |  14 ++--
> >   fs/erofs/internal.h     |   5 +-
> >   fs/erofs/super.c        | 143 +++++++++++++++++++++++++++++++++++++++-
> >   4 files changed, 157 insertions(+), 7 deletions(-)
> > 
> > diff --git a/fs/erofs/decompressor.c b/fs/erofs/decompressor.c
> > index 97538ff24a19..27aa6a99b371 100644
> > --- a/fs/erofs/decompressor.c
> > +++ b/fs/erofs/decompressor.c
> > @@ -41,7 +41,7 @@ int z_erofs_load_lz4_config(struct super_block *sb,
> >   		}
> >   		distance = le16_to_cpu(lz4->max_distance);
> >   	} else {
> > -		distance = le16_to_cpu(dsb->lz4_max_distance);
> > +		distance = le16_to_cpu(dsb->u1.lz4_max_distance);
> >   	}
> >   	EROFS_SB(sb)->lz4.max_distance_pages = distance ?
> > diff --git a/fs/erofs/erofs_fs.h b/fs/erofs/erofs_fs.h
> > index e0f3c0db1f82..5a126493d4d9 100644
> > --- a/fs/erofs/erofs_fs.h
> > +++ b/fs/erofs/erofs_fs.h
> > @@ -18,15 +18,16 @@
> >    * be incompatible with this kernel version.
> >    */
> >   #define EROFS_FEATURE_INCOMPAT_LZ4_0PADDING	0x00000001
> > +#define EROFS_FEATURE_INCOMPAT_COMPR_CFGS	0x00000002
> >   #define EROFS_ALL_FEATURE_INCOMPAT		EROFS_FEATURE_INCOMPAT_LZ4_0PADDING
> > -/* 128-byte erofs on-disk super block */
> > +/* erofs on-disk super block (currently 128 bytes) */
> >   struct erofs_super_block {
> >   	__le32 magic;           /* file system magic number */
> >   	__le32 checksum;        /* crc32c(super_block) */
> >   	__le32 feature_compat;
> >   	__u8 blkszbits;         /* support block_size == PAGE_SIZE only */
> > -	__u8 reserved;
> > +	__u8 sb_extslots;	/* superblock size = 128 + sb_extslots * 16 */
> >   	__le16 root_nid;	/* nid of root directory */
> >   	__le64 inos;            /* total valid ino # (== f_files - f_favail) */
> > @@ -39,8 +40,12 @@ struct erofs_super_block {
> >   	__u8 uuid[16];          /* 128-bit uuid for volume */
> >   	__u8 volume_name[16];   /* volume name */
> >   	__le32 feature_incompat;
> > -	/* customized lz4 sliding window size instead of 64k by default */
> > -	__le16 lz4_max_distance;
> > +	union {
> > +		/* bitmap for available compression algorithms */
> > +		__le16 available_compr_algs;
> > +		/* customized sliding window size instead of 64k by default */
> > +		__le16 lz4_max_distance;
> > +	} __packed u1;
> >   	__u8 reserved2[42];
> >   };
> > @@ -196,6 +201,7 @@ enum {
> >   	Z_EROFS_COMPRESSION_LZ4	= 0,
> >   	Z_EROFS_COMPRESSION_MAX
> >   };
> > +#define Z_EROFS_ALL_COMPR_ALGS		(1 << (Z_EROFS_COMPRESSION_MAX - 1))
> >   /* 14 bytes (+ length field = 16 bytes) */
> >   struct z_erofs_lz4_cfgs {
> > diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
> > index 46b977f348eb..f3fa895d809f 100644
> > --- a/fs/erofs/internal.h
> > +++ b/fs/erofs/internal.h
> > @@ -75,6 +75,7 @@ struct erofs_sb_info {
> >   	struct xarray managed_pslots;
> >   	unsigned int shrinker_run_no;
> > +	u16 available_compr_algs;
> >   	/* pseudo inode to manage cached pages */
> >   	struct inode *managed_cache;
> > @@ -90,6 +91,7 @@ struct erofs_sb_info {
> >   	/* inode slot unit size in bit shift */
> >   	unsigned char islotbits;
> > +	u32 sb_size;			/* total superblock size */
> >   	u32 build_time_nsec;
> >   	u64 build_time;
> > @@ -233,6 +235,7 @@ static inline bool erofs_sb_has_##name(struct erofs_sb_info *sbi) \
> >   }
> >   EROFS_FEATURE_FUNCS(lz4_0padding, incompat, INCOMPAT_LZ4_0PADDING)
> > +EROFS_FEATURE_FUNCS(compr_cfgs, incompat, INCOMPAT_COMPR_CFGS)
> >   EROFS_FEATURE_FUNCS(sb_chksum, compat, COMPAT_SB_CHKSUM)
> >   /* atomic flag definitions */
> > @@ -454,7 +457,7 @@ static inline int z_erofs_load_lz4_config(struct super_block *sb,
> >   				  struct erofs_super_block *dsb,
> >   				  struct z_erofs_lz4_cfgs *lz4, int len)
> >   {
> > -	if (lz4 || dsb->lz4_max_distance) {
> > +	if (lz4 || dsb->u1.lz4_max_distance) {
> >   		erofs_err(sb, "lz4 algorithm isn't enabled");
> >   		return -EINVAL;
> >   	}
> > diff --git a/fs/erofs/super.c b/fs/erofs/super.c
> > index 1ca8da3f2125..628c751634fe 100644
> > --- a/fs/erofs/super.c
> > +++ b/fs/erofs/super.c
> > @@ -122,6 +122,138 @@ static bool check_layout_compatibility(struct super_block *sb,
> >   	return true;
> >   }
> > +#ifdef CONFIG_EROFS_FS_ZIP
> > +/* read variable-sized metadata, offset will be aligned by 4-byte */
> > +static void *erofs_read_metadata(struct super_block *sb, struct page **pagep,
> > +				 erofs_off_t *offset, int *lengthp)
> > +{
> > +	struct page *page = *pagep;
> > +	u8 *buffer, *ptr;
> > +	int len, i, cnt;
> > +	erofs_blk_t blk;
> > +
> > +	*offset = round_up(*offset, 4);
> > +	blk = erofs_blknr(*offset);
> > +
> > +	if (!page || page->index != blk) {
> > +		if (page) {
> > +			unlock_page(page);
> > +			put_page(page);
> > +		}
> > +		page = erofs_get_meta_page(sb, blk);
> > +		if (IS_ERR(page))
> > +			goto err_nullpage;
> > +	}
> > +
> > +	ptr = kmap(page);
> > +	len = le16_to_cpu(*(__le16 *)&ptr[erofs_blkoff(*offset)]);
> > +	if (!len)
> > +		len = U16_MAX + 1;
> > +	buffer = kmalloc(len, GFP_KERNEL);
> > +	if (!buffer) {
> > +		buffer = ERR_PTR(-ENOMEM);
> > +		goto out;
> > +	}
> > +	*offset += sizeof(__le16);
> > +	*lengthp = len;
> > +
> > +	for (i = 0; i < len; i += cnt) {
> > +		cnt = min(EROFS_BLKSIZ - (int)erofs_blkoff(*offset), len - i);
> > +		blk = erofs_blknr(*offset);
> > +
> > +		if (!page || page->index != blk) {
> > +			if (page) {
> > +				kunmap(page);
> > +				unlock_page(page);
> > +				put_page(page);
> > +			}
> > +			page = erofs_get_meta_page(sb, blk);
> > +			if (IS_ERR(page)) {
> > +				kfree(buffer);
> > +				goto err_nullpage;
> > +			}
> > +			ptr = kmap(page);
> > +		}
> > +		memcpy(buffer + i, ptr + erofs_blkoff(*offset), cnt);
> > +		*offset += cnt;
> > +	}
> > +out:
> > +	kunmap(page);
> > +	*pagep = page;
> > +	return buffer;
> > +err_nullpage:
> > +	*pagep = NULL;
> > +	return page;
> > +}
> > +
> > +static int erofs_load_compr_cfgs(struct super_block *sb,
> > +				 struct erofs_super_block *dsb)
> > +{
> > +	struct erofs_sb_info *sbi;
> > +	struct page *page;
> > +	unsigned int algs, alg;
> > +	erofs_off_t offset;
> > +	int size, ret;
> > +
> > +	sbi = EROFS_SB(sb);
> > +	sbi->available_compr_algs = le16_to_cpu(dsb->u1.available_compr_algs);
> > +
> > +	if (sbi->available_compr_algs & ~Z_EROFS_ALL_COMPR_ALGS) {
> > +		erofs_err(sb,
> > +"try to load compressed image with unsupported algorithms %x",
> 
> Minor style issue:
> 
> 			"try to load compressed image with unsupported "
> 			"algorithms %x",
> 			sbi->available_compr_algs & ~Z_EROFS_ALL_COMPR_ALGS);

If I remembered correctly, kernel code style suggests "don't break the print
message, it'd more easy to grep". Actually such style above is from XFS, and
can pass checkpatch.pl check, see:

(fs/xfs/xfs_log_recover.c) xlog_recover():
		/*
		 * Version 5 superblock log feature mask validation. We know the
		 * log is dirty so check if there are any unknown log features
		 * in what we need to recover. If there are unknown features
		 * (e.g. unsupported transactions, then simply reject the
		 * attempt at recovery before touching anything.
		 */
		if (XFS_SB_VERSION_NUM(&log->l_mp->m_sb) == XFS_SB_VERSION_5 &&
		    xfs_sb_has_incompat_log_feature(&log->l_mp->m_sb,
					XFS_SB_FEAT_INCOMPAT_LOG_UNKNOWN)) {
			xfs_warn(log->l_mp,
"Superblock has unknown incompatible log features (0x%x) enabled.",
				(log->l_mp->m_sb.sb_features_log_incompat &
					XFS_SB_FEAT_INCOMPAT_LOG_UNKNOWN));
			xfs_warn(log->l_mp,
"The log can not be fully and/or safely recovered by this kernel.");
			xfs_warn(log->l_mp,
"Please recover the log on a kernel that supports the unknown features.");
			return -EINVAL;
		}

If that does't look ok for us, I could use > 80 line for this instead,
but I tend to not break the message ..

> 
> > +			  sbi->available_compr_algs & ~Z_EROFS_ALL_COMPR_ALGS);
> > +		return -EINVAL;
> > +	}
> > +
> > +	offset = EROFS_SUPER_OFFSET + sbi->sb_size;
> > +	page = NULL;
> > +	alg = 0;
> > +	ret = 0;
> > +
> > +	for (algs = sbi->available_compr_algs; algs; algs >>= 1, ++alg) {
> > +		void *data;
> > +
> > +		if (!(algs & 1))
> > +			continue;
> > +
> > +		data = erofs_read_metadata(sb, &page, &offset, &size);
> > +		if (IS_ERR(data)) {
> > +			ret = PTR_ERR(data);
> > +			goto err;
> > +		}
> > +
> > +		switch (alg) {
> > +		case Z_EROFS_COMPRESSION_LZ4:
> > +			ret = z_erofs_load_lz4_config(sb, dsb, data, size);
> > +			break;
> > +		default:
> > +			DBG_BUGON(1);
> > +			ret = -EFAULT;
> > +		}
> > +		kfree(data);
> > +		if (ret)
> > +			goto err;
> > +	}
> > +err:
> > +	if (page) {
> > +		unlock_page(page);
> > +		put_page(page);
> > +	}
> > +	return ret;
> > +}
> > +#else
> > +static int erofs_load_compr_cfgs(struct super_block *sb,
> > +				 struct erofs_super_block *dsb)
> > +{
> > +	if (dsb->u1.available_compr_algs) {
> > +		erofs_err(sb,
> > +"try to load compressed image when compression is disabled");
> 
> Ditto,
> 		erofs_err(sb, "try to load compressed image when "
> 			  "compression is disabled");
> 

The same above....

> 
> > +		return -EINVAL;
> > +	}
> > +	return 0;
> > +}
> > +#endif
> > +
> >   static int erofs_read_superblock(struct super_block *sb)
> >   {
> >   	struct erofs_sb_info *sbi;
> > @@ -166,6 +298,12 @@ static int erofs_read_superblock(struct super_block *sb)
> >   	if (!check_layout_compatibility(sb, dsb))
> >   		goto out;
> > +	sbi->sb_size = 128 + dsb->sb_extslots * 16;
> 
> 	sbi->sb_size = sizeof(struct erofs_super_block) +
> 			dsb->sb_extslots * EROFS_EXTSLOT_SIZE;
> 
> Otherwise it looks good to me,
> 
> Reviewed-by: Chao Yu <yuchao0@huawei.com>

Thanks for the review!

Thanks,
Gao Xiang

> 
> Thanks,
> 
> > +	if (sbi->sb_size > EROFS_BLKSIZ) {
> > +		erofs_err(sb, "invalid sb_extslots %u (more than a fs block)",
> > +			  sbi->sb_size);
> > +		goto out;
> > +	}
> >   	sbi->blocks = le32_to_cpu(dsb->blocks);
> >   	sbi->meta_blkaddr = le32_to_cpu(dsb->meta_blkaddr);
> >   #ifdef CONFIG_EROFS_FS_XATTR
> > @@ -189,7 +327,10 @@ static int erofs_read_superblock(struct super_block *sb)
> >   	}
> >   	/* parse on-disk compression configurations */
> > -	ret = z_erofs_load_lz4_config(sb, dsb, NULL, 0);
> > +	if (erofs_sb_has_compr_cfgs(sbi))
> > +		ret = erofs_load_compr_cfgs(sb, dsb);
> > +	else
> > +		ret = z_erofs_load_lz4_config(sb, dsb, NULL, 0);
> >   out:
> >   	kunmap(page);
> >   	put_page(page);
> > 
> 


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

* Re: [PATCH v2 4/4] erofs: add on-disk compression configurations
  2021-03-29  6:36       ` Gao Xiang
@ 2021-03-29  6:43         ` Chao Yu
  2021-03-29  6:55           ` Gao Xiang
  0 siblings, 1 reply; 11+ messages in thread
From: Chao Yu @ 2021-03-29  6:43 UTC (permalink / raw)
  To: Gao Xiang; +Cc: linux-erofs, LKML

On 2021/3/29 14:36, Gao Xiang wrote:
> Hi Chao,
> 
> On Mon, Mar 29, 2021 at 02:26:05PM +0800, Chao Yu wrote:
>> On 2021/3/29 9:23, Gao Xiang wrote:
>>> From: Gao Xiang <hsiangkao@redhat.com>
>>>
>>> Add a bitmap for available compression algorithms and a variable-sized
>>> on-disk table for compression options in preparation for upcoming big
>>> pcluster and LZMA algorithm, which follows the end of super block.
>>>
>>> To parse the compression options, the bitmap is scanned one by one.
>>> For each available algorithm, there is data followed by 2-byte `length'
>>> correspondingly (it's enough for most cases, or entire fs blocks should
>>> be used.)
>>>
>>> With such available algorithm bitmap, kernel itself can also refuse to
>>> mount such filesystem if any unsupported compression algorithm exists.
>>>
>>> Note that COMPR_CFGS feature will be enabled with BIG_PCLUSTER.
>>>
>>> Signed-off-by: Gao Xiang <hsiangkao@redhat.com>
>>> ---
>>>    fs/erofs/decompressor.c |   2 +-
>>>    fs/erofs/erofs_fs.h     |  14 ++--
>>>    fs/erofs/internal.h     |   5 +-
>>>    fs/erofs/super.c        | 143 +++++++++++++++++++++++++++++++++++++++-
>>>    4 files changed, 157 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/fs/erofs/decompressor.c b/fs/erofs/decompressor.c
>>> index 97538ff24a19..27aa6a99b371 100644
>>> --- a/fs/erofs/decompressor.c
>>> +++ b/fs/erofs/decompressor.c
>>> @@ -41,7 +41,7 @@ int z_erofs_load_lz4_config(struct super_block *sb,
>>>    		}
>>>    		distance = le16_to_cpu(lz4->max_distance);
>>>    	} else {
>>> -		distance = le16_to_cpu(dsb->lz4_max_distance);
>>> +		distance = le16_to_cpu(dsb->u1.lz4_max_distance);
>>>    	}
>>>    	EROFS_SB(sb)->lz4.max_distance_pages = distance ?
>>> diff --git a/fs/erofs/erofs_fs.h b/fs/erofs/erofs_fs.h
>>> index e0f3c0db1f82..5a126493d4d9 100644
>>> --- a/fs/erofs/erofs_fs.h
>>> +++ b/fs/erofs/erofs_fs.h
>>> @@ -18,15 +18,16 @@
>>>     * be incompatible with this kernel version.
>>>     */
>>>    #define EROFS_FEATURE_INCOMPAT_LZ4_0PADDING	0x00000001
>>> +#define EROFS_FEATURE_INCOMPAT_COMPR_CFGS	0x00000002
>>>    #define EROFS_ALL_FEATURE_INCOMPAT		EROFS_FEATURE_INCOMPAT_LZ4_0PADDING
>>> -/* 128-byte erofs on-disk super block */
>>> +/* erofs on-disk super block (currently 128 bytes) */
>>>    struct erofs_super_block {
>>>    	__le32 magic;           /* file system magic number */
>>>    	__le32 checksum;        /* crc32c(super_block) */
>>>    	__le32 feature_compat;
>>>    	__u8 blkszbits;         /* support block_size == PAGE_SIZE only */
>>> -	__u8 reserved;
>>> +	__u8 sb_extslots;	/* superblock size = 128 + sb_extslots * 16 */
>>>    	__le16 root_nid;	/* nid of root directory */
>>>    	__le64 inos;            /* total valid ino # (== f_files - f_favail) */
>>> @@ -39,8 +40,12 @@ struct erofs_super_block {
>>>    	__u8 uuid[16];          /* 128-bit uuid for volume */
>>>    	__u8 volume_name[16];   /* volume name */
>>>    	__le32 feature_incompat;
>>> -	/* customized lz4 sliding window size instead of 64k by default */
>>> -	__le16 lz4_max_distance;
>>> +	union {
>>> +		/* bitmap for available compression algorithms */
>>> +		__le16 available_compr_algs;
>>> +		/* customized sliding window size instead of 64k by default */
>>> +		__le16 lz4_max_distance;
>>> +	} __packed u1;
>>>    	__u8 reserved2[42];
>>>    };
>>> @@ -196,6 +201,7 @@ enum {
>>>    	Z_EROFS_COMPRESSION_LZ4	= 0,
>>>    	Z_EROFS_COMPRESSION_MAX
>>>    };
>>> +#define Z_EROFS_ALL_COMPR_ALGS		(1 << (Z_EROFS_COMPRESSION_MAX - 1))
>>>    /* 14 bytes (+ length field = 16 bytes) */
>>>    struct z_erofs_lz4_cfgs {
>>> diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
>>> index 46b977f348eb..f3fa895d809f 100644
>>> --- a/fs/erofs/internal.h
>>> +++ b/fs/erofs/internal.h
>>> @@ -75,6 +75,7 @@ struct erofs_sb_info {
>>>    	struct xarray managed_pslots;
>>>    	unsigned int shrinker_run_no;
>>> +	u16 available_compr_algs;
>>>    	/* pseudo inode to manage cached pages */
>>>    	struct inode *managed_cache;
>>> @@ -90,6 +91,7 @@ struct erofs_sb_info {
>>>    	/* inode slot unit size in bit shift */
>>>    	unsigned char islotbits;
>>> +	u32 sb_size;			/* total superblock size */
>>>    	u32 build_time_nsec;
>>>    	u64 build_time;
>>> @@ -233,6 +235,7 @@ static inline bool erofs_sb_has_##name(struct erofs_sb_info *sbi) \
>>>    }
>>>    EROFS_FEATURE_FUNCS(lz4_0padding, incompat, INCOMPAT_LZ4_0PADDING)
>>> +EROFS_FEATURE_FUNCS(compr_cfgs, incompat, INCOMPAT_COMPR_CFGS)
>>>    EROFS_FEATURE_FUNCS(sb_chksum, compat, COMPAT_SB_CHKSUM)
>>>    /* atomic flag definitions */
>>> @@ -454,7 +457,7 @@ static inline int z_erofs_load_lz4_config(struct super_block *sb,
>>>    				  struct erofs_super_block *dsb,
>>>    				  struct z_erofs_lz4_cfgs *lz4, int len)
>>>    {
>>> -	if (lz4 || dsb->lz4_max_distance) {
>>> +	if (lz4 || dsb->u1.lz4_max_distance) {
>>>    		erofs_err(sb, "lz4 algorithm isn't enabled");
>>>    		return -EINVAL;
>>>    	}
>>> diff --git a/fs/erofs/super.c b/fs/erofs/super.c
>>> index 1ca8da3f2125..628c751634fe 100644
>>> --- a/fs/erofs/super.c
>>> +++ b/fs/erofs/super.c
>>> @@ -122,6 +122,138 @@ static bool check_layout_compatibility(struct super_block *sb,
>>>    	return true;
>>>    }
>>> +#ifdef CONFIG_EROFS_FS_ZIP
>>> +/* read variable-sized metadata, offset will be aligned by 4-byte */
>>> +static void *erofs_read_metadata(struct super_block *sb, struct page **pagep,
>>> +				 erofs_off_t *offset, int *lengthp)
>>> +{
>>> +	struct page *page = *pagep;
>>> +	u8 *buffer, *ptr;
>>> +	int len, i, cnt;
>>> +	erofs_blk_t blk;
>>> +
>>> +	*offset = round_up(*offset, 4);
>>> +	blk = erofs_blknr(*offset);
>>> +
>>> +	if (!page || page->index != blk) {
>>> +		if (page) {
>>> +			unlock_page(page);
>>> +			put_page(page);
>>> +		}
>>> +		page = erofs_get_meta_page(sb, blk);
>>> +		if (IS_ERR(page))
>>> +			goto err_nullpage;
>>> +	}
>>> +
>>> +	ptr = kmap(page);
>>> +	len = le16_to_cpu(*(__le16 *)&ptr[erofs_blkoff(*offset)]);
>>> +	if (!len)
>>> +		len = U16_MAX + 1;
>>> +	buffer = kmalloc(len, GFP_KERNEL);
>>> +	if (!buffer) {
>>> +		buffer = ERR_PTR(-ENOMEM);
>>> +		goto out;
>>> +	}
>>> +	*offset += sizeof(__le16);
>>> +	*lengthp = len;
>>> +
>>> +	for (i = 0; i < len; i += cnt) {
>>> +		cnt = min(EROFS_BLKSIZ - (int)erofs_blkoff(*offset), len - i);
>>> +		blk = erofs_blknr(*offset);
>>> +
>>> +		if (!page || page->index != blk) {
>>> +			if (page) {
>>> +				kunmap(page);
>>> +				unlock_page(page);
>>> +				put_page(page);
>>> +			}
>>> +			page = erofs_get_meta_page(sb, blk);
>>> +			if (IS_ERR(page)) {
>>> +				kfree(buffer);
>>> +				goto err_nullpage;
>>> +			}
>>> +			ptr = kmap(page);
>>> +		}
>>> +		memcpy(buffer + i, ptr + erofs_blkoff(*offset), cnt);
>>> +		*offset += cnt;
>>> +	}
>>> +out:
>>> +	kunmap(page);
>>> +	*pagep = page;
>>> +	return buffer;
>>> +err_nullpage:
>>> +	*pagep = NULL;
>>> +	return page;
>>> +}
>>> +
>>> +static int erofs_load_compr_cfgs(struct super_block *sb,
>>> +				 struct erofs_super_block *dsb)
>>> +{
>>> +	struct erofs_sb_info *sbi;
>>> +	struct page *page;
>>> +	unsigned int algs, alg;
>>> +	erofs_off_t offset;
>>> +	int size, ret;
>>> +
>>> +	sbi = EROFS_SB(sb);
>>> +	sbi->available_compr_algs = le16_to_cpu(dsb->u1.available_compr_algs);
>>> +
>>> +	if (sbi->available_compr_algs & ~Z_EROFS_ALL_COMPR_ALGS) {
>>> +		erofs_err(sb,
>>> +"try to load compressed image with unsupported algorithms %x",
>>
>> Minor style issue:
>>
>> 			"try to load compressed image with unsupported "
>> 			"algorithms %x",
>> 			sbi->available_compr_algs & ~Z_EROFS_ALL_COMPR_ALGS);
> 
> If I remembered correctly, kernel code style suggests "don't break the print
> message, it'd more easy to grep". Actually such style above is from XFS, and
> can pass checkpatch.pl check, see:
> 
> (fs/xfs/xfs_log_recover.c) xlog_recover():
> 		/*
> 		 * Version 5 superblock log feature mask validation. We know the
> 		 * log is dirty so check if there are any unknown log features
> 		 * in what we need to recover. If there are unknown features
> 		 * (e.g. unsupported transactions, then simply reject the
> 		 * attempt at recovery before touching anything.
> 		 */
> 		if (XFS_SB_VERSION_NUM(&log->l_mp->m_sb) == XFS_SB_VERSION_5 &&
> 		    xfs_sb_has_incompat_log_feature(&log->l_mp->m_sb,
> 					XFS_SB_FEAT_INCOMPAT_LOG_UNKNOWN)) {
> 			xfs_warn(log->l_mp,
> "Superblock has unknown incompatible log features (0x%x) enabled.",
> 				(log->l_mp->m_sb.sb_features_log_incompat &
> 					XFS_SB_FEAT_INCOMPAT_LOG_UNKNOWN));
> 			xfs_warn(log->l_mp,
> "The log can not be fully and/or safely recovered by this kernel.");
> 			xfs_warn(log->l_mp,
> "Please recover the log on a kernel that supports the unknown features.");
> 			return -EINVAL;
> 		}
> 
> If that does't look ok for us, I could use > 80 line for this instead,
> but I tend to not break the message ..

Xiang,

Ah, I didn't notice this is following above style, if it's fine to you,
let's use some tabs in front of message line, though it will cause
exceeding 80 line warning.

Thanks,

> 
>>
>>> +			  sbi->available_compr_algs & ~Z_EROFS_ALL_COMPR_ALGS);
>>> +		return -EINVAL;
>>> +	}
>>> +
>>> +	offset = EROFS_SUPER_OFFSET + sbi->sb_size;
>>> +	page = NULL;
>>> +	alg = 0;
>>> +	ret = 0;
>>> +
>>> +	for (algs = sbi->available_compr_algs; algs; algs >>= 1, ++alg) {
>>> +		void *data;
>>> +
>>> +		if (!(algs & 1))
>>> +			continue;
>>> +
>>> +		data = erofs_read_metadata(sb, &page, &offset, &size);
>>> +		if (IS_ERR(data)) {
>>> +			ret = PTR_ERR(data);
>>> +			goto err;
>>> +		}
>>> +
>>> +		switch (alg) {
>>> +		case Z_EROFS_COMPRESSION_LZ4:
>>> +			ret = z_erofs_load_lz4_config(sb, dsb, data, size);
>>> +			break;
>>> +		default:
>>> +			DBG_BUGON(1);
>>> +			ret = -EFAULT;
>>> +		}
>>> +		kfree(data);
>>> +		if (ret)
>>> +			goto err;
>>> +	}
>>> +err:
>>> +	if (page) {
>>> +		unlock_page(page);
>>> +		put_page(page);
>>> +	}
>>> +	return ret;
>>> +}
>>> +#else
>>> +static int erofs_load_compr_cfgs(struct super_block *sb,
>>> +				 struct erofs_super_block *dsb)
>>> +{
>>> +	if (dsb->u1.available_compr_algs) {
>>> +		erofs_err(sb,
>>> +"try to load compressed image when compression is disabled");
>>
>> Ditto,
>> 		erofs_err(sb, "try to load compressed image when "
>> 			  "compression is disabled");
>>
> 
> The same above....
> 
>>
>>> +		return -EINVAL;
>>> +	}
>>> +	return 0;
>>> +}
>>> +#endif
>>> +
>>>    static int erofs_read_superblock(struct super_block *sb)
>>>    {
>>>    	struct erofs_sb_info *sbi;
>>> @@ -166,6 +298,12 @@ static int erofs_read_superblock(struct super_block *sb)
>>>    	if (!check_layout_compatibility(sb, dsb))
>>>    		goto out;
>>> +	sbi->sb_size = 128 + dsb->sb_extslots * 16;
>>
>> 	sbi->sb_size = sizeof(struct erofs_super_block) +
>> 			dsb->sb_extslots * EROFS_EXTSLOT_SIZE;
>>
>> Otherwise it looks good to me,
>>
>> Reviewed-by: Chao Yu <yuchao0@huawei.com>
> 
> Thanks for the review!
> 
> Thanks,
> Gao Xiang
> 
>>
>> Thanks,
>>
>>> +	if (sbi->sb_size > EROFS_BLKSIZ) {
>>> +		erofs_err(sb, "invalid sb_extslots %u (more than a fs block)",
>>> +			  sbi->sb_size);
>>> +		goto out;
>>> +	}
>>>    	sbi->blocks = le32_to_cpu(dsb->blocks);
>>>    	sbi->meta_blkaddr = le32_to_cpu(dsb->meta_blkaddr);
>>>    #ifdef CONFIG_EROFS_FS_XATTR
>>> @@ -189,7 +327,10 @@ static int erofs_read_superblock(struct super_block *sb)
>>>    	}
>>>    	/* parse on-disk compression configurations */
>>> -	ret = z_erofs_load_lz4_config(sb, dsb, NULL, 0);
>>> +	if (erofs_sb_has_compr_cfgs(sbi))
>>> +		ret = erofs_load_compr_cfgs(sb, dsb);
>>> +	else
>>> +		ret = z_erofs_load_lz4_config(sb, dsb, NULL, 0);
>>>    out:
>>>    	kunmap(page);
>>>    	put_page(page);
>>>
>>
> 
> .
> 

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

* Re: [PATCH v2 4/4] erofs: add on-disk compression configurations
  2021-03-29  6:43         ` Chao Yu
@ 2021-03-29  6:55           ` Gao Xiang
  2021-03-29  7:52             ` Chao Yu
  0 siblings, 1 reply; 11+ messages in thread
From: Gao Xiang @ 2021-03-29  6:55 UTC (permalink / raw)
  To: Chao Yu; +Cc: linux-erofs, LKML

On Mon, Mar 29, 2021 at 02:43:48PM +0800, Chao Yu wrote:

...

> > > > +
> > > > +static int erofs_load_compr_cfgs(struct super_block *sb,
> > > > +				 struct erofs_super_block *dsb)
> > > > +{
> > > > +	struct erofs_sb_info *sbi;
> > > > +	struct page *page;
> > > > +	unsigned int algs, alg;
> > > > +	erofs_off_t offset;
> > > > +	int size, ret;
> > > > +
> > > > +	sbi = EROFS_SB(sb);
> > > > +	sbi->available_compr_algs = le16_to_cpu(dsb->u1.available_compr_algs);
> > > > +
> > > > +	if (sbi->available_compr_algs & ~Z_EROFS_ALL_COMPR_ALGS) {
> > > > +		erofs_err(sb,
> > > > +"try to load compressed image with unsupported algorithms %x",
> > > 
> > > Minor style issue:
> > > 
> > > 			"try to load compressed image with unsupported "
> > > 			"algorithms %x",
> > > 			sbi->available_compr_algs & ~Z_EROFS_ALL_COMPR_ALGS);
> > 
> > If I remembered correctly, kernel code style suggests "don't break the print
> > message, it'd more easy to grep". Actually such style above is from XFS, and
> > can pass checkpatch.pl check, see:
> > 
> > (fs/xfs/xfs_log_recover.c) xlog_recover():
> > 		/*
> > 		 * Version 5 superblock log feature mask validation. We know the
> > 		 * log is dirty so check if there are any unknown log features
> > 		 * in what we need to recover. If there are unknown features
> > 		 * (e.g. unsupported transactions, then simply reject the
> > 		 * attempt at recovery before touching anything.
> > 		 */
> > 		if (XFS_SB_VERSION_NUM(&log->l_mp->m_sb) == XFS_SB_VERSION_5 &&
> > 		    xfs_sb_has_incompat_log_feature(&log->l_mp->m_sb,
> > 					XFS_SB_FEAT_INCOMPAT_LOG_UNKNOWN)) {
> > 			xfs_warn(log->l_mp,
> > "Superblock has unknown incompatible log features (0x%x) enabled.",
> > 				(log->l_mp->m_sb.sb_features_log_incompat &
> > 					XFS_SB_FEAT_INCOMPAT_LOG_UNKNOWN));
> > 			xfs_warn(log->l_mp,
> > "The log can not be fully and/or safely recovered by this kernel.");
> > 			xfs_warn(log->l_mp,
> > "Please recover the log on a kernel that supports the unknown features.");
> > 			return -EINVAL;
> > 		}
> > 
> > If that does't look ok for us, I could use > 80 line for this instead,
> > but I tend to not break the message ..
> 
> Xiang,
> 
> Ah, I didn't notice this is following above style, if it's fine to you,
> let's use some tabs in front of message line, though it will cause
> exceeding 80 line warning.
> 

I found a reference here,
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst?h=v5.11#n99
also vaguely remembered some threads from Linus, but hard to find now :-(

ok, I will insert tabs instead, thanks for the suggestion!

Thanks,
Gao Xiang


> Thanks,


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

* Re: [PATCH v2 4/4] erofs: add on-disk compression configurations
  2021-03-29  6:55           ` Gao Xiang
@ 2021-03-29  7:52             ` Chao Yu
  0 siblings, 0 replies; 11+ messages in thread
From: Chao Yu @ 2021-03-29  7:52 UTC (permalink / raw)
  To: Gao Xiang; +Cc: linux-erofs, LKML

On 2021/3/29 14:55, Gao Xiang wrote:
> I found a reference here,
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst?h=v5.11#n99
> also vaguely remembered some threads from Linus, but hard to find now :-(

Sure, we can follow this rule for erofs code style. :)

Thanks,

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

* [PATCH v3 4/4] erofs: add on-disk compression configurations
  2021-03-29  1:23   ` [PATCH v2 4/4] erofs: add on-disk compression configurations Gao Xiang via Linux-erofs
  2021-03-29  6:26     ` Chao Yu
@ 2021-03-29 10:00     ` Gao Xiang via Linux-erofs
  1 sibling, 0 replies; 11+ messages in thread
From: Gao Xiang via Linux-erofs @ 2021-03-29 10:00 UTC (permalink / raw)
  To: linux-erofs, Chao Yu, Chao Yu; +Cc: LKML

From: Gao Xiang <hsiangkao@redhat.com>

Add a bitmap for available compression algorithms and a variable-sized
on-disk table for compression options in preparation for upcoming big
pcluster and LZMA algorithm, which follows the end of super block.

To parse the compression options, the bitmap is scanned one by one.
For each available algorithm, there is data followed by 2-byte `length'
correspondingly (it's enough for most cases, or entire fs blocks should
be used.)

With such available algorithm bitmap, kernel itself can also refuse to
mount such filesystem if any unsupported compression algorithm exists.

Note that COMPR_CFGS feature will be enabled with BIG_PCLUSTER.

Reviewed-by: Chao Yu <yuchao0@huawei.com>
Signed-off-by: Gao Xiang <hsiangkao@redhat.com>
---
changes since v2:
 - move upwords print messages since even using a new extra line with
   indentation, they still exceed 80 char;
 - introduce EROFS_SB_EXTSLOT_SIZE but remaining 128 since
   sizeof(struct erofs_super_block) could be changed in the future.

 fs/erofs/decompressor.c |   2 +-
 fs/erofs/erofs_fs.h     |  16 +++--
 fs/erofs/internal.h     |   5 +-
 fs/erofs/super.c        | 141 +++++++++++++++++++++++++++++++++++++++-
 4 files changed, 157 insertions(+), 7 deletions(-)

diff --git a/fs/erofs/decompressor.c b/fs/erofs/decompressor.c
index 97538ff24a19..27aa6a99b371 100644
--- a/fs/erofs/decompressor.c
+++ b/fs/erofs/decompressor.c
@@ -41,7 +41,7 @@ int z_erofs_load_lz4_config(struct super_block *sb,
 		}
 		distance = le16_to_cpu(lz4->max_distance);
 	} else {
-		distance = le16_to_cpu(dsb->lz4_max_distance);
+		distance = le16_to_cpu(dsb->u1.lz4_max_distance);
 	}
 
 	EROFS_SB(sb)->lz4.max_distance_pages = distance ?
diff --git a/fs/erofs/erofs_fs.h b/fs/erofs/erofs_fs.h
index 700597e9c810..17bc0b5f117d 100644
--- a/fs/erofs/erofs_fs.h
+++ b/fs/erofs/erofs_fs.h
@@ -18,15 +18,18 @@
  * be incompatible with this kernel version.
  */
 #define EROFS_FEATURE_INCOMPAT_LZ4_0PADDING	0x00000001
+#define EROFS_FEATURE_INCOMPAT_COMPR_CFGS	0x00000002
 #define EROFS_ALL_FEATURE_INCOMPAT		EROFS_FEATURE_INCOMPAT_LZ4_0PADDING
 
-/* 128-byte erofs on-disk super block */
+#define EROFS_SB_EXTSLOT_SIZE	16
+
+/* erofs on-disk super block (currently 128 bytes) */
 struct erofs_super_block {
 	__le32 magic;           /* file system magic number */
 	__le32 checksum;        /* crc32c(super_block) */
 	__le32 feature_compat;
 	__u8 blkszbits;         /* support block_size == PAGE_SIZE only */
-	__u8 reserved;
+	__u8 sb_extslots;	/* superblock size = 128 + sb_extslots * 16 */
 
 	__le16 root_nid;	/* nid of root directory */
 	__le64 inos;            /* total valid ino # (== f_files - f_favail) */
@@ -39,8 +42,12 @@ struct erofs_super_block {
 	__u8 uuid[16];          /* 128-bit uuid for volume */
 	__u8 volume_name[16];   /* volume name */
 	__le32 feature_incompat;
-	/* customized lz4 sliding window size instead of 64k by default */
-	__le16 lz4_max_distance;
+	union {
+		/* bitmap for available compression algorithms */
+		__le16 available_compr_algs;
+		/* customized sliding window size instead of 64k by default */
+		__le16 lz4_max_distance;
+	} __packed u1;
 	__u8 reserved2[42];
 };
 
@@ -199,6 +206,7 @@ enum {
 	Z_EROFS_COMPRESSION_LZ4	= 0,
 	Z_EROFS_COMPRESSION_MAX
 };
+#define Z_EROFS_ALL_COMPR_ALGS		(1 << (Z_EROFS_COMPRESSION_MAX - 1))
 
 /* 14 bytes (+ length field = 16 bytes) */
 struct z_erofs_lz4_cfgs {
diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h
index b02fc64fcece..60063bbbb91a 100644
--- a/fs/erofs/internal.h
+++ b/fs/erofs/internal.h
@@ -75,6 +75,7 @@ struct erofs_sb_info {
 	struct xarray managed_pslots;
 
 	unsigned int shrinker_run_no;
+	u16 available_compr_algs;
 
 	/* pseudo inode to manage cached pages */
 	struct inode *managed_cache;
@@ -90,6 +91,7 @@ struct erofs_sb_info {
 	/* inode slot unit size in bit shift */
 	unsigned char islotbits;
 
+	u32 sb_size;			/* total superblock size */
 	u32 build_time_nsec;
 	u64 build_time;
 
@@ -233,6 +235,7 @@ static inline bool erofs_sb_has_##name(struct erofs_sb_info *sbi) \
 }
 
 EROFS_FEATURE_FUNCS(lz4_0padding, incompat, INCOMPAT_LZ4_0PADDING)
+EROFS_FEATURE_FUNCS(compr_cfgs, incompat, INCOMPAT_COMPR_CFGS)
 EROFS_FEATURE_FUNCS(sb_chksum, compat, COMPAT_SB_CHKSUM)
 
 /* atomic flag definitions */
@@ -452,7 +455,7 @@ static inline int z_erofs_load_lz4_config(struct super_block *sb,
 				  struct erofs_super_block *dsb,
 				  struct z_erofs_lz4_cfgs *lz4, int len)
 {
-	if (lz4 || dsb->lz4_max_distance) {
+	if (lz4 || dsb->u1.lz4_max_distance) {
 		erofs_err(sb, "lz4 algorithm isn't enabled");
 		return -EINVAL;
 	}
diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index 1ca8da3f2125..ec9e5fb073b5 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -122,6 +122,136 @@ static bool check_layout_compatibility(struct super_block *sb,
 	return true;
 }
 
+#ifdef CONFIG_EROFS_FS_ZIP
+/* read variable-sized metadata, offset will be aligned by 4-byte */
+static void *erofs_read_metadata(struct super_block *sb, struct page **pagep,
+				 erofs_off_t *offset, int *lengthp)
+{
+	struct page *page = *pagep;
+	u8 *buffer, *ptr;
+	int len, i, cnt;
+	erofs_blk_t blk;
+
+	*offset = round_up(*offset, 4);
+	blk = erofs_blknr(*offset);
+
+	if (!page || page->index != blk) {
+		if (page) {
+			unlock_page(page);
+			put_page(page);
+		}
+		page = erofs_get_meta_page(sb, blk);
+		if (IS_ERR(page))
+			goto err_nullpage;
+	}
+
+	ptr = kmap(page);
+	len = le16_to_cpu(*(__le16 *)&ptr[erofs_blkoff(*offset)]);
+	if (!len)
+		len = U16_MAX + 1;
+	buffer = kmalloc(len, GFP_KERNEL);
+	if (!buffer) {
+		buffer = ERR_PTR(-ENOMEM);
+		goto out;
+	}
+	*offset += sizeof(__le16);
+	*lengthp = len;
+
+	for (i = 0; i < len; i += cnt) {
+		cnt = min(EROFS_BLKSIZ - (int)erofs_blkoff(*offset), len - i);
+		blk = erofs_blknr(*offset);
+
+		if (!page || page->index != blk) {
+			if (page) {
+				kunmap(page);
+				unlock_page(page);
+				put_page(page);
+			}
+			page = erofs_get_meta_page(sb, blk);
+			if (IS_ERR(page)) {
+				kfree(buffer);
+				goto err_nullpage;
+			}
+			ptr = kmap(page);
+		}
+		memcpy(buffer + i, ptr + erofs_blkoff(*offset), cnt);
+		*offset += cnt;
+	}
+out:
+	kunmap(page);
+	*pagep = page;
+	return buffer;
+err_nullpage:
+	*pagep = NULL;
+	return page;
+}
+
+static int erofs_load_compr_cfgs(struct super_block *sb,
+				 struct erofs_super_block *dsb)
+{
+	struct erofs_sb_info *sbi;
+	struct page *page;
+	unsigned int algs, alg;
+	erofs_off_t offset;
+	int size, ret;
+
+	sbi = EROFS_SB(sb);
+	sbi->available_compr_algs = le16_to_cpu(dsb->u1.available_compr_algs);
+
+	if (sbi->available_compr_algs & ~Z_EROFS_ALL_COMPR_ALGS) {
+		erofs_err(sb, "try to load compressed fs with unsupported algorithms %x",
+			  sbi->available_compr_algs & ~Z_EROFS_ALL_COMPR_ALGS);
+		return -EINVAL;
+	}
+
+	offset = EROFS_SUPER_OFFSET + sbi->sb_size;
+	page = NULL;
+	alg = 0;
+	ret = 0;
+
+	for (algs = sbi->available_compr_algs; algs; algs >>= 1, ++alg) {
+		void *data;
+
+		if (!(algs & 1))
+			continue;
+
+		data = erofs_read_metadata(sb, &page, &offset, &size);
+		if (IS_ERR(data)) {
+			ret = PTR_ERR(data);
+			goto err;
+		}
+
+		switch (alg) {
+		case Z_EROFS_COMPRESSION_LZ4:
+			ret = z_erofs_load_lz4_config(sb, dsb, data, size);
+			break;
+		default:
+			DBG_BUGON(1);
+			ret = -EFAULT;
+		}
+		kfree(data);
+		if (ret)
+			goto err;
+	}
+err:
+	if (page) {
+		unlock_page(page);
+		put_page(page);
+	}
+	return ret;
+}
+#else
+static int erofs_load_compr_cfgs(struct super_block *sb,
+				 struct erofs_super_block *dsb)
+{
+	if (dsb->u1.available_compr_algs) {
+		erofs_err(sb, "try to load compressed fs when compression is disabled");
+		return -EINVAL;
+	}
+	return 0;
+}
+#endif
+
 static int erofs_read_superblock(struct super_block *sb)
 {
 	struct erofs_sb_info *sbi;
@@ -166,6 +296,12 @@ static int erofs_read_superblock(struct super_block *sb)
 	if (!check_layout_compatibility(sb, dsb))
 		goto out;
 
+	sbi->sb_size = 128 + dsb->sb_extslots * EROFS_SB_EXTSLOT_SIZE;
+	if (sbi->sb_size > EROFS_BLKSIZ) {
+		erofs_err(sb, "invalid sb_extslots %u (more than a fs block)",
+			  sbi->sb_size);
+		goto out;
+	}
 	sbi->blocks = le32_to_cpu(dsb->blocks);
 	sbi->meta_blkaddr = le32_to_cpu(dsb->meta_blkaddr);
 #ifdef CONFIG_EROFS_FS_XATTR
@@ -189,7 +325,10 @@ static int erofs_read_superblock(struct super_block *sb)
 	}
 
 	/* parse on-disk compression configurations */
-	ret = z_erofs_load_lz4_config(sb, dsb, NULL, 0);
+	if (erofs_sb_has_compr_cfgs(sbi))
+		ret = erofs_load_compr_cfgs(sb, dsb);
+	else
+		ret = z_erofs_load_lz4_config(sb, dsb, NULL, 0);
 out:
 	kunmap(page);
 	put_page(page);
-- 
2.20.1


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

end of thread, other threads:[~2021-03-29 10:00 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20210329012308.28743-1-hsiangkao.ref@aol.com>
2021-03-29  1:23 ` [PATCH v2 0/4] erofs: introduce on-disk compression configurations Gao Xiang via Linux-erofs
2021-03-29  1:23   ` [PATCH v2 1/4] erofs: introduce erofs_sb_has_xxx() helpers Gao Xiang via Linux-erofs
2021-03-29  1:23   ` [PATCH v2 2/4] erofs: support adjust lz4 history window size Gao Xiang via Linux-erofs
2021-03-29  1:23   ` [PATCH v2 3/4] erofs: introduce on-disk lz4 fs configurations Gao Xiang via Linux-erofs
2021-03-29  1:23   ` [PATCH v2 4/4] erofs: add on-disk compression configurations Gao Xiang via Linux-erofs
2021-03-29  6:26     ` Chao Yu
2021-03-29  6:36       ` Gao Xiang
2021-03-29  6:43         ` Chao Yu
2021-03-29  6:55           ` Gao Xiang
2021-03-29  7:52             ` Chao Yu
2021-03-29 10:00     ` [PATCH v3 " Gao Xiang via Linux-erofs

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