Re: [PATCH] erofs-utils: mkfs: also handle last compacted 2B pack in z_erofs_drop_inline_pcluster

Hi Zhiguo,

On 2026/5/11 15:54, Zhiguo Niu wrote:
> Gao Xiang <[email protected]> 于2026年5月11日周一 12:01写道：
>>
>> Hi Zhiguo,
>>
>> On 2026/4/29 15:59, Zhiguo Niu wrote:
>>> With ztailpacking enabled, the current process assumes that a compacted_4b_end
>>> always exists in the compacted pack. However, in some specific files, the
>>> compacted pack may not have a compacted_4b_end. This leads to an incorrect
>>> modification of the last compacted_2B entry, resulting in incorrect modification
>>> of its clusterofs. In subsequent fsck operations, incorrect parameters will
>>> affect the decompression of the penultimate pcluster.
>>>
>>> This patch determines whether the last entry of the current compacted pack
>>> belongs to compacted 2B or 4B and then updates the correct bits accordingly.
>>>
>>> Fixes: a7c1f0575ef8 ("erofs-utils: lib: refine tailpcluster compression approach")
>>> Signed-off-by: Zhiguo Niu <[email protected]>
>>
>> Sorry for late response.
>>
>> I do think the issue is valid, but either the previous
>> solution or the proposed one is ugly.
> 
> Hi Xiang,
> Yes it would be ideal if the same piece of common code could cover
> both scenarios.
> But I haven't figured it out yet, so I'll distinguish them like this for now. ^^
> thanks!
>>
Could you confirm if the following diff fixes the issue?


diff --git a/include/erofs/defs.h b/include/erofs/defs.h
index 9f3d0f9c35bc..0e4c2a9b53c7 100644
--- a/include/erofs/defs.h
+++ b/include/erofs/defs.h
@@ -218,6 +218,11 @@ typedef int64_t         s64;
  #define get_unaligned(ptr)	__get_unaligned_t(typeof(*(ptr)), (ptr))
  #define put_unaligned(val, ptr) __put_unaligned_t(typeof(*(ptr)), (val), (ptr))

