* [RFC PATCH 0/2] erofs-utils: compression inline feature
@ 2021-10-25 12:30 Yue Hu
2021-10-25 12:30 ` [RFC PATCH 1/2] erofs-utils: support tail-packing inline compressed data Yue Hu
2021-10-25 12:30 ` [RFC PATCH 2/2] erofs-utils: fuse: " Yue Hu
0 siblings, 2 replies; 8+ messages in thread
From: Yue Hu @ 2021-10-25 12:30 UTC (permalink / raw)
To: linux-erofs; +Cc: huyue2, geshifei, zhangwen, shaojunjun
Now, we only support tail-end inline data for uncompressed file. We should
also support it for compressed file to decrease tail extent I/O and save
save more space. That is original intention from Xiang.
Note that, current mapping logic code in erofsuse is only based on
non-bigpcluster in erofsfuse, but i would like to present it. May refine
it for big pcluster later. Let's foucs on this on-disk improvement first.
Thanks.
Yue Hu (2):
erofs-utils: support tail-packing inline compressed data
erofs-utils: fuse: support tail-packing inline compressed data
include/erofs/internal.h | 2 +
include/erofs_fs.h | 6 ++-
lib/compress.c | 74 ++++++++++++++++++++++++-------
lib/compressor.c | 9 ++--
lib/decompress.c | 4 ++
lib/inode.c | 50 +++++++++++----------
lib/zmap.c | 95 +++++++++++++++++++++++++++++++++++++---
mkfs/main.c | 6 +++
8 files changed, 199 insertions(+), 47 deletions(-)
--
2.29.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* [RFC PATCH 1/2] erofs-utils: support tail-packing inline compressed data
2021-10-25 12:30 [RFC PATCH 0/2] erofs-utils: compression inline feature Yue Hu
@ 2021-10-25 12:30 ` Yue Hu
2021-10-26 12:00 ` Gao Xiang
2021-10-25 12:30 ` [RFC PATCH 2/2] erofs-utils: fuse: " Yue Hu
1 sibling, 1 reply; 8+ messages in thread
From: Yue Hu @ 2021-10-25 12:30 UTC (permalink / raw)
To: linux-erofs; +Cc: huyue2, geshifei, zhangwen, shaojunjun
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@yulong.com>
---
include/erofs/internal.h | 2 ++
include/erofs_fs.h | 6 +++-
lib/compress.c | 74 ++++++++++++++++++++++++++++++++--------
lib/compressor.c | 9 ++---
lib/decompress.c | 4 +++
lib/inode.c | 50 +++++++++++++++------------
mkfs/main.c | 6 ++++
7 files changed, 109 insertions(+), 42 deletions(-)
diff --git a/include/erofs/internal.h b/include/erofs/internal.h
index da7be56..42ae1ed 100644
--- a/include/erofs/internal.h
+++ b/include/erofs/internal.h
@@ -109,6 +109,7 @@ static inline void erofs_sb_clear_##name(void) \
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(tailpacking, incompat, INCOMPAT_TAILPACKING)
EROFS_FEATURE_FUNCS(sb_chksum, compat, COMPAT_SB_CHKSUM)
#define EROFS_I_EA_INITED (1 << 0)
@@ -148,6 +149,7 @@ struct erofs_inode {
unsigned char inode_isize;
/* inline tail-end packing size */
unsigned short idata_size;
+ bool idata_raw;
unsigned int xattr_isize;
unsigned int extent_isize;
diff --git a/include/erofs_fs.h b/include/erofs_fs.h
index 18fc182..7700b27 100644
--- a/include/erofs_fs.h
+++ b/include/erofs_fs.h
@@ -22,10 +22,12 @@
#define EROFS_FEATURE_INCOMPAT_LZ4_0PADDING 0x00000001
#define EROFS_FEATURE_INCOMPAT_COMPR_CFGS 0x00000002
#define EROFS_FEATURE_INCOMPAT_BIG_PCLUSTER 0x00000002
+#define EROFS_FEATURE_INCOMPAT_TAILPACKING 0x00000004
#define EROFS_ALL_FEATURE_INCOMPAT \
(EROFS_FEATURE_INCOMPAT_LZ4_0PADDING | \
EROFS_FEATURE_INCOMPAT_COMPR_CFGS | \
- EROFS_FEATURE_INCOMPAT_BIG_PCLUSTER)
+ EROFS_FEATURE_INCOMPAT_BIG_PCLUSTER | \
+ EROFS_FEATURE_INCOMPAT_TAILPACKING)
#define EROFS_SB_EXTSLOT_SIZE 16
@@ -230,10 +232,12 @@ 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 : 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_TAILPACKING 0x0008
struct z_erofs_map_header {
__le32 h_reserved1;
diff --git a/lib/compress.c b/lib/compress.c
index 2093bfd..c19f554 100644
--- a/lib/compress.c
+++ b/lib/compress.c
@@ -159,6 +159,18 @@ static unsigned int z_erofs_get_max_pclusterblks(struct erofs_inode *inode)
return cfg.c_physical_clusterblks;
}
+static int z_erofs_fill_inline_data(struct erofs_inode *inode, char *data,
+ unsigned int len, bool raw)
+{
+ inode->idata_raw = raw;
+ 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)
@@ -169,15 +181,19 @@ 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)
+ if (erofs_sb_has_tailpacking()) {
+ tail_pcluster = true;
+ pclustersize = EROFS_BLKSIZ;
+ } else if (len <= EROFS_BLKSIZ)
goto nocompression;
} else {
break;
@@ -194,6 +210,17 @@ static int vle_compress_one(struct erofs_inode *inode,
inode->i_srcpath,
erofs_strerror(ret));
}
+ if (tail_pcluster && len < EROFS_BLKSIZ) {
+ ret = z_erofs_fill_inline_data(inode,
+ (char *)(ctx->queue + ctx->head), len,
+ true);
+ if (ret)
+ return ret;
+ count = len;
+ raw = true;
+ ctx->compressedblks = 1;
+ goto add_head;
+ }
nocompression:
ret = write_uncompressed_extent(ctx, &len, dst);
if (ret < 0)
@@ -202,6 +229,16 @@ nocompression:
ctx->compressedblks = 1;
raw = true;
} else {
+ if (tail_pcluster && ret < EROFS_BLKSIZ &&
+ !(len - count)) {
+ ret = z_erofs_fill_inline_data(inode, dst, ret,
+ false);
+ if (ret)
+ return ret;
+ raw = false;
+ ctx->compressedblks = 1;
+ goto add_head;
+ }
const unsigned int tailused = ret & (EROFS_BLKSIZ - 1);
const unsigned int padding =
erofs_sb_has_lz4_0padding() && tailused ?
@@ -226,11 +263,13 @@ nocompression:
raw = false;
}
+add_head:
ctx->head += count;
/* write compression indexes for this pcluster */
vle_write_indexes(ctx, count, raw);
- ctx->blkaddr += ctx->compressedblks;
+ if (!inode->idata_size)
+ ctx->blkaddr += ctx->compressedblks;
len -= count;
if (!final && ctx->head >= EROFS_CONFIG_COMPR_MAX_SZ) {
@@ -475,7 +514,7 @@ int erofs_write_compressed_file(struct erofs_inode *inode)
fd = open(inode->i_srcpath, O_RDONLY | O_BINARY);
if (fd < 0) {
ret = -errno;
- goto err_free;
+ goto err_free_meta;
}
/* allocate main data buffer */
@@ -530,16 +569,19 @@ int erofs_write_compressed_file(struct erofs_inode *inode)
goto err_bdrop;
}
+ inode->idata_size = 0;
+
/* do the final round */
ret = vle_compress_one(inode, &ctx, true);
if (ret)
- goto err_bdrop;
+ goto err_free_id;
/* fall back to no compression mode */
compressed_blocks = ctx.blkaddr - blkaddr;
- if (compressed_blocks >= BLK_ROUND_UP(inode->i_size)) {
+ if (compressed_blocks >= BLK_ROUND_UP(inode->i_size) -
+ (inode->idata_size ? 1 : 0) ) {
ret = -ENOSPC;
- goto err_bdrop;
+ goto err_free_id;
}
vle_write_indexes_final(&ctx);
@@ -553,12 +595,11 @@ int erofs_write_compressed_file(struct erofs_inode *inode)
inode->i_srcpath, (unsigned long long)inode->i_size,
compressed_blocks);
- /*
- * TODO: need to move erofs_bdrop to erofs_write_tail_end
- * when both mkfs & kernel support compression inline.
- */
- erofs_bdrop(bh, false);
- inode->idata_size = 0;
+ if (inode->idata_size)
+ inode->bh_data = bh;
+ else
+ erofs_bdrop(bh, false);
+
inode->u.i_blocks = compressed_blocks;
legacymetasize = ctx.metacur - compressmeta;
@@ -573,11 +614,14 @@ int erofs_write_compressed_file(struct erofs_inode *inode)
inode->compressmeta = compressmeta;
return 0;
+err_free_id:
+ if (inode->idata)
+ free(inode->idata);
err_bdrop:
erofs_bdrop(bh, true); /* revoke buffer */
err_close:
close(fd);
-err_free:
+err_free_meta:
free(compressmeta);
return ret;
}
diff --git a/lib/compressor.c b/lib/compressor.c
index 8836e0c..26189b6 100644
--- a/lib/compressor.c
+++ b/lib/compressor.c
@@ -28,7 +28,7 @@ int erofs_compress_destsize(struct erofs_compress *c,
void *dst,
unsigned int dstsize)
{
- unsigned uncompressed_size;
+ unsigned compressed_size;
int ret;
DBG_BUGON(!c->alg);
@@ -41,9 +41,10 @@ int erofs_compress_destsize(struct erofs_compress *c,
return ret;
/* check if there is enough gains to compress */
- uncompressed_size = *srcsize;
- if (roundup(ret, EROFS_BLKSIZ) >= uncompressed_size *
- c->compress_threshold / 100)
+ compressed_size = *srcsize <= EROFS_BLKSIZ ? ret :
+ roundup(ret, EROFS_BLKSIZ);
+
+ if (*srcsize <= compressed_size * c->compress_threshold / 100)
return -EAGAIN;
return ret;
}
diff --git a/lib/decompress.c b/lib/decompress.c
index 490c4bc..0b6678d 100644
--- a/lib/decompress.c
+++ b/lib/decompress.c
@@ -9,6 +9,7 @@
#include "erofs/decompress.h"
#include "erofs/err.h"
+#include "erofs/print.h"
#ifdef LZ4_ENABLED
#include <lz4.h>
@@ -50,6 +51,9 @@ static int z_erofs_decompress_lz4(struct z_erofs_decompress_req *rq)
rq->decodedlength);
if (ret != (int)rq->decodedlength) {
+ erofs_err("failed to %s decompress %d in[%u, %u] out[%u]",
+ rq->partial_decoding ? "partial" : "full",
+ ret, rq->inputsize, inputmargin, rq->decodedlength);
ret = -EIO;
goto out;
}
diff --git a/lib/inode.c b/lib/inode.c
index 787e5b4..1c08608 100644
--- a/lib/inode.c
+++ b/lib/inode.c
@@ -521,9 +521,6 @@ int erofs_prepare_tail_block(struct erofs_inode *inode)
struct erofs_buffer_head *bh;
int ret;
- if (!inode->idata_size)
- return 0;
-
bh = inode->bh_data;
if (!bh) {
bh = erofs_balloc(DATA, EROFS_BLKSIZ, 0, 0);
@@ -557,26 +554,21 @@ int erofs_prepare_inode_buffer(struct erofs_inode *inode)
inodesize = Z_EROFS_VLE_EXTENT_ALIGN(inodesize) +
inode->extent_isize;
- if (is_inode_layout_compression(inode))
- goto noinline;
-
- /*
- * if the file size is block-aligned for uncompressed files,
- * should use EROFS_INODE_FLAT_PLAIN data mapping mode.
- */
if (!inode->idata_size)
- inode->datalayout = EROFS_INODE_FLAT_PLAIN;
+ goto noinline;
bh = erofs_balloc(INODE, inodesize, 0, inode->idata_size);
if (bh == ERR_PTR(-ENOSPC)) {
int ret;
- inode->datalayout = EROFS_INODE_FLAT_PLAIN;
-noinline:
/* expend an extra block for tail-end data */
ret = erofs_prepare_tail_block(inode);
if (ret)
return ret;
+noinline:
+ if (!is_inode_layout_compression(inode))
+ inode->datalayout = EROFS_INODE_FLAT_PLAIN;
+
bh = erofs_balloc(INODE, inodesize, 0, 0);
if (IS_ERR(bh))
return PTR_ERR(bh);
@@ -584,7 +576,16 @@ noinline:
} else if (IS_ERR(bh)) {
return PTR_ERR(bh);
} else if (inode->idata_size) {
- inode->datalayout = EROFS_INODE_FLAT_INLINE;
+ if (is_inode_layout_compression(inode)) {
+ struct z_erofs_map_header *h = inode->compressmeta;
+ h->h_advise |= Z_EROFS_ADVISE_TAILPACKING;
+ erofs_dbg("%s: inline %scompressed data (%u bytes)",
+ inode->i_srcpath,
+ inode->idata_raw ? "un" : "",
+ inode->idata_size);
+ } else {
+ inode->datalayout = EROFS_INODE_FLAT_INLINE;
+ }
/* allocate inline buffer */
ibh = erofs_battach(bh, META, inode->idata_size);
@@ -640,20 +641,25 @@ int erofs_write_tail_end(struct erofs_inode *inode)
ibh->op = &erofs_write_inline_bhops;
} else {
int ret;
- erofs_off_t pos;
+ erofs_off_t pos, pos0;
+ const unsigned short padding = EROFS_BLKSIZ - inode->idata_size;
erofs_mapbh(bh->block);
pos = erofs_btell(bh, true) - EROFS_BLKSIZ;
+ pos0 = pos + inode->idata_size;
+
+ if (is_inode_layout_compression(inode) &&
+ erofs_sb_has_lz4_0padding() && !inode->idata_raw) {
+ pos0 = pos;
+ pos += padding;
+ }
ret = dev_write(inode->idata, pos, inode->idata_size);
if (ret)
return ret;
- if (inode->idata_size < EROFS_BLKSIZ) {
- ret = dev_fillzero(pos + inode->idata_size,
- EROFS_BLKSIZ - inode->idata_size,
- false);
- if (ret)
- return ret;
- }
+ ret = dev_fillzero(pos0, padding, false);
+ if (ret)
+ return ret;
+
inode->idata_size = 0;
free(inode->idata);
inode->idata = NULL;
diff --git a/mkfs/main.c b/mkfs/main.c
index e476189..7dd29df 100644
--- a/mkfs/main.c
+++ b/mkfs/main.c
@@ -48,6 +48,7 @@ static struct option long_options[] = {
{"product-out", required_argument, NULL, 11},
{"fs-config-file", required_argument, NULL, 12},
#endif
+ {"inline", no_argument, NULL, 13},
{0, 0, 0, 0},
};
@@ -87,6 +88,7 @@ static void usage(void)
" --all-root make all files owned by root\n"
" --help display this help and exit\n"
" --max-extent-bytes=# set maximum decompressed extent size # in bytes\n"
+ " --inline tail-packing inline compressed data\n"
#ifndef NDEBUG
" --random-pclusterblks randomize pclusterblks for big pcluster (debugging only)\n"
#endif
@@ -304,6 +306,10 @@ static int mkfs_parse_options_cfg(int argc, char *argv[])
}
cfg.c_physical_clusterblks = i / EROFS_BLKSIZ;
break;
+ case 13:
+ erofs_sb_set_tailpacking();
+ erofs_warn("EXPERIMENTAL compression inline feature in use. Use at your own risk!");
+ break;
case 1:
usage();
--
2.29.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [RFC PATCH 2/2] erofs-utils: fuse: support tail-packing inline compressed data
2021-10-25 12:30 [RFC PATCH 0/2] erofs-utils: compression inline feature Yue Hu
2021-10-25 12:30 ` [RFC PATCH 1/2] erofs-utils: support tail-packing inline compressed data Yue Hu
@ 2021-10-25 12:30 ` Yue Hu
1 sibling, 0 replies; 8+ messages in thread
From: Yue Hu @ 2021-10-25 12:30 UTC (permalink / raw)
To: linux-erofs; +Cc: huyue2, geshifei, zhangwen, shaojunjun
Add tail-packing inline compressed data support for erofsfuse.
Signed-off-by: Yue Hu <huyue2@yulong.com>
---
lib/zmap.c | 95 +++++++++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 90 insertions(+), 5 deletions(-)
diff --git a/lib/zmap.c b/lib/zmap.c
index 1084faa..de79f03 100644
--- a/lib/zmap.c
+++ b/lib/zmap.c
@@ -442,6 +442,66 @@ err_bonus_cblkcnt:
return -EFSCORRUPTED;
}
+static erofs_off_t compacted_inline_data_addr(struct erofs_inode *vi)
+{
+ const erofs_off_t ebase = round_up(iloc(vi->nid) + vi->inode_isize +
+ vi->xattr_isize, 8) +
+ sizeof(struct z_erofs_map_header);
+ const unsigned int totalidx = DIV_ROUND_UP(vi->i_size, EROFS_BLKSIZ);
+ unsigned int compacted_4b_initial, compacted_4b_end;
+ unsigned int compacted_2b;
+ erofs_off_t addr;
+
+ compacted_4b_initial = (32 - ebase % 32) / 4;
+ if (compacted_4b_initial == 32 / 4)
+ compacted_4b_initial = 0;
+
+ if (compacted_4b_initial > totalidx) {
+ compacted_4b_initial = 0;
+ compacted_2b = 0;
+ } else if (vi->z_advise & Z_EROFS_ADVISE_COMPACTED_2B) {
+ compacted_2b = rounddown(totalidx - compacted_4b_initial, 16);
+ } else
+ compacted_2b = 0;
+
+ compacted_4b_end = totalidx - compacted_4b_initial - compacted_2b;
+
+ addr = ebase;
+ addr += compacted_4b_initial * 4;
+ addr += compacted_2b * 2;
+ if (compacted_4b_end > 1)
+ addr += (compacted_4b_end/2) * 8;
+ if (compacted_4b_end % 2)
+ addr += 8;
+
+ return addr;
+}
+
+static erofs_off_t legacy_inline_data_addr(struct erofs_inode *vi)
+{
+ const erofs_off_t ibase = iloc(vi->nid);
+ const unsigned int totalidx = DIV_ROUND_UP(vi->i_size, EROFS_BLKSIZ);
+ erofs_off_t addr;
+
+ addr = Z_EROFS_VLE_LEGACY_INDEX_ALIGN(ibase + vi->inode_isize +
+ vi->xattr_isize) +
+ totalidx * sizeof(struct z_erofs_vle_decompressed_index);
+ return addr;
+}
+
+static erofs_off_t z_erofs_inline_data_addr(struct erofs_inode *vi)
+{
+ const unsigned int datamode = vi->datalayout;
+
+ if (datamode == EROFS_INODE_FLAT_COMPRESSION)
+ return compacted_inline_data_addr(vi);
+
+ if (datamode == EROFS_INODE_FLAT_COMPRESSION_LEGACY)
+ return legacy_inline_data_addr(vi);
+
+ return -EINVAL;
+}
+
int z_erofs_map_blocks_iter(struct erofs_inode *vi,
struct erofs_map_blocks *map)
{
@@ -454,6 +514,7 @@ int z_erofs_map_blocks_iter(struct erofs_inode *vi,
unsigned int lclusterbits, endoff;
unsigned long initial_lcn;
unsigned long long ofs, end;
+ bool tailpacking;
/* when trying to read beyond EOF, leave it unmapped */
if (map->m_la >= vi->i_size) {
@@ -467,6 +528,8 @@ int z_erofs_map_blocks_iter(struct erofs_inode *vi,
if (err)
goto out;
+ tailpacking = vi->z_advise & Z_EROFS_ADVISE_TAILPACKING;
+
lclusterbits = vi->z_logical_clusterbits;
ofs = map->m_la;
initial_lcn = ofs >> lclusterbits;
@@ -478,6 +541,9 @@ int z_erofs_map_blocks_iter(struct erofs_inode *vi,
map->m_flags = EROFS_MAP_ZIPPED; /* by default, compressed */
end = (m.lcn + 1ULL) << lclusterbits;
+
+ map->m_flags &= ~EROFS_MAP_META;
+
switch (m.type) {
case Z_EROFS_VLE_CLUSTER_TYPE_PLAIN:
if (endoff >= m.clusterofs)
@@ -485,6 +551,16 @@ int z_erofs_map_blocks_iter(struct erofs_inode *vi,
case Z_EROFS_VLE_CLUSTER_TYPE_HEAD:
if (endoff >= m.clusterofs) {
map->m_la = (m.lcn << lclusterbits) | m.clusterofs;
+ if (tailpacking &&
+ end == round_up(vi->i_size, 1<<lclusterbits)) {
+ end = vi->i_size;
+ map->m_flags |= EROFS_MAP_META;
+ }
+ if (tailpacking &&
+ end == round_down(vi->i_size, 1<<lclusterbits) &&
+ vi->i_size - map->m_la <= EROFS_BLKSIZ) {
+ map->m_flags |= EROFS_MAP_META;
+ }
break;
}
/* m.lcn should be >= 1 if endoff < m.clusterofs */
@@ -495,7 +571,10 @@ int z_erofs_map_blocks_iter(struct erofs_inode *vi,
goto out;
}
end = (m.lcn << lclusterbits) | m.clusterofs;
- map->m_flags |= EROFS_MAP_FULL_MAPPED;
+ if (tailpacking && end == vi->i_size)
+ map->m_flags |= EROFS_MAP_META;
+ else
+ map->m_flags |= EROFS_MAP_FULL_MAPPED;
m.delta[0] = 1;
case Z_EROFS_VLE_CLUSTER_TYPE_NONHEAD:
/* get the correspoinding first chunk */
@@ -511,11 +590,17 @@ int z_erofs_map_blocks_iter(struct erofs_inode *vi,
}
map->m_llen = end - map->m_la;
- map->m_pa = blknr_to_addr(m.pblk);
- err = z_erofs_get_extent_compressedlen(&m, initial_lcn);
- if (err)
- goto out;
+ if (map->m_flags & EROFS_MAP_META) {
+ map->m_pa = z_erofs_inline_data_addr(vi);
+ map->m_plen = EROFS_BLKSIZ;
+ } else {
+ map->m_pa = blknr_to_addr(m.pblk);
+
+ err = z_erofs_get_extent_compressedlen(&m, initial_lcn);
+ if (err)
+ goto out;
+ }
map->m_flags |= EROFS_MAP_MAPPED;
out:
--
2.29.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [RFC PATCH 1/2] erofs-utils: support tail-packing inline compressed data
2021-10-25 12:30 ` [RFC PATCH 1/2] erofs-utils: support tail-packing inline compressed data Yue Hu
@ 2021-10-26 12:00 ` Gao Xiang
2021-10-27 1:58 ` Yue Hu
0 siblings, 1 reply; 8+ messages in thread
From: Gao Xiang @ 2021-10-26 12:00 UTC (permalink / raw)
To: Yue Hu; +Cc: geshifei, zhangwen, linux-erofs, shaojunjun
Hi Yue,
On Mon, Oct 25, 2021 at 08:30:43PM +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@yulong.com>
The patch roughly looks good to me. I'll play with it this week later.
Some comments inline:
> ---
> include/erofs/internal.h | 2 ++
> include/erofs_fs.h | 6 +++-
> lib/compress.c | 74 ++++++++++++++++++++++++++++++++--------
> lib/compressor.c | 9 ++---
> lib/decompress.c | 4 +++
> lib/inode.c | 50 +++++++++++++++------------
> mkfs/main.c | 6 ++++
> 7 files changed, 109 insertions(+), 42 deletions(-)
>
> diff --git a/include/erofs/internal.h b/include/erofs/internal.h
> index da7be56..42ae1ed 100644
> --- a/include/erofs/internal.h
> +++ b/include/erofs/internal.h
> @@ -109,6 +109,7 @@ static inline void erofs_sb_clear_##name(void) \
> 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(tailpacking, incompat, INCOMPAT_TAILPACKING)
> EROFS_FEATURE_FUNCS(sb_chksum, compat, COMPAT_SB_CHKSUM)
>
> #define EROFS_I_EA_INITED (1 << 0)
> @@ -148,6 +149,7 @@ struct erofs_inode {
> unsigned char inode_isize;
> /* inline tail-end packing size */
> unsigned short idata_size;
> + bool idata_raw;
>
> unsigned int xattr_isize;
> unsigned int extent_isize;
> diff --git a/include/erofs_fs.h b/include/erofs_fs.h
> index 18fc182..7700b27 100644
> --- a/include/erofs_fs.h
> +++ b/include/erofs_fs.h
> @@ -22,10 +22,12 @@
> #define EROFS_FEATURE_INCOMPAT_LZ4_0PADDING 0x00000001
> #define EROFS_FEATURE_INCOMPAT_COMPR_CFGS 0x00000002
> #define EROFS_FEATURE_INCOMPAT_BIG_PCLUSTER 0x00000002
> +#define EROFS_FEATURE_INCOMPAT_TAILPACKING 0x00000004
> #define EROFS_ALL_FEATURE_INCOMPAT \
> (EROFS_FEATURE_INCOMPAT_LZ4_0PADDING | \
> EROFS_FEATURE_INCOMPAT_COMPR_CFGS | \
> - EROFS_FEATURE_INCOMPAT_BIG_PCLUSTER)
> + EROFS_FEATURE_INCOMPAT_BIG_PCLUSTER | \
> + EROFS_FEATURE_INCOMPAT_TAILPACKING)
>
> #define EROFS_SB_EXTSLOT_SIZE 16
>
> @@ -230,10 +232,12 @@ 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 : 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_TAILPACKING 0x0008
>
> struct z_erofs_map_header {
> __le32 h_reserved1;
> diff --git a/lib/compress.c b/lib/compress.c
> index 2093bfd..c19f554 100644
> --- a/lib/compress.c
> +++ b/lib/compress.c
> @@ -159,6 +159,18 @@ static unsigned int z_erofs_get_max_pclusterblks(struct erofs_inode *inode)
> return cfg.c_physical_clusterblks;
> }
>
> +static int z_erofs_fill_inline_data(struct erofs_inode *inode, char *data,
> + unsigned int len, bool raw)
> +{
> + inode->idata_raw = raw;
> + 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)
> @@ -169,15 +181,19 @@ 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)
> + if (erofs_sb_has_tailpacking()) {
> + tail_pcluster = true;
> + pclustersize = EROFS_BLKSIZ;
Not quite sure if such condition can be trigged for many times...
Think about it. If the original pclustersize == 16 * EROFS_BLKSIZ, so we
could have at least 16 new pclustersize == EROFS_BLKSIZ then?
But only the last pclustersize == EROFS_BLKSIZ can be inlined...
> + } else if (len <= EROFS_BLKSIZ)
> goto nocompression;
> } else {
> break;
> @@ -194,6 +210,17 @@ static int vle_compress_one(struct erofs_inode *inode,
> inode->i_srcpath,
> erofs_strerror(ret));
> }
> + if (tail_pcluster && len < EROFS_BLKSIZ) {
> + ret = z_erofs_fill_inline_data(inode,
> + (char *)(ctx->queue + ctx->head), len,
> + true);
> + if (ret)
> + return ret;
> + count = len;
> + raw = true;
> + ctx->compressedblks = 1;
> + goto add_head;
> + }
> nocompression:
> ret = write_uncompressed_extent(ctx, &len, dst);
> if (ret < 0)
> @@ -202,6 +229,16 @@ nocompression:
> ctx->compressedblks = 1;
> raw = true;
> } else {
> + if (tail_pcluster && ret < EROFS_BLKSIZ &&
> + !(len - count)) {
> + ret = z_erofs_fill_inline_data(inode, dst, ret,
> + false);
> + if (ret)
> + return ret;
> + raw = false;
> + ctx->compressedblks = 1;
> + goto add_head;
> + }
> const unsigned int tailused = ret & (EROFS_BLKSIZ - 1);
> const unsigned int padding =
> erofs_sb_has_lz4_0padding() && tailused ?
> @@ -226,11 +263,13 @@ nocompression:
> raw = false;
> }
>
> +add_head:
> ctx->head += count;
> /* write compression indexes for this pcluster */
> vle_write_indexes(ctx, count, raw);
>
> - ctx->blkaddr += ctx->compressedblks;
> + if (!inode->idata_size)
> + ctx->blkaddr += ctx->compressedblks;
> len -= count;
>
> if (!final && ctx->head >= EROFS_CONFIG_COMPR_MAX_SZ) {
> @@ -475,7 +514,7 @@ int erofs_write_compressed_file(struct erofs_inode *inode)
> fd = open(inode->i_srcpath, O_RDONLY | O_BINARY);
> if (fd < 0) {
> ret = -errno;
> - goto err_free;
> + goto err_free_meta;
> }
>
> /* allocate main data buffer */
> @@ -530,16 +569,19 @@ int erofs_write_compressed_file(struct erofs_inode *inode)
> goto err_bdrop;
> }
>
> + inode->idata_size = 0;
> +
> /* do the final round */
> ret = vle_compress_one(inode, &ctx, true);
> if (ret)
> - goto err_bdrop;
> + goto err_free_id;
>
> /* fall back to no compression mode */
> compressed_blocks = ctx.blkaddr - blkaddr;
> - if (compressed_blocks >= BLK_ROUND_UP(inode->i_size)) {
> + if (compressed_blocks >= BLK_ROUND_UP(inode->i_size) -
> + (inode->idata_size ? 1 : 0) ) {
> ret = -ENOSPC;
> - goto err_bdrop;
> + goto err_free_id;
> }
>
> vle_write_indexes_final(&ctx);
> @@ -553,12 +595,11 @@ int erofs_write_compressed_file(struct erofs_inode *inode)
> inode->i_srcpath, (unsigned long long)inode->i_size,
> compressed_blocks);
>
> - /*
> - * TODO: need to move erofs_bdrop to erofs_write_tail_end
> - * when both mkfs & kernel support compression inline.
> - */
> - erofs_bdrop(bh, false);
> - inode->idata_size = 0;
> + if (inode->idata_size)
> + inode->bh_data = bh;
> + else
> + erofs_bdrop(bh, false);
> +
> inode->u.i_blocks = compressed_blocks;
>
> legacymetasize = ctx.metacur - compressmeta;
> @@ -573,11 +614,14 @@ int erofs_write_compressed_file(struct erofs_inode *inode)
> inode->compressmeta = compressmeta;
> return 0;
>
> +err_free_id:
> + if (inode->idata)
> + free(inode->idata);
> err_bdrop:
> erofs_bdrop(bh, true); /* revoke buffer */
> err_close:
> close(fd);
> -err_free:
> +err_free_meta:
> free(compressmeta);
> return ret;
> }
> diff --git a/lib/compressor.c b/lib/compressor.c
> index 8836e0c..26189b6 100644
> --- a/lib/compressor.c
> +++ b/lib/compressor.c
> @@ -28,7 +28,7 @@ int erofs_compress_destsize(struct erofs_compress *c,
> void *dst,
> unsigned int dstsize)
> {
> - unsigned uncompressed_size;
> + unsigned compressed_size;
> int ret;
>
> DBG_BUGON(!c->alg);
> @@ -41,9 +41,10 @@ int erofs_compress_destsize(struct erofs_compress *c,
> return ret;
>
> /* check if there is enough gains to compress */
> - uncompressed_size = *srcsize;
> - if (roundup(ret, EROFS_BLKSIZ) >= uncompressed_size *
> - c->compress_threshold / 100)
> + compressed_size = *srcsize <= EROFS_BLKSIZ ? ret :
> + roundup(ret, EROFS_BLKSIZ);
> +
> + if (*srcsize <= compressed_size * c->compress_threshold / 100)
> return -EAGAIN;
> return ret;
> }
> diff --git a/lib/decompress.c b/lib/decompress.c
> index 490c4bc..0b6678d 100644
> --- a/lib/decompress.c
> +++ b/lib/decompress.c
> @@ -9,6 +9,7 @@
>
> #include "erofs/decompress.h"
> #include "erofs/err.h"
> +#include "erofs/print.h"
>
> #ifdef LZ4_ENABLED
> #include <lz4.h>
> @@ -50,6 +51,9 @@ static int z_erofs_decompress_lz4(struct z_erofs_decompress_req *rq)
> rq->decodedlength);
>
> if (ret != (int)rq->decodedlength) {
> + erofs_err("failed to %s decompress %d in[%u, %u] out[%u]",
> + rq->partial_decoding ? "partial" : "full",
> + ret, rq->inputsize, inputmargin, rq->decodedlength);
> ret = -EIO;
> goto out;
> }
> diff --git a/lib/inode.c b/lib/inode.c
> index 787e5b4..1c08608 100644
> --- a/lib/inode.c
> +++ b/lib/inode.c
> @@ -521,9 +521,6 @@ int erofs_prepare_tail_block(struct erofs_inode *inode)
> struct erofs_buffer_head *bh;
> int ret;
>
> - if (!inode->idata_size)
> - return 0;
> -
> bh = inode->bh_data;
> if (!bh) {
> bh = erofs_balloc(DATA, EROFS_BLKSIZ, 0, 0);
> @@ -557,26 +554,21 @@ int erofs_prepare_inode_buffer(struct erofs_inode *inode)
> inodesize = Z_EROFS_VLE_EXTENT_ALIGN(inodesize) +
> inode->extent_isize;
>
> - if (is_inode_layout_compression(inode))
> - goto noinline;
> -
> - /*
> - * if the file size is block-aligned for uncompressed files,
> - * should use EROFS_INODE_FLAT_PLAIN data mapping mode.
> - */
> if (!inode->idata_size)
> - inode->datalayout = EROFS_INODE_FLAT_PLAIN;
> + goto noinline;
>
> bh = erofs_balloc(INODE, inodesize, 0, inode->idata_size);
> if (bh == ERR_PTR(-ENOSPC)) {
> int ret;
>
> - inode->datalayout = EROFS_INODE_FLAT_PLAIN;
> -noinline:
> /* expend an extra block for tail-end data */
> ret = erofs_prepare_tail_block(inode);
> if (ret)
> return ret;
> +noinline:
> + if (!is_inode_layout_compression(inode))
> + inode->datalayout = EROFS_INODE_FLAT_PLAIN;
> +
> bh = erofs_balloc(INODE, inodesize, 0, 0);
> if (IS_ERR(bh))
> return PTR_ERR(bh);
> @@ -584,7 +576,16 @@ noinline:
> } else if (IS_ERR(bh)) {
> return PTR_ERR(bh);
> } else if (inode->idata_size) {
> - inode->datalayout = EROFS_INODE_FLAT_INLINE;
> + if (is_inode_layout_compression(inode)) {
> + struct z_erofs_map_header *h = inode->compressmeta;
> + h->h_advise |= Z_EROFS_ADVISE_TAILPACKING;
> + erofs_dbg("%s: inline %scompressed data (%u bytes)",
> + inode->i_srcpath,
> + inode->idata_raw ? "un" : "",
> + inode->idata_size);
> + } else {
> + inode->datalayout = EROFS_INODE_FLAT_INLINE;
> + }
>
> /* allocate inline buffer */
> ibh = erofs_battach(bh, META, inode->idata_size);
> @@ -640,20 +641,25 @@ int erofs_write_tail_end(struct erofs_inode *inode)
> ibh->op = &erofs_write_inline_bhops;
> } else {
> int ret;
> - erofs_off_t pos;
> + erofs_off_t pos, pos0;
> + const unsigned short padding = EROFS_BLKSIZ - inode->idata_size;
>
> erofs_mapbh(bh->block);
> pos = erofs_btell(bh, true) - EROFS_BLKSIZ;
> + pos0 = pos + inode->idata_size;
> +
> + if (is_inode_layout_compression(inode) &&
> + erofs_sb_has_lz4_0padding() && !inode->idata_raw) {
> + pos0 = pos;
> + pos += padding;
> + }
I'd suggest don't use 0padding but record compressed_sized for
tail-packing pcluster in the map header.... (need 2 bytes to record)
Thanks,
Gao Xiang
> ret = dev_write(inode->idata, pos, inode->idata_size);
> if (ret)
> return ret;
> - if (inode->idata_size < EROFS_BLKSIZ) {
> - ret = dev_fillzero(pos + inode->idata_size,
> - EROFS_BLKSIZ - inode->idata_size,
> - false);
> - if (ret)
> - return ret;
> - }
> + ret = dev_fillzero(pos0, padding, false);
> + if (ret)
> + return ret;
> +
> inode->idata_size = 0;
> free(inode->idata);
> inode->idata = NULL;
> diff --git a/mkfs/main.c b/mkfs/main.c
> index e476189..7dd29df 100644
> --- a/mkfs/main.c
> +++ b/mkfs/main.c
> @@ -48,6 +48,7 @@ static struct option long_options[] = {
> {"product-out", required_argument, NULL, 11},
> {"fs-config-file", required_argument, NULL, 12},
> #endif
> + {"inline", no_argument, NULL, 13},
> {0, 0, 0, 0},
> };
>
> @@ -87,6 +88,7 @@ static void usage(void)
> " --all-root make all files owned by root\n"
> " --help display this help and exit\n"
> " --max-extent-bytes=# set maximum decompressed extent size # in bytes\n"
> + " --inline tail-packing inline compressed data\n"
> #ifndef NDEBUG
> " --random-pclusterblks randomize pclusterblks for big pcluster (debugging only)\n"
> #endif
> @@ -304,6 +306,10 @@ static int mkfs_parse_options_cfg(int argc, char *argv[])
> }
> cfg.c_physical_clusterblks = i / EROFS_BLKSIZ;
> break;
> + case 13:
> + erofs_sb_set_tailpacking();
> + erofs_warn("EXPERIMENTAL compression inline feature in use. Use at your own risk!");
> + break;
>
> case 1:
> usage();
> --
> 2.29.0
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH 1/2] erofs-utils: support tail-packing inline compressed data
2021-10-26 12:00 ` Gao Xiang
@ 2021-10-27 1:58 ` Yue Hu
2021-10-27 7:21 ` Yue Hu
0 siblings, 1 reply; 8+ messages in thread
From: Yue Hu @ 2021-10-27 1:58 UTC (permalink / raw)
To: Gao Xiang; +Cc: linux-erofs, Yue Hu, geshifei, zhangwen, shaojunjun
Hi Xiang,
On Tue, 26 Oct 2021 20:00:37 +0800
Gao Xiang <hsiangkao@linux.alibaba.com> wrote:
> Hi Yue,
>
> On Mon, Oct 25, 2021 at 08:30:43PM +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@yulong.com>
>
> The patch roughly looks good to me. I'll play with it this week later.
>
> Some comments inline:
>
> > ---
> > include/erofs/internal.h | 2 ++
> > include/erofs_fs.h | 6 +++-
> > lib/compress.c | 74 ++++++++++++++++++++++++++++++++--------
> > lib/compressor.c | 9 ++---
> > lib/decompress.c | 4 +++
> > lib/inode.c | 50 +++++++++++++++------------
> > mkfs/main.c | 6 ++++
> > 7 files changed, 109 insertions(+), 42 deletions(-)
> >
> > diff --git a/include/erofs/internal.h b/include/erofs/internal.h
> > index da7be56..42ae1ed 100644
> > --- a/include/erofs/internal.h
> > +++ b/include/erofs/internal.h
> > @@ -109,6 +109,7 @@ static inline void erofs_sb_clear_##name(void) \
> > 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(tailpacking, incompat, INCOMPAT_TAILPACKING)
> > EROFS_FEATURE_FUNCS(sb_chksum, compat, COMPAT_SB_CHKSUM)
> >
> > #define EROFS_I_EA_INITED (1 << 0)
> > @@ -148,6 +149,7 @@ struct erofs_inode {
> > unsigned char inode_isize;
> > /* inline tail-end packing size */
> > unsigned short idata_size;
> > + bool idata_raw;
> >
> > unsigned int xattr_isize;
> > unsigned int extent_isize;
> > diff --git a/include/erofs_fs.h b/include/erofs_fs.h
> > index 18fc182..7700b27 100644
> > --- a/include/erofs_fs.h
> > +++ b/include/erofs_fs.h
> > @@ -22,10 +22,12 @@
> > #define EROFS_FEATURE_INCOMPAT_LZ4_0PADDING 0x00000001
> > #define EROFS_FEATURE_INCOMPAT_COMPR_CFGS 0x00000002
> > #define EROFS_FEATURE_INCOMPAT_BIG_PCLUSTER 0x00000002
> > +#define EROFS_FEATURE_INCOMPAT_TAILPACKING 0x00000004
> > #define EROFS_ALL_FEATURE_INCOMPAT \
> > (EROFS_FEATURE_INCOMPAT_LZ4_0PADDING | \
> > EROFS_FEATURE_INCOMPAT_COMPR_CFGS | \
> > - EROFS_FEATURE_INCOMPAT_BIG_PCLUSTER)
> > + EROFS_FEATURE_INCOMPAT_BIG_PCLUSTER | \
> > + EROFS_FEATURE_INCOMPAT_TAILPACKING)
> >
> > #define EROFS_SB_EXTSLOT_SIZE 16
> >
> > @@ -230,10 +232,12 @@ 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 : 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_TAILPACKING 0x0008
> >
> > struct z_erofs_map_header {
> > __le32 h_reserved1;
> > diff --git a/lib/compress.c b/lib/compress.c
> > index 2093bfd..c19f554 100644
> > --- a/lib/compress.c
> > +++ b/lib/compress.c
> > @@ -159,6 +159,18 @@ static unsigned int z_erofs_get_max_pclusterblks(struct erofs_inode *inode)
> > return cfg.c_physical_clusterblks;
> > }
> >
> > +static int z_erofs_fill_inline_data(struct erofs_inode *inode, char *data,
> > + unsigned int len, bool raw)
> > +{
> > + inode->idata_raw = raw;
> > + 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)
> > @@ -169,15 +181,19 @@ 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)
> > + if (erofs_sb_has_tailpacking()) {
> > + tail_pcluster = true;
> > + pclustersize = EROFS_BLKSIZ;
>
> Not quite sure if such condition can be trigged for many times...
>
> Think about it. If the original pclustersize == 16 * EROFS_BLKSIZ, so we
> could have at least 16 new pclustersize == EROFS_BLKSIZ then?
>
> But only the last pclustersize == EROFS_BLKSIZ can be inlined...
Let me think about it more.
>
> > + } else if (len <= EROFS_BLKSIZ)
> > goto nocompression;
> > } else {
> > break;
> > @@ -194,6 +210,17 @@ static int vle_compress_one(struct erofs_inode *inode,
> > inode->i_srcpath,
> > erofs_strerror(ret));
> > }
> > + if (tail_pcluster && len < EROFS_BLKSIZ) {
> > + ret = z_erofs_fill_inline_data(inode,
> > + (char *)(ctx->queue + ctx->head), len,
> > + true);
> > + if (ret)
> > + return ret;
> > + count = len;
> > + raw = true;
> > + ctx->compressedblks = 1;
> > + goto add_head;
> > + }
> > nocompression:
> > ret = write_uncompressed_extent(ctx, &len, dst);
> > if (ret < 0)
> > @@ -202,6 +229,16 @@ nocompression:
> > ctx->compressedblks = 1;
> > raw = true;
> > } else {
> > + if (tail_pcluster && ret < EROFS_BLKSIZ &&
> > + !(len - count)) {
> > + ret = z_erofs_fill_inline_data(inode, dst, ret,
> > + false);
> > + if (ret)
> > + return ret;
> > + raw = false;
> > + ctx->compressedblks = 1;
> > + goto add_head;
> > + }
> > const unsigned int tailused = ret & (EROFS_BLKSIZ - 1);
> > const unsigned int padding =
> > erofs_sb_has_lz4_0padding() && tailused ?
> > @@ -226,11 +263,13 @@ nocompression:
> > raw = false;
> > }
> >
> > +add_head:
> > ctx->head += count;
> > /* write compression indexes for this pcluster */
> > vle_write_indexes(ctx, count, raw);
> >
> > - ctx->blkaddr += ctx->compressedblks;
> > + if (!inode->idata_size)
> > + ctx->blkaddr += ctx->compressedblks;
> > len -= count;
> >
> > if (!final && ctx->head >= EROFS_CONFIG_COMPR_MAX_SZ) {
> > @@ -475,7 +514,7 @@ int erofs_write_compressed_file(struct erofs_inode *inode)
> > fd = open(inode->i_srcpath, O_RDONLY | O_BINARY);
> > if (fd < 0) {
> > ret = -errno;
> > - goto err_free;
> > + goto err_free_meta;
> > }
> >
> > /* allocate main data buffer */
> > @@ -530,16 +569,19 @@ int erofs_write_compressed_file(struct erofs_inode *inode)
> > goto err_bdrop;
> > }
> >
> > + inode->idata_size = 0;
> > +
> > /* do the final round */
> > ret = vle_compress_one(inode, &ctx, true);
> > if (ret)
> > - goto err_bdrop;
> > + goto err_free_id;
> >
> > /* fall back to no compression mode */
> > compressed_blocks = ctx.blkaddr - blkaddr;
> > - if (compressed_blocks >= BLK_ROUND_UP(inode->i_size)) {
> > + if (compressed_blocks >= BLK_ROUND_UP(inode->i_size) -
> > + (inode->idata_size ? 1 : 0) ) {
> > ret = -ENOSPC;
> > - goto err_bdrop;
> > + goto err_free_id;
> > }
> >
> > vle_write_indexes_final(&ctx);
> > @@ -553,12 +595,11 @@ int erofs_write_compressed_file(struct erofs_inode *inode)
> > inode->i_srcpath, (unsigned long long)inode->i_size,
> > compressed_blocks);
> >
> > - /*
> > - * TODO: need to move erofs_bdrop to erofs_write_tail_end
> > - * when both mkfs & kernel support compression inline.
> > - */
> > - erofs_bdrop(bh, false);
> > - inode->idata_size = 0;
> > + if (inode->idata_size)
> > + inode->bh_data = bh;
> > + else
> > + erofs_bdrop(bh, false);
> > +
> > inode->u.i_blocks = compressed_blocks;
> >
> > legacymetasize = ctx.metacur - compressmeta;
> > @@ -573,11 +614,14 @@ int erofs_write_compressed_file(struct erofs_inode *inode)
> > inode->compressmeta = compressmeta;
> > return 0;
> >
> > +err_free_id:
> > + if (inode->idata)
> > + free(inode->idata);
> > err_bdrop:
> > erofs_bdrop(bh, true); /* revoke buffer */
> > err_close:
> > close(fd);
> > -err_free:
> > +err_free_meta:
> > free(compressmeta);
> > return ret;
> > }
> > diff --git a/lib/compressor.c b/lib/compressor.c
> > index 8836e0c..26189b6 100644
> > --- a/lib/compressor.c
> > +++ b/lib/compressor.c
> > @@ -28,7 +28,7 @@ int erofs_compress_destsize(struct erofs_compress *c,
> > void *dst,
> > unsigned int dstsize)
> > {
> > - unsigned uncompressed_size;
> > + unsigned compressed_size;
> > int ret;
> >
> > DBG_BUGON(!c->alg);
> > @@ -41,9 +41,10 @@ int erofs_compress_destsize(struct erofs_compress *c,
> > return ret;
> >
> > /* check if there is enough gains to compress */
> > - uncompressed_size = *srcsize;
> > - if (roundup(ret, EROFS_BLKSIZ) >= uncompressed_size *
> > - c->compress_threshold / 100)
> > + compressed_size = *srcsize <= EROFS_BLKSIZ ? ret :
> > + roundup(ret, EROFS_BLKSIZ);
> > +
> > + if (*srcsize <= compressed_size * c->compress_threshold / 100)
> > return -EAGAIN;
> > return ret;
> > }
> > diff --git a/lib/decompress.c b/lib/decompress.c
> > index 490c4bc..0b6678d 100644
> > --- a/lib/decompress.c
> > +++ b/lib/decompress.c
> > @@ -9,6 +9,7 @@
> >
> > #include "erofs/decompress.h"
> > #include "erofs/err.h"
> > +#include "erofs/print.h"
> >
> > #ifdef LZ4_ENABLED
> > #include <lz4.h>
> > @@ -50,6 +51,9 @@ static int z_erofs_decompress_lz4(struct z_erofs_decompress_req *rq)
> > rq->decodedlength);
> >
> > if (ret != (int)rq->decodedlength) {
> > + erofs_err("failed to %s decompress %d in[%u, %u] out[%u]",
> > + rq->partial_decoding ? "partial" : "full",
> > + ret, rq->inputsize, inputmargin, rq->decodedlength);
> > ret = -EIO;
> > goto out;
> > }
> > diff --git a/lib/inode.c b/lib/inode.c
> > index 787e5b4..1c08608 100644
> > --- a/lib/inode.c
> > +++ b/lib/inode.c
> > @@ -521,9 +521,6 @@ int erofs_prepare_tail_block(struct erofs_inode *inode)
> > struct erofs_buffer_head *bh;
> > int ret;
> >
> > - if (!inode->idata_size)
> > - return 0;
> > -
> > bh = inode->bh_data;
> > if (!bh) {
> > bh = erofs_balloc(DATA, EROFS_BLKSIZ, 0, 0);
> > @@ -557,26 +554,21 @@ int erofs_prepare_inode_buffer(struct erofs_inode *inode)
> > inodesize = Z_EROFS_VLE_EXTENT_ALIGN(inodesize) +
> > inode->extent_isize;
> >
> > - if (is_inode_layout_compression(inode))
> > - goto noinline;
> > -
> > - /*
> > - * if the file size is block-aligned for uncompressed files,
> > - * should use EROFS_INODE_FLAT_PLAIN data mapping mode.
> > - */
> > if (!inode->idata_size)
> > - inode->datalayout = EROFS_INODE_FLAT_PLAIN;
> > + goto noinline;
> >
> > bh = erofs_balloc(INODE, inodesize, 0, inode->idata_size);
> > if (bh == ERR_PTR(-ENOSPC)) {
> > int ret;
> >
> > - inode->datalayout = EROFS_INODE_FLAT_PLAIN;
> > -noinline:
> > /* expend an extra block for tail-end data */
> > ret = erofs_prepare_tail_block(inode);
> > if (ret)
> > return ret;
> > +noinline:
> > + if (!is_inode_layout_compression(inode))
> > + inode->datalayout = EROFS_INODE_FLAT_PLAIN;
> > +
> > bh = erofs_balloc(INODE, inodesize, 0, 0);
> > if (IS_ERR(bh))
> > return PTR_ERR(bh);
> > @@ -584,7 +576,16 @@ noinline:
> > } else if (IS_ERR(bh)) {
> > return PTR_ERR(bh);
> > } else if (inode->idata_size) {
> > - inode->datalayout = EROFS_INODE_FLAT_INLINE;
> > + if (is_inode_layout_compression(inode)) {
> > + struct z_erofs_map_header *h = inode->compressmeta;
> > + h->h_advise |= Z_EROFS_ADVISE_TAILPACKING;
> > + erofs_dbg("%s: inline %scompressed data (%u bytes)",
> > + inode->i_srcpath,
> > + inode->idata_raw ? "un" : "",
> > + inode->idata_size);
> > + } else {
> > + inode->datalayout = EROFS_INODE_FLAT_INLINE;
> > + }
> >
> > /* allocate inline buffer */
> > ibh = erofs_battach(bh, META, inode->idata_size);
> > @@ -640,20 +641,25 @@ int erofs_write_tail_end(struct erofs_inode *inode)
> > ibh->op = &erofs_write_inline_bhops;
> > } else {
> > int ret;
> > - erofs_off_t pos;
> > + erofs_off_t pos, pos0;
> > + const unsigned short padding = EROFS_BLKSIZ - inode->idata_size;
> >
> > erofs_mapbh(bh->block);
> > pos = erofs_btell(bh, true) - EROFS_BLKSIZ;
> > + pos0 = pos + inode->idata_size;
> > +
> > + if (is_inode_layout_compression(inode) &&
> > + erofs_sb_has_lz4_0padding() && !inode->idata_raw) {
> > + pos0 = pos;
> > + pos += padding;
> > + }
>
> I'd suggest don't use 0padding but record compressed_sized for
> tail-packing pcluster in the map header.... (need 2 bytes to record)
It's right if we can handle it when full decompression.
Thanks.
>
> Thanks,
> Gao Xiang
>
> > ret = dev_write(inode->idata, pos, inode->idata_size);
> > if (ret)
> > return ret;
> > - if (inode->idata_size < EROFS_BLKSIZ) {
> > - ret = dev_fillzero(pos + inode->idata_size,
> > - EROFS_BLKSIZ - inode->idata_size,
> > - false);
> > - if (ret)
> > - return ret;
> > - }
> > + ret = dev_fillzero(pos0, padding, false);
> > + if (ret)
> > + return ret;
> > +
> > inode->idata_size = 0;
> > free(inode->idata);
> > inode->idata = NULL;
> > diff --git a/mkfs/main.c b/mkfs/main.c
> > index e476189..7dd29df 100644
> > --- a/mkfs/main.c
> > +++ b/mkfs/main.c
> > @@ -48,6 +48,7 @@ static struct option long_options[] = {
> > {"product-out", required_argument, NULL, 11},
> > {"fs-config-file", required_argument, NULL, 12},
> > #endif
> > + {"inline", no_argument, NULL, 13},
> > {0, 0, 0, 0},
> > };
> >
> > @@ -87,6 +88,7 @@ static void usage(void)
> > " --all-root make all files owned by root\n"
> > " --help display this help and exit\n"
> > " --max-extent-bytes=# set maximum decompressed extent size # in bytes\n"
> > + " --inline tail-packing inline compressed data\n"
> > #ifndef NDEBUG
> > " --random-pclusterblks randomize pclusterblks for big pcluster (debugging only)\n"
> > #endif
> > @@ -304,6 +306,10 @@ static int mkfs_parse_options_cfg(int argc, char *argv[])
> > }
> > cfg.c_physical_clusterblks = i / EROFS_BLKSIZ;
> > break;
> > + case 13:
> > + erofs_sb_set_tailpacking();
> > + erofs_warn("EXPERIMENTAL compression inline feature in use. Use at your own risk!");
> > + break;
> >
> > case 1:
> > usage();
> > --
> > 2.29.0
> >
> >
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH 1/2] erofs-utils: support tail-packing inline compressed data
2021-10-27 1:58 ` Yue Hu
@ 2021-10-27 7:21 ` Yue Hu
2021-10-27 7:34 ` Gao Xiang
0 siblings, 1 reply; 8+ messages in thread
From: Yue Hu @ 2021-10-27 7:21 UTC (permalink / raw)
To: Gao Xiang; +Cc: linux-erofs, Yue Hu, geshifei, zhangwen, shaojunjun
Hi Xiang,
On Wed, 27 Oct 2021 09:58:07 +0800
Yue Hu <zbestahu@gmail.com> wrote:
> Hi Xiang,
>
> On Tue, 26 Oct 2021 20:00:37 +0800
> Gao Xiang <hsiangkao@linux.alibaba.com> wrote:
>
> > Hi Yue,
> >
> > On Mon, Oct 25, 2021 at 08:30:43PM +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@yulong.com>
> >
> > The patch roughly looks good to me. I'll play with it this week later.
> >
> > Some comments inline:
> >
> > > ---
> > > include/erofs/internal.h | 2 ++
> > > include/erofs_fs.h | 6 +++-
> > > lib/compress.c | 74 ++++++++++++++++++++++++++++++++--------
> > > lib/compressor.c | 9 ++---
> > > lib/decompress.c | 4 +++
> > > lib/inode.c | 50 +++++++++++++++------------
> > > mkfs/main.c | 6 ++++
> > > 7 files changed, 109 insertions(+), 42 deletions(-)
> > >
> > > diff --git a/include/erofs/internal.h b/include/erofs/internal.h
> > > index da7be56..42ae1ed 100644
> > > --- a/include/erofs/internal.h
> > > +++ b/include/erofs/internal.h
> > > @@ -109,6 +109,7 @@ static inline void erofs_sb_clear_##name(void) \
> > > 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(tailpacking, incompat, INCOMPAT_TAILPACKING)
> > > EROFS_FEATURE_FUNCS(sb_chksum, compat, COMPAT_SB_CHKSUM)
> > >
> > > #define EROFS_I_EA_INITED (1 << 0)
> > > @@ -148,6 +149,7 @@ struct erofs_inode {
> > > unsigned char inode_isize;
> > > /* inline tail-end packing size */
> > > unsigned short idata_size;
> > > + bool idata_raw;
> > >
> > > unsigned int xattr_isize;
> > > unsigned int extent_isize;
> > > diff --git a/include/erofs_fs.h b/include/erofs_fs.h
> > > index 18fc182..7700b27 100644
> > > --- a/include/erofs_fs.h
> > > +++ b/include/erofs_fs.h
> > > @@ -22,10 +22,12 @@
> > > #define EROFS_FEATURE_INCOMPAT_LZ4_0PADDING 0x00000001
> > > #define EROFS_FEATURE_INCOMPAT_COMPR_CFGS 0x00000002
> > > #define EROFS_FEATURE_INCOMPAT_BIG_PCLUSTER 0x00000002
> > > +#define EROFS_FEATURE_INCOMPAT_TAILPACKING 0x00000004
> > > #define EROFS_ALL_FEATURE_INCOMPAT \
> > > (EROFS_FEATURE_INCOMPAT_LZ4_0PADDING | \
> > > EROFS_FEATURE_INCOMPAT_COMPR_CFGS | \
> > > - EROFS_FEATURE_INCOMPAT_BIG_PCLUSTER)
> > > + EROFS_FEATURE_INCOMPAT_BIG_PCLUSTER | \
> > > + EROFS_FEATURE_INCOMPAT_TAILPACKING)
> > >
> > > #define EROFS_SB_EXTSLOT_SIZE 16
> > >
> > > @@ -230,10 +232,12 @@ 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 : 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_TAILPACKING 0x0008
> > >
> > > struct z_erofs_map_header {
> > > __le32 h_reserved1;
> > > diff --git a/lib/compress.c b/lib/compress.c
> > > index 2093bfd..c19f554 100644
> > > --- a/lib/compress.c
> > > +++ b/lib/compress.c
> > > @@ -159,6 +159,18 @@ static unsigned int z_erofs_get_max_pclusterblks(struct erofs_inode *inode)
> > > return cfg.c_physical_clusterblks;
> > > }
> > >
> > > +static int z_erofs_fill_inline_data(struct erofs_inode *inode, char *data,
> > > + unsigned int len, bool raw)
> > > +{
> > > + inode->idata_raw = raw;
> > > + 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)
> > > @@ -169,15 +181,19 @@ 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)
> > > + if (erofs_sb_has_tailpacking()) {
> > > + tail_pcluster = true;
> > > + pclustersize = EROFS_BLKSIZ;
> >
> > Not quite sure if such condition can be trigged for many times...
> >
> > Think about it. If the original pclustersize == 16 * EROFS_BLKSIZ, so we
> > could have at least 16 new pclustersize == EROFS_BLKSIZ then?
> >
> > But only the last pclustersize == EROFS_BLKSIZ can be inlined...
>
> Let me think about it more.
I understand we need to compress the tail pcluster(len <= pclustersize) by destsize
of fixed 4KB to get better inline result. rt?
Thanks.
>
> >
> > > + } else if (len <= EROFS_BLKSIZ)
> > > goto nocompression;
> > > } else {
> > > break;
> > > @@ -194,6 +210,17 @@ static int vle_compress_one(struct erofs_inode *inode,
> > > inode->i_srcpath,
> > > erofs_strerror(ret));
> > > }
> > > + if (tail_pcluster && len < EROFS_BLKSIZ) {
> > > + ret = z_erofs_fill_inline_data(inode,
> > > + (char *)(ctx->queue + ctx->head), len,
> > > + true);
> > > + if (ret)
> > > + return ret;
> > > + count = len;
> > > + raw = true;
> > > + ctx->compressedblks = 1;
> > > + goto add_head;
> > > + }
> > > nocompression:
> > > ret = write_uncompressed_extent(ctx, &len, dst);
> > > if (ret < 0)
> > > @@ -202,6 +229,16 @@ nocompression:
> > > ctx->compressedblks = 1;
> > > raw = true;
> > > } else {
> > > + if (tail_pcluster && ret < EROFS_BLKSIZ &&
> > > + !(len - count)) {
> > > + ret = z_erofs_fill_inline_data(inode, dst, ret,
> > > + false);
> > > + if (ret)
> > > + return ret;
> > > + raw = false;
> > > + ctx->compressedblks = 1;
> > > + goto add_head;
> > > + }
> > > const unsigned int tailused = ret & (EROFS_BLKSIZ - 1);
> > > const unsigned int padding =
> > > erofs_sb_has_lz4_0padding() && tailused ?
> > > @@ -226,11 +263,13 @@ nocompression:
> > > raw = false;
> > > }
> > >
> > > +add_head:
> > > ctx->head += count;
> > > /* write compression indexes for this pcluster */
> > > vle_write_indexes(ctx, count, raw);
> > >
> > > - ctx->blkaddr += ctx->compressedblks;
> > > + if (!inode->idata_size)
> > > + ctx->blkaddr += ctx->compressedblks;
> > > len -= count;
> > >
> > > if (!final && ctx->head >= EROFS_CONFIG_COMPR_MAX_SZ) {
> > > @@ -475,7 +514,7 @@ int erofs_write_compressed_file(struct erofs_inode *inode)
> > > fd = open(inode->i_srcpath, O_RDONLY | O_BINARY);
> > > if (fd < 0) {
> > > ret = -errno;
> > > - goto err_free;
> > > + goto err_free_meta;
> > > }
> > >
> > > /* allocate main data buffer */
> > > @@ -530,16 +569,19 @@ int erofs_write_compressed_file(struct erofs_inode *inode)
> > > goto err_bdrop;
> > > }
> > >
> > > + inode->idata_size = 0;
> > > +
> > > /* do the final round */
> > > ret = vle_compress_one(inode, &ctx, true);
> > > if (ret)
> > > - goto err_bdrop;
> > > + goto err_free_id;
> > >
> > > /* fall back to no compression mode */
> > > compressed_blocks = ctx.blkaddr - blkaddr;
> > > - if (compressed_blocks >= BLK_ROUND_UP(inode->i_size)) {
> > > + if (compressed_blocks >= BLK_ROUND_UP(inode->i_size) -
> > > + (inode->idata_size ? 1 : 0) ) {
> > > ret = -ENOSPC;
> > > - goto err_bdrop;
> > > + goto err_free_id;
> > > }
> > >
> > > vle_write_indexes_final(&ctx);
> > > @@ -553,12 +595,11 @@ int erofs_write_compressed_file(struct erofs_inode *inode)
> > > inode->i_srcpath, (unsigned long long)inode->i_size,
> > > compressed_blocks);
> > >
> > > - /*
> > > - * TODO: need to move erofs_bdrop to erofs_write_tail_end
> > > - * when both mkfs & kernel support compression inline.
> > > - */
> > > - erofs_bdrop(bh, false);
> > > - inode->idata_size = 0;
> > > + if (inode->idata_size)
> > > + inode->bh_data = bh;
> > > + else
> > > + erofs_bdrop(bh, false);
> > > +
> > > inode->u.i_blocks = compressed_blocks;
> > >
> > > legacymetasize = ctx.metacur - compressmeta;
> > > @@ -573,11 +614,14 @@ int erofs_write_compressed_file(struct erofs_inode *inode)
> > > inode->compressmeta = compressmeta;
> > > return 0;
> > >
> > > +err_free_id:
> > > + if (inode->idata)
> > > + free(inode->idata);
> > > err_bdrop:
> > > erofs_bdrop(bh, true); /* revoke buffer */
> > > err_close:
> > > close(fd);
> > > -err_free:
> > > +err_free_meta:
> > > free(compressmeta);
> > > return ret;
> > > }
> > > diff --git a/lib/compressor.c b/lib/compressor.c
> > > index 8836e0c..26189b6 100644
> > > --- a/lib/compressor.c
> > > +++ b/lib/compressor.c
> > > @@ -28,7 +28,7 @@ int erofs_compress_destsize(struct erofs_compress *c,
> > > void *dst,
> > > unsigned int dstsize)
> > > {
> > > - unsigned uncompressed_size;
> > > + unsigned compressed_size;
> > > int ret;
> > >
> > > DBG_BUGON(!c->alg);
> > > @@ -41,9 +41,10 @@ int erofs_compress_destsize(struct erofs_compress *c,
> > > return ret;
> > >
> > > /* check if there is enough gains to compress */
> > > - uncompressed_size = *srcsize;
> > > - if (roundup(ret, EROFS_BLKSIZ) >= uncompressed_size *
> > > - c->compress_threshold / 100)
> > > + compressed_size = *srcsize <= EROFS_BLKSIZ ? ret :
> > > + roundup(ret, EROFS_BLKSIZ);
> > > +
> > > + if (*srcsize <= compressed_size * c->compress_threshold / 100)
> > > return -EAGAIN;
> > > return ret;
> > > }
> > > diff --git a/lib/decompress.c b/lib/decompress.c
> > > index 490c4bc..0b6678d 100644
> > > --- a/lib/decompress.c
> > > +++ b/lib/decompress.c
> > > @@ -9,6 +9,7 @@
> > >
> > > #include "erofs/decompress.h"
> > > #include "erofs/err.h"
> > > +#include "erofs/print.h"
> > >
> > > #ifdef LZ4_ENABLED
> > > #include <lz4.h>
> > > @@ -50,6 +51,9 @@ static int z_erofs_decompress_lz4(struct z_erofs_decompress_req *rq)
> > > rq->decodedlength);
> > >
> > > if (ret != (int)rq->decodedlength) {
> > > + erofs_err("failed to %s decompress %d in[%u, %u] out[%u]",
> > > + rq->partial_decoding ? "partial" : "full",
> > > + ret, rq->inputsize, inputmargin, rq->decodedlength);
> > > ret = -EIO;
> > > goto out;
> > > }
> > > diff --git a/lib/inode.c b/lib/inode.c
> > > index 787e5b4..1c08608 100644
> > > --- a/lib/inode.c
> > > +++ b/lib/inode.c
> > > @@ -521,9 +521,6 @@ int erofs_prepare_tail_block(struct erofs_inode *inode)
> > > struct erofs_buffer_head *bh;
> > > int ret;
> > >
> > > - if (!inode->idata_size)
> > > - return 0;
> > > -
> > > bh = inode->bh_data;
> > > if (!bh) {
> > > bh = erofs_balloc(DATA, EROFS_BLKSIZ, 0, 0);
> > > @@ -557,26 +554,21 @@ int erofs_prepare_inode_buffer(struct erofs_inode *inode)
> > > inodesize = Z_EROFS_VLE_EXTENT_ALIGN(inodesize) +
> > > inode->extent_isize;
> > >
> > > - if (is_inode_layout_compression(inode))
> > > - goto noinline;
> > > -
> > > - /*
> > > - * if the file size is block-aligned for uncompressed files,
> > > - * should use EROFS_INODE_FLAT_PLAIN data mapping mode.
> > > - */
> > > if (!inode->idata_size)
> > > - inode->datalayout = EROFS_INODE_FLAT_PLAIN;
> > > + goto noinline;
> > >
> > > bh = erofs_balloc(INODE, inodesize, 0, inode->idata_size);
> > > if (bh == ERR_PTR(-ENOSPC)) {
> > > int ret;
> > >
> > > - inode->datalayout = EROFS_INODE_FLAT_PLAIN;
> > > -noinline:
> > > /* expend an extra block for tail-end data */
> > > ret = erofs_prepare_tail_block(inode);
> > > if (ret)
> > > return ret;
> > > +noinline:
> > > + if (!is_inode_layout_compression(inode))
> > > + inode->datalayout = EROFS_INODE_FLAT_PLAIN;
> > > +
> > > bh = erofs_balloc(INODE, inodesize, 0, 0);
> > > if (IS_ERR(bh))
> > > return PTR_ERR(bh);
> > > @@ -584,7 +576,16 @@ noinline:
> > > } else if (IS_ERR(bh)) {
> > > return PTR_ERR(bh);
> > > } else if (inode->idata_size) {
> > > - inode->datalayout = EROFS_INODE_FLAT_INLINE;
> > > + if (is_inode_layout_compression(inode)) {
> > > + struct z_erofs_map_header *h = inode->compressmeta;
> > > + h->h_advise |= Z_EROFS_ADVISE_TAILPACKING;
> > > + erofs_dbg("%s: inline %scompressed data (%u bytes)",
> > > + inode->i_srcpath,
> > > + inode->idata_raw ? "un" : "",
> > > + inode->idata_size);
> > > + } else {
> > > + inode->datalayout = EROFS_INODE_FLAT_INLINE;
> > > + }
> > >
> > > /* allocate inline buffer */
> > > ibh = erofs_battach(bh, META, inode->idata_size);
> > > @@ -640,20 +641,25 @@ int erofs_write_tail_end(struct erofs_inode *inode)
> > > ibh->op = &erofs_write_inline_bhops;
> > > } else {
> > > int ret;
> > > - erofs_off_t pos;
> > > + erofs_off_t pos, pos0;
> > > + const unsigned short padding = EROFS_BLKSIZ - inode->idata_size;
> > >
> > > erofs_mapbh(bh->block);
> > > pos = erofs_btell(bh, true) - EROFS_BLKSIZ;
> > > + pos0 = pos + inode->idata_size;
> > > +
> > > + if (is_inode_layout_compression(inode) &&
> > > + erofs_sb_has_lz4_0padding() && !inode->idata_raw) {
> > > + pos0 = pos;
> > > + pos += padding;
> > > + }
> >
> > I'd suggest don't use 0padding but record compressed_sized for
> > tail-packing pcluster in the map header.... (need 2 bytes to record)
>
> It's right if we can handle it when full decompression.
>
> Thanks.
>
> >
> > Thanks,
> > Gao Xiang
> >
> > > ret = dev_write(inode->idata, pos, inode->idata_size);
> > > if (ret)
> > > return ret;
> > > - if (inode->idata_size < EROFS_BLKSIZ) {
> > > - ret = dev_fillzero(pos + inode->idata_size,
> > > - EROFS_BLKSIZ - inode->idata_size,
> > > - false);
> > > - if (ret)
> > > - return ret;
> > > - }
> > > + ret = dev_fillzero(pos0, padding, false);
> > > + if (ret)
> > > + return ret;
> > > +
> > > inode->idata_size = 0;
> > > free(inode->idata);
> > > inode->idata = NULL;
> > > diff --git a/mkfs/main.c b/mkfs/main.c
> > > index e476189..7dd29df 100644
> > > --- a/mkfs/main.c
> > > +++ b/mkfs/main.c
> > > @@ -48,6 +48,7 @@ static struct option long_options[] = {
> > > {"product-out", required_argument, NULL, 11},
> > > {"fs-config-file", required_argument, NULL, 12},
> > > #endif
> > > + {"inline", no_argument, NULL, 13},
> > > {0, 0, 0, 0},
> > > };
> > >
> > > @@ -87,6 +88,7 @@ static void usage(void)
> > > " --all-root make all files owned by root\n"
> > > " --help display this help and exit\n"
> > > " --max-extent-bytes=# set maximum decompressed extent size # in bytes\n"
> > > + " --inline tail-packing inline compressed data\n"
> > > #ifndef NDEBUG
> > > " --random-pclusterblks randomize pclusterblks for big pcluster (debugging only)\n"
> > > #endif
> > > @@ -304,6 +306,10 @@ static int mkfs_parse_options_cfg(int argc, char *argv[])
> > > }
> > > cfg.c_physical_clusterblks = i / EROFS_BLKSIZ;
> > > break;
> > > + case 13:
> > > + erofs_sb_set_tailpacking();
> > > + erofs_warn("EXPERIMENTAL compression inline feature in use. Use at your own risk!");
> > > + break;
> > >
> > > case 1:
> > > usage();
> > > --
> > > 2.29.0
> > >
> > >
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH 1/2] erofs-utils: support tail-packing inline compressed data
2021-10-27 7:21 ` Yue Hu
@ 2021-10-27 7:34 ` Gao Xiang
2021-10-27 8:02 ` Yue Hu
0 siblings, 1 reply; 8+ messages in thread
From: Gao Xiang @ 2021-10-27 7:34 UTC (permalink / raw)
To: Yue Hu; +Cc: linux-erofs, Yue Hu, geshifei, zhangwen, shaojunjun
Hi Yue,
On Wed, Oct 27, 2021 at 03:21:37PM +0800, Yue Hu wrote:
...
> > > > - if (len <= pclustersize) {
> > > > + if (!tail_pcluster && len <= pclustersize) {
> > > > if (final) {
> > > > - if (len <= EROFS_BLKSIZ)
> > > > + if (erofs_sb_has_tailpacking()) {
> > > > + tail_pcluster = true;
> > > > + pclustersize = EROFS_BLKSIZ;
> > >
> > > Not quite sure if such condition can be trigged for many times...
> > >
> > > Think about it. If the original pclustersize == 16 * EROFS_BLKSIZ, so we
> > > could have at least 16 new pclustersize == EROFS_BLKSIZ then?
> > >
> > > But only the last pclustersize == EROFS_BLKSIZ can be inlined...
> >
> > Let me think about it more.
>
> I understand we need to compress the tail pcluster(len <= pclustersize) by destsize
> of fixed 4KB to get better inline result. rt?
I think this is the tricky part of tail-packing inline support for
compressed data.
As you may know, EROFS supports variable-sized blocks for each pcluster
so you could change pclustersize accordingly for the last pclusters.
For example, originally if the size of the last one pcluster is
16 * EROFS_BLKSIZ (therefore it cannot be tail-packing directly), there
are 2 policies in practice can be achieved:
1) compress with 2 pclusters ---
X pcluster size + Y (Y <= 4KiB) pcluster size (so the last one can
be tail-packing);
2) compress with 4KiB pclusters ---
4KiB pcluster + 4KiB pcluster + ... + Z (Z <= 4KiB) pcluster
I'm not sure which one is easier to implement, maybe 2) is easier, so we
could implement it first.
Thanks,
Gao Xiang
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH 1/2] erofs-utils: support tail-packing inline compressed data
2021-10-27 7:34 ` Gao Xiang
@ 2021-10-27 8:02 ` Yue Hu
0 siblings, 0 replies; 8+ messages in thread
From: Yue Hu @ 2021-10-27 8:02 UTC (permalink / raw)
To: Gao Xiang; +Cc: linux-erofs, Yue Hu, geshifei, zhangwen, shaojunjun
Hi Xiang,
On Wed, 27 Oct 2021 15:34:58 +0800
Gao Xiang <hsiangkao@linux.alibaba.com> wrote:
> Hi Yue,
>
> On Wed, Oct 27, 2021 at 03:21:37PM +0800, Yue Hu wrote:
>
> ...
>
> > > > > - if (len <= pclustersize) {
> > > > > + if (!tail_pcluster && len <= pclustersize) {
> > > > > if (final) {
> > > > > - if (len <= EROFS_BLKSIZ)
> > > > > + if (erofs_sb_has_tailpacking()) {
> > > > > + tail_pcluster = true;
> > > > > + pclustersize = EROFS_BLKSIZ;
> > > >
> > > > Not quite sure if such condition can be trigged for many times...
> > > >
> > > > Think about it. If the original pclustersize == 16 * EROFS_BLKSIZ, so we
> > > > could have at least 16 new pclustersize == EROFS_BLKSIZ then?
> > > >
> > > > But only the last pclustersize == EROFS_BLKSIZ can be inlined...
> > >
> > > Let me think about it more.
> >
> > I understand we need to compress the tail pcluster(len <= pclustersize) by destsize
> > of fixed 4KB to get better inline result. rt?
>
> I think this is the tricky part of tail-packing inline support for
> compressed data.
>
> As you may know, EROFS supports variable-sized blocks for each pcluster
> so you could change pclustersize accordingly for the last pclusters.
>
> For example, originally if the size of the last one pcluster is
> 16 * EROFS_BLKSIZ (therefore it cannot be tail-packing directly), there
> are 2 policies in practice can be achieved:
> 1) compress with 2 pclusters ---
> X pcluster size + Y (Y <= 4KiB) pcluster size (so the last one can
> be tail-packing);
Sounds good. Let me check.
> 2) compress with 4KiB pclusters ---
> 4KiB pcluster + 4KiB pcluster + ... + Z (Z <= 4KiB) pcluster
>
> I'm not sure which one is easier to implement, maybe 2) is easier, so we
> could implement it first.
Yeah, the patch already includes it.
Thanks.
>
> Thanks,
> Gao Xiang
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-10-27 8:03 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-25 12:30 [RFC PATCH 0/2] erofs-utils: compression inline feature Yue Hu
2021-10-25 12:30 ` [RFC PATCH 1/2] erofs-utils: support tail-packing inline compressed data Yue Hu
2021-10-26 12:00 ` Gao Xiang
2021-10-27 1:58 ` Yue Hu
2021-10-27 7:21 ` Yue Hu
2021-10-27 7:34 ` Gao Xiang
2021-10-27 8:02 ` Yue Hu
2021-10-25 12:30 ` [RFC PATCH 2/2] erofs-utils: fuse: " Yue Hu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.