Re: [PATCH v2 2/3] erofs: introduce the secondary compression head

From: Yue Hu
Date: Fri Oct 08 2021 - 23:50:41 EST

On Sat, 9 Oct 2021 04:08:38 +0800
Gao Xiang <xiang@xxxxxxxxxx> wrote:

> From: Gao Xiang <hsiangkao@xxxxxxxxxxxxxxxxx>
>
> Previously, for each HEAD lcluster, it can be either HEAD or PLAIN
> lcluster to indicate whether the whole pcluster is compressed or not.
>
> In this patch, a new HEAD2 head type is introduced to specify another
> compression algorithm other than the primary algorithm for each
> compressed file, which can be used for upcoming LZMA compression and
> LZ4 range dictionary compression for various data patterns.
>
> It has been stayed in the EROFS roadmap for years. Complete it now!
>
> Signed-off-by: Gao Xiang <hsiangkao@xxxxxxxxxxxxxxxxx>
> ---
> fs/erofs/erofs_fs.h | 8 +++++---
> fs/erofs/zmap.c | 36 +++++++++++++++++++++++++++---------
> 2 files changed, 32 insertions(+), 12 deletions(-)
>
> diff --git a/fs/erofs/erofs_fs.h b/fs/erofs/erofs_fs.h
> index b0b23f41abc3..f579c8c78fff 100644
> --- a/fs/erofs/erofs_fs.h
> +++ b/fs/erofs/erofs_fs.h
> @@ -21,11 +21,13 @@
> #define EROFS_FEATURE_INCOMPAT_COMPR_CFGS 0x00000002
> #define EROFS_FEATURE_INCOMPAT_BIG_PCLUSTER 0x00000002
> #define EROFS_FEATURE_INCOMPAT_CHUNKED_FILE 0x00000004
> +#define EROFS_FEATURE_INCOMPAT_COMPR_HEAD2 0x00000008
> #define EROFS_ALL_FEATURE_INCOMPAT \
> (EROFS_FEATURE_INCOMPAT_LZ4_0PADDING | \
> EROFS_FEATURE_INCOMPAT_COMPR_CFGS | \
> EROFS_FEATURE_INCOMPAT_BIG_PCLUSTER | \
> - EROFS_FEATURE_INCOMPAT_CHUNKED_FILE)
> + EROFS_FEATURE_INCOMPAT_CHUNKED_FILE | \
> + EROFS_FEATURE_INCOMPAT_COMPR_HEAD2)
>
> #define EROFS_SB_EXTSLOT_SIZE 16
>
> @@ -314,9 +316,9 @@ struct z_erofs_map_header {
> */
> enum {
> Z_EROFS_VLE_CLUSTER_TYPE_PLAIN = 0,
> - Z_EROFS_VLE_CLUSTER_TYPE_HEAD = 1,
> + Z_EROFS_VLE_CLUSTER_TYPE_HEAD1 = 1,
> Z_EROFS_VLE_CLUSTER_TYPE_NONHEAD = 2,
> - Z_EROFS_VLE_CLUSTER_TYPE_RESERVED = 3,
> + Z_EROFS_VLE_CLUSTER_TYPE_HEAD2 = 3,
> Z_EROFS_VLE_CLUSTER_TYPE_MAX
> };
>
> diff --git a/fs/erofs/zmap.c b/fs/erofs/zmap.c
> index 9d9c26343dab..03945f15ceae 100644
> --- a/fs/erofs/zmap.c
> +++ b/fs/erofs/zmap.c
> @@ -69,11 +69,17 @@ static int z_erofs_fill_inode_lazy(struct inode *inode)
> vi->z_algorithmtype[1] = h->h_algorithmtype >> 4;
>
> if (vi->z_algorithmtype[0] >= Z_EROFS_COMPRESSION_MAX) {
> - erofs_err(sb, "unknown compression format %u for nid %llu, please upgrade kernel",
> + erofs_err(sb, "unknown HEAD1 format %u for nid %llu, please upgrade kernel",
> vi->z_algorithmtype[0], vi->nid);
> err = -EOPNOTSUPP;
> goto unmap_done;
> }
> + if (vi->z_algorithmtype[1] >= Z_EROFS_COMPRESSION_MAX) {
> + erofs_err(sb, "unknown HEAD2 format %u for nid %llu, please upgrade kernel",
> + vi->z_algorithmtype[1], vi->nid);
> + err = -EOPNOTSUPP;
> + goto unmap_done;
> + }

Seems duplicated a little, how about below code?

if (vi->z_algorithmtype[i] >= Z_EROFS_COMPRESSION_MAX ||
vi->z_algorithmtype[++i] >= Z_EROFS_COMPRESSION_MAX) {
erofs_err(sb, "unknown HEAD%u format %u for nid %llu, please upgrade kernel",
i, vi->z_algorithmtype[i], vi->nid);
err = -EOPNOTSUPP;
goto unmap_done;
}

>
> vi->z_logical_clusterbits = LOG_BLOCK_SIZE + (h->h_clusterbits & 7);
> if (!erofs_sb_has_big_pcluster(EROFS_SB(sb)) &&
> @@ -189,7 +195,8 @@ static int legacy_load_cluster_from_disk(struct z_erofs_maprecorder *m,
> m->delta[1] = le16_to_cpu(di->di_u.delta[1]);
> break;
> case Z_EROFS_VLE_CLUSTER_TYPE_PLAIN:
> - case Z_EROFS_VLE_CLUSTER_TYPE_HEAD:
> + case Z_EROFS_VLE_CLUSTER_TYPE_HEAD1:
> + case Z_EROFS_VLE_CLUSTER_TYPE_HEAD2:
> m->clusterofs = le16_to_cpu(di->di_clusterofs);
> m->pblk = le32_to_cpu(di->di_u.blkaddr);
> break;
> @@ -446,7 +453,8 @@ static int z_erofs_extent_lookback(struct z_erofs_maprecorder *m,
> }
> return z_erofs_extent_lookback(m, m->delta[0]);
> case Z_EROFS_VLE_CLUSTER_TYPE_PLAIN:
> - case Z_EROFS_VLE_CLUSTER_TYPE_HEAD:
> + case Z_EROFS_VLE_CLUSTER_TYPE_HEAD1:
> + case Z_EROFS_VLE_CLUSTER_TYPE_HEAD2:
> m->headtype = m->type;
> map->m_la = (lcn << lclusterbits) | m->clusterofs;
> break;
> @@ -470,13 +478,18 @@ static int z_erofs_get_extent_compressedlen(struct z_erofs_maprecorder *m,
> int err;
>
> DBG_BUGON(m->type != Z_EROFS_VLE_CLUSTER_TYPE_PLAIN &&
> - m->type != Z_EROFS_VLE_CLUSTER_TYPE_HEAD);
> + m->type != Z_EROFS_VLE_CLUSTER_TYPE_HEAD1 &&
> + m->type != Z_EROFS_VLE_CLUSTER_TYPE_HEAD2);
> + DBG_BUGON(m->type != m->headtype);
> +
> if (m->headtype == Z_EROFS_VLE_CLUSTER_TYPE_PLAIN ||
> - !(vi->z_advise & Z_EROFS_ADVISE_BIG_PCLUSTER_1)) {
> + ((m->headtype == Z_EROFS_VLE_CLUSTER_TYPE_HEAD1) &&
> + !(vi->z_advise & Z_EROFS_ADVISE_BIG_PCLUSTER_1)) ||
> + ((m->headtype == Z_EROFS_VLE_CLUSTER_TYPE_HEAD2) &&
> + !(vi->z_advise & Z_EROFS_ADVISE_BIG_PCLUSTER_2))) {
> map->m_plen = 1 << lclusterbits;
> return 0;
> }
> -
> lcn = m->lcn + 1;
> if (m->compressedlcs)
> goto out;
> @@ -498,7 +511,8 @@ static int z_erofs_get_extent_compressedlen(struct z_erofs_maprecorder *m,
>
> switch (m->type) {
> case Z_EROFS_VLE_CLUSTER_TYPE_PLAIN:
> - case Z_EROFS_VLE_CLUSTER_TYPE_HEAD:
> + case Z_EROFS_VLE_CLUSTER_TYPE_HEAD1:
> + case Z_EROFS_VLE_CLUSTER_TYPE_HEAD2:
> /*
> * if the 1st NONHEAD lcluster is actually PLAIN or HEAD type
> * rather than CBLKCNT, it's a 1 lcluster-sized pcluster.
> @@ -553,7 +567,8 @@ static int z_erofs_get_extent_decompressedlen(struct z_erofs_maprecorder *m)
> DBG_BUGON(!m->delta[1] &&
> m->clusterofs != 1 << lclusterbits);
> } else if (m->type == Z_EROFS_VLE_CLUSTER_TYPE_PLAIN ||
> - m->type == Z_EROFS_VLE_CLUSTER_TYPE_HEAD) {
> + m->type == Z_EROFS_VLE_CLUSTER_TYPE_HEAD1 ||
> + m->type == Z_EROFS_VLE_CLUSTER_TYPE_HEAD2) {
> /* go on until the next HEAD lcluster */
> if (lcn != headlcn)
> break;
> @@ -612,7 +627,8 @@ int z_erofs_map_blocks_iter(struct inode *inode,
>
> switch (m.type) {
> case Z_EROFS_VLE_CLUSTER_TYPE_PLAIN:
> - case Z_EROFS_VLE_CLUSTER_TYPE_HEAD:
> + case Z_EROFS_VLE_CLUSTER_TYPE_HEAD1:
> + case Z_EROFS_VLE_CLUSTER_TYPE_HEAD2:
> if (endoff >= m.clusterofs) {
> m.headtype = m.type;
> map->m_la = (m.lcn << lclusterbits) | m.clusterofs;
> @@ -654,6 +670,8 @@ int z_erofs_map_blocks_iter(struct inode *inode,
>
> if (m.headtype == Z_EROFS_VLE_CLUSTER_TYPE_PLAIN)
> map->m_algorithmformat = Z_EROFS_COMPRESSION_SHIFTED;
> + else if (m.headtype == Z_EROFS_VLE_CLUSTER_TYPE_HEAD2)
> + map->m_algorithmformat = vi->z_algorithmtype[1];
> else
> map->m_algorithmformat = vi->z_algorithmtype[0];
>