All of lore.kernel.org
 help / color / mirror / Atom feed
* [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.