+static inline u32 get_unaligned_le16(const void *p)
+{
+	return le32_to_cpu(__get_unaligned_t(__le16, p));
+}
+
  static inline u32 get_unaligned_le32(const void *p)
  {
  	return le32_to_cpu(__get_unaligned_t(__le32, p));
diff --git a/include/erofs/internal.h b/include/erofs/internal.h
index 450e2647cca7..2cc9cc8009aa 100644
--- a/include/erofs/internal.h
+++ b/include/erofs/internal.h
@@ -210,6 +210,12 @@ struct erofs_diskbuf;
  #define EROFS_INODE_DATA_SOURCE_RESVSP		3
  #define EROFS_INODE_DATA_SOURCE_REBUILD_BLOB	4

+enum erofs_idata_type {
+	EROFS_IDATA_TYPE_RAW,
+	EROFS_IDATA_TYPE_COMPRESSED_DEFAULT,
+	EROFS_IDATA_TYPE_COMPRESSED_END_OF_2B,
+};
+
  #define EROFS_I_BLKADDR_DEV_ID_BIT		48

  struct erofs_inode {
@@ -262,7 +268,7 @@ struct erofs_inode {
  	unsigned short idata_size;
  	char datasource;
  	bool in_metabox;
-	bool compressed_idata;
+	char idata_type;
  	bool lazy_tailblock;
  	bool opaque;
  	/* OVL: non-merge dir that may contain whiteout entries */
diff --git a/lib/compress.c b/lib/compress.c
index 62d2672cb665..e171aee48c0b 100644
--- a/lib/compress.c
+++ b/lib/compress.c
@@ -483,7 +483,7 @@ static int z_erofs_fill_inline_data(struct erofs_inode *inode, void *data,
  {
  	inode->z_advise |= Z_EROFS_ADVISE_INLINE_PCLUSTER;
  	inode->idata_size = len;
-	inode->compressed_idata = !raw;
+	inode->idata_type = EROFS_IDATA_TYPE_COMPRESSED_DEFAULT;

  	inode->idata = malloc(inode->idata_size);
  	if (!inode->idata)
@@ -980,7 +980,8 @@ int z_erofs_convert_to_compacted_format(struct erofs_inode *inode,
  					      &dummy_head, big_pcluster);
  		compacted_2b -= 16;
  	}
-	DBG_BUGON(compacted_2b);
+	if (!compacted_4b_end && inode->idata_size)
+		inode->idata_type = EROFS_IDATA_TYPE_COMPRESSED_END_OF_2B;

  	/* generate compacted_4b_end */
  	while (compacted_4b_end > 1) {
@@ -1210,10 +1211,12 @@ void z_erofs_drop_inline_pcluster(struct erofs_inode *inode)

  	h->h_advise = cpu_to_le16(le16_to_cpu(h->h_advise) &
  				  ~Z_EROFS_ADVISE_INLINE_PCLUSTER);
+	DBG_BUGON(inode->idata_size != le16_to_cpu(h->h_idata_size));
  	h->h_idata_size = 0;
+
  	if (!inode->eof_tailraw)
  		return;
-	DBG_BUGON(inode->compressed_idata != true);
+	DBG_BUGON(inode->idata_type != EROFS_IDATA_TYPE_RAW);

  	/* patch the EOF lcluster to uncompressed type first */
  	if (inode->datalayout == EROFS_INODE_COMPRESSED_FULL) {
@@ -1224,18 +1227,26 @@ void z_erofs_drop_inline_pcluster(struct erofs_inode *inode)
  		di->di_advise = cpu_to_le16(type);
  	} else if (inode->datalayout == EROFS_INODE_COMPRESSED_COMPACT) {
  		/* handle the last compacted 4B pack */
-		unsigned int eofs, base, pos, v, lo;
+		unsigned int lclusterbits = inode->z_lclusterbits;
+		unsigned int lobits, eofs, base, pos, v;
  		u8 *out;

-		eofs = inode->extent_isize -
-			(4 << (BLK_ROUND_UP(sbi, inode->i_size) & 1));
-		base = round_down(eofs, 8);
-		pos = 16 /* encodebits */ * ((eofs - base) / 4);
-		out = inode->compressmeta + base;
-		lo = erofs_blkoff(sbi, get_unaligned_le32(out + pos / 8));
-		v = (type << sbi->blkszbits) | lo;
-		out[pos / 8] = v & 0xff;
-		out[pos / 8 + 1] = v >> 8;
+		lobits = max(lclusterbits, ilog2(Z_EROFS_LI_D0_CBLKCNT) + 1U);
+
+		if (inode->idata_type == EROFS_IDATA_TYPE_COMPRESSED_DEFAULT) {
+			eofs = inode->extent_isize -
+				(4 << (BLK_ROUND_UP(sbi, inode->i_size) & 1));
+			base = round_down(eofs, 8);
+			pos = 16 /* encodebits */ * ((eofs - base) / 4);
+			out = inode->compressmeta + base + pos / 8;
+		} else {
+			out = inode->compressmeta + inode->extent_isize - 4 - 2;
+			lobits = 16 - 14 /* encodebits */ + lobits;
+		}
+		v = (get_unaligned_le16(out) & (BIT(lobits) - 1)) |
+			(type << lobits);
+		*out = v & 0xff;
+		*(out + 1) = v >> 8;
  	} else {
  		DBG_BUGON(1);
  		return;
@@ -1244,7 +1255,7 @@ void z_erofs_drop_inline_pcluster(struct erofs_inode *inode)
  	/* replace idata with prepared uncompressed data */
  	inode->idata = inode->eof_tailraw;
  	inode->idata_size = inode->eof_tailrawsize;
-	inode->compressed_idata = false;
+	inode->idata_type = EROFS_IDATA_TYPE_RAW;
  	inode->eof_tailraw = NULL;
  }

diff --git a/lib/inode.c b/lib/inode.c
index 735319e1d3bf..c225faa121e7 100644
--- a/lib/inode.c
+++ b/lib/inode.c
@@ -1043,7 +1043,7 @@ noinline:
  		if (is_inode_layout_compression(inode)) {
  			DBG_BUGON(!params->ztailpacking);
  			erofs_dbg("Inline %scompressed data (%u bytes) to %s",
-				  inode->compressed_idata ? "" : "un",
+				  inode->idata_type == EROFS_IDATA_TYPE_RAW ? "un": "",
  				  inode->idata_size, inode->i_srcpath);
  			erofs_sb_set_ztailpacking(sbi);
  		} else {
@@ -1149,7 +1149,8 @@ static int erofs_write_tail_end(struct erofs_importer *im,
  		pos = erofs_btell(bh, true) - erofs_blksiz(sbi);

  		/* 0'ed data should be padded at head for 0padding conversion */
-		h0 = erofs_sb_has_lz4_0padding(sbi) && inode->compressed_idata;
+		h0 = erofs_sb_has_lz4_0padding(sbi) &&
+			inode->idata_type != EROFS_IDATA_TYPE_RAW;
  		DBG_BUGON(inode->idata_size > erofs_blksiz(sbi));

  		iov[h0] = (struct iovec) { .iov_base = inode->idata,