[RFC PATCH v3 1/2] erofs-utils: support tail-packing inline compressed data
Yue Hu
zbestahu at gmail.com
Fri Dec 10 15:01:40 AEDT 2021
Hi Xiang,
On Fri, 10 Dec 2021 11:52:08 +0800
Gao Xiang <hsiangkao at linux.alibaba.com> wrote:
> Hi Yue,
>
> On Wed, Nov 17, 2021 at 05:22:00PM +0800, Yue Hu wrote:
> > Currently, we only support tail-end inline data for uncompressed
> > files, let's support it as well for compressed files.
> >
> > The idea is from Xiang.
> >
> > Signed-off-by: Yue Hu <huyue2 at yulong.com>
>
> Sorry about delay. Finally I seeked some time to look into this...
>
> > ---
> > v3:
> > - based on commit 9fe440d0ac03 ("erofs-utils: mkfs: introduce --quiet option").
> > - move h_idata_size ahead of h_advise for compatibility.
> > - rename feature/update messages which i think they are more exact.
> >
> > v2:
> > - add 2 bytes to record compressed size of tail-pcluster suggested from
> > Xiang and update related code.
> >
> > include/erofs/internal.h | 1 +
> > include/erofs_fs.h | 10 ++++-
> > lib/compress.c | 83 ++++++++++++++++++++++++++++++----------
> > lib/compressor.c | 9 +++--
> > lib/decompress.c | 4 ++
> > lib/inode.c | 42 ++++++++++----------
> > mkfs/main.c | 7 ++++
> > 7 files changed, 108 insertions(+), 48 deletions(-)
> >
> > diff --git a/include/erofs/internal.h b/include/erofs/internal.h
> > index 8b154ed..54e5939 100644
> > --- a/include/erofs/internal.h
> > +++ b/include/erofs/internal.h
> > @@ -110,6 +110,7 @@ EROFS_FEATURE_FUNCS(lz4_0padding, incompat, INCOMPAT_LZ4_0PADDING)
> > EROFS_FEATURE_FUNCS(compr_cfgs, incompat, INCOMPAT_COMPR_CFGS)
> > EROFS_FEATURE_FUNCS(big_pcluster, incompat, INCOMPAT_BIG_PCLUSTER)
> > EROFS_FEATURE_FUNCS(chunked_file, incompat, INCOMPAT_CHUNKED_FILE)
> > +EROFS_FEATURE_FUNCS(tail_packing, incompat, INCOMPAT_TAIL_PACKING)
>
> How about moving fuse support as [PATCH 1/2] and introducing these?
>
> Also the on-disk feature name is somewhat confusing, maybe
> INCOMPAT_ZTAILPACKING
>
> is better?
ok, i will do that.
>
> > EROFS_FEATURE_FUNCS(sb_chksum, compat, COMPAT_SB_CHKSUM)
> >
> > #define EROFS_I_EA_INITED (1 << 0)
> > diff --git a/include/erofs_fs.h b/include/erofs_fs.h
> > index 66a68e3..0ebcd5b 100644
> > --- a/include/erofs_fs.h
> > +++ b/include/erofs_fs.h
> > @@ -22,11 +22,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_TAIL_PACKING 0x00000010
> > #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_TAIL_PACKING)
> >
> > #define EROFS_SB_EXTSLOT_SIZE 16
> >
> > @@ -266,13 +268,17 @@ struct z_erofs_lz4_cfgs {
> > * (4B) + 2B + (4B) if compacted 2B is on.
> > * bit 1 : HEAD1 big pcluster (0 - off; 1 - on)
> > * bit 2 : HEAD2 big pcluster (0 - off; 1 - on)
> > + * bit 3 : tail-packing inline (un)compressed data
> > */
> > #define Z_EROFS_ADVISE_COMPACTED_2B 0x0001
> > #define Z_EROFS_ADVISE_BIG_PCLUSTER_1 0x0002
> > #define Z_EROFS_ADVISE_BIG_PCLUSTER_2 0x0004
> > +#define Z_EROFS_ADVISE_INLINE_DATA 0x0008
> >
> > struct z_erofs_map_header {
> > - __le32 h_reserved1;
> > + __le16 h_reserved1;
> > + /* record the (un)compressed size of tail-packing pcluster */
> > + __le16 h_idata_size;
> > __le16 h_advise;
> > /*
> > * bit 0-3 : algorithm type of head 1 (logical cluster type 01);
> > diff --git a/lib/compress.c b/lib/compress.c
> > index 6ca5bed..d7d60b9 100644
> > --- a/lib/compress.c
> > +++ b/lib/compress.c
> > @@ -70,11 +70,10 @@ static void vle_write_indexes(struct z_erofs_vle_compress_ctx *ctx,
> >
> > di.di_clusterofs = cpu_to_le16(ctx->clusterofs);
> >
> > - /* whether the tail-end uncompressed block or not */
> > + /* whether the tail-end (un)compressed block or not */
> > if (!d1) {
> > - /* TODO: tail-packing inline compressed data */
> > - DBG_BUGON(!raw);
> > - type = Z_EROFS_VLE_CLUSTER_TYPE_PLAIN;
> > + type = raw ? Z_EROFS_VLE_CLUSTER_TYPE_PLAIN :
> > + Z_EROFS_VLE_CLUSTER_TYPE_HEAD;
> > advise = cpu_to_le16(type << Z_EROFS_VLE_DI_CLUSTER_TYPE_BIT);
> >
> > di.di_advise = advise;
> > @@ -162,6 +161,17 @@ static unsigned int z_erofs_get_max_pclusterblks(struct erofs_inode *inode)
> > return cfg.c_pclusterblks_def;
> > }
> >
> > +static int z_erofs_fill_inline_data(struct erofs_inode *inode, char *data,
> > + unsigned int len)
> > +{
> > + inode->idata_size = len;
> > + inode->idata = malloc(inode->idata_size);
> > + if (!inode->idata)
> > + return -ENOMEM;
> > + memcpy(inode->idata, data, inode->idata_size);
> > + return 0;
> > +}
> > +
> > static int vle_compress_one(struct erofs_inode *inode,
> > struct z_erofs_vle_compress_ctx *ctx,
> > bool final)
> > @@ -172,15 +182,20 @@ static int vle_compress_one(struct erofs_inode *inode,
> > int ret;
> > static char dstbuf[EROFS_CONFIG_COMPR_MAX_SZ + EROFS_BLKSIZ];
> > char *const dst = dstbuf + EROFS_BLKSIZ;
> > + bool tail_pcluster = false;
> >
> > while (len) {
> > - const unsigned int pclustersize =
> > + unsigned int pclustersize =
> > z_erofs_get_max_pclusterblks(inode) * EROFS_BLKSIZ;
> > bool raw;
> >
> > - if (len <= pclustersize) {
> > + if (!tail_pcluster && len <= pclustersize) {
> > if (final) {
> > - if (len <= EROFS_BLKSIZ)
> > + /* TODO: compress with 2 pclusters */
> > + if (erofs_sb_has_tail_packing()) {
> > + tail_pcluster = true;
> > + pclustersize = EROFS_BLKSIZ;
> > + } else if (len <= EROFS_BLKSIZ)
> > goto nocompression;
> > } else {
>
> It seems somewhat messy...
> Just a rough thought, how about the following code (not even tested...)
>
> --- a/lib/compress.c
> +++ b/lib/compress.c
> @@ -182,21 +182,22 @@ static int vle_compress_one(struct erofs_inode *inode,
> int ret;
> static char dstbuf[EROFS_CONFIG_COMPR_MAX_SZ + EROFS_BLKSIZ];
> char *const dst = dstbuf + EROFS_BLKSIZ;
> - bool tail_pcluster = false;
> + bool trailing = false;
>
> while (len) {
> unsigned int pclustersize =
> z_erofs_get_max_pclusterblks(inode) * EROFS_BLKSIZ;
> bool raw;
>
> - if (!tail_pcluster && len <= pclustersize) {
> + if (len <= pclustersize) {
> if (final) {
> /* TODO: compress with 2 pclusters */
> if (erofs_sb_has_tail_packing()) {
> - tail_pcluster = true;
> + trailing = true;
> pclustersize = EROFS_BLKSIZ;
> - } else if (len <= EROFS_BLKSIZ)
> + } else if (len <= EROFS_BLKSIZ) {
> goto nocompression;
> + }
> } else {
> break;
> }
> @@ -211,23 +212,29 @@ static int vle_compress_one(struct erofs_inode *inode,
> inode->i_srcpath,
> erofs_strerror(ret));
> }
> - if (tail_pcluster && len < EROFS_BLKSIZ) {
> + if (trailing && len < EROFS_BLKSIZ) {
> ret = z_erofs_fill_inline_data(inode,
> (char *)(ctx->queue + ctx->head), len);
> if (ret)
> return ret;
> count = len;
> raw = true;
> + ctx->compressedblks = 0;
> + } else {
> +nocompression:
> + ret = write_uncompressed_extent(ctx, &len, dst);
> + if (ret < 0)
> + return ret;
> + count = ret;
> ctx->compressedblks = 1;
> - goto add_head;
> + raw = true;
> }
> -nocompression:
> - ret = write_uncompressed_extent(ctx, &len, dst);
> - if (ret < 0)
> + } else if (trailing && ret < EROFS_BLKSIZ && len == count) {
> + ret = z_erofs_fill_inline_data(inode, dst, ret);
> + if (ret)
> return ret;
> - count = ret;
> - ctx->compressedblks = 1;
> - raw = true;
> + raw = false;
> + ctx->compressedblks = 0;
> } else {
> if (tail_pcluster && ret < EROFS_BLKSIZ &&
> !(len - count)) {
> @@ -262,7 +269,6 @@ nocompression:
> raw = false;
> }
>
> -add_head:
> ctx->head += count;
> /* write compression indexes for this pcluster */
> vle_write_indexes(ctx, count, raw);
>
> Thanks,
> Gao Xiang
Let me think above.
Thanks.
More information about the Linux-erofs
mailing list