Re: [PATCH] erofs-utils: lib: fix ztailpacking fallback across lclusters

[zhaoyifan (H)]

On 2026/7/20 19:42, Zhiguo Niu wrote:
Yifan Zhao <zhaoyifa...@huawei.com> 于2026年7月9日周四 20:36写道：
With ztailpacking, the final compressed pcluster is first stored as
inline data.  If the inode metadata area cannot hold it, mkfs falls back
to a normal tail block and drops the inline pcluster marker.

The current fallback path assumes that the inline tail pcluster belongs
to the EOF lcluster.  That is not always true: the tail pcluster can
start in the previous lcluster and end at EOF, while its raw size still
fits in one block.  In that case, patching the EOF lcluster is
semantically wrong.

Let's keep raw tail data whenever it fits in one block, and convert the
corresponding lcluster index to PLAIN during fallback.

Reported-by: Alberto Salvia Novella <es204904...@gmail.com>
Closes: https://github.com/erofs/erofs-utils/issues/51
Assisted-by: Codex:GPT-5.5
Signed-off-by: Yifan Zhao <zhaoyifa...@huawei.com>
---
Hi Yifan,
I tested this patch to focus on the issue fixed by commit
277a42502a7a, and it passed.
But I have some questions:
  include/erofs/internal.h |   5 +-
  lib/compress.c           | 109 ++++++++++++++++++++++++++++-----------
  2 files changed, 83 insertions(+), 31 deletions(-)

diff --git a/include/erofs/internal.h b/include/erofs/internal.h
index 2cc9cc8..bdde41f 100644
--- a/include/erofs/internal.h
+++ b/include/erofs/internal.h
@@ -212,8 +212,11 @@ struct erofs_diskbuf;

  enum erofs_idata_type {
         EROFS_IDATA_TYPE_RAW,
-       EROFS_IDATA_TYPE_COMPRESSED_DEFAULT,
+       EROFS_IDATA_TYPE_COMPRESSED,
+       /* compressed idata follows a final 2B compacted index pack */
         EROFS_IDATA_TYPE_COMPRESSED_END_OF_2B,
+       /* compressed idata follows a final single-entry 4B pack after a 2B 
pack */
+       EROFS_IDATA_TYPE_COMPRESSED_4B1_PREV2B,
  };

  #define EROFS_I_BLKADDR_DEV_ID_BIT             48
diff --git a/lib/compress.c b/lib/compress.c
index f7ad5a1..ec90f65 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->idata_type = EROFS_IDATA_TYPE_COMPRESSED_DEFAULT;
+       inode->idata_type = EROFS_IDATA_TYPE_COMPRESSED;

         inode->idata = malloc(inode->idata_size);
         if (!inode->idata)
@@ -664,7 +664,7 @@ frag_packing:
                 ictx->fragemitted = true;
         /* tailpcluster should be less than 1 block */
         } else if (may_inline && len == e->length && compressedsize < blksz) {
-               if (ctx->clusterofs + len <= blksz) {
Shouldn't this condition restrict the `tail pcluster` so that it
corresponds to only  eof`lcluster`?
and we drop the inline pcluster just when  eof_tailraw is not null.
Thanks!

Hi Zhiguo,


A tail pcluster may start in the previous lcluster and end in the EOF 
lcluster while its whole raw payload still fits in one physical block. 
In that case `ctx->clusterofs + len > blksz`, but `len <= blksz`, but it 
should still be converted into a single PLAIN pcluster.

This patch targets this edge case so I think change the if statement 
here is necessary?


Thanks,

Yifan

+               if (len <= blksz) {
                         inode->eof_tailraw = malloc(len);
                         if (!inode->eof_tailraw)
                                 return -ENOMEM;
@@ -962,6 +962,14 @@ int z_erofs_convert_to_compacted_format(struct erofs_inode 
*inode,
                 dummy_head = true;
         }

+       if (inode->idata_size) {
+               if (compacted_2b && !compacted_4b_end)
+                       inode->idata_type = 
EROFS_IDATA_TYPE_COMPRESSED_END_OF_2B;
+               else if (compacted_2b && compacted_4b_end == 1)
+                       inode->idata_type =
+                               EROFS_IDATA_TYPE_COMPRESSED_4B1_PREV2B;
+       }
+
         /* generate compacted_4b_initial */
         while (compacted_4b_initial) {
                 in = parse_legacy_indexes(cv, 2, in);
@@ -974,8 +982,6 @@ int z_erofs_convert_to_compacted_format(struct erofs_inode 
*inode,

         /* generate compacted_2b */
         if (compacted_2b) {
-               if (!compacted_4b_end && inode->idata_size)
-                       inode->idata_type = 
EROFS_IDATA_TYPE_COMPRESSED_END_OF_2B;
                 do {
                         in = parse_legacy_indexes(cv, 16, in);
                         out = write_compacted_indexes(out, cv, &blkaddr,
@@ -1205,11 +1211,65 @@ out:
         return metabuf;
  }

+static void z_erofs_patch_tail_compacted_index(struct erofs_inode *inode,
+                                              bool previous,
+                                              unsigned int type)
+{
+       const unsigned int totalidx = BLK_ROUND_UP(inode->sbi, inode->i_size);
+       const unsigned int lobits = max_t(unsigned int, inode->z_lclusterbits,
+                               ilog2(Z_EROFS_LI_D0_CBLKCNT) + 1U);
+       u8 *base = inode->compressmeta;
+       u8 *pack = base + inode->extent_isize;
+       unsigned int bitpos, bitoff;
+       u8 *out;
+       u32 v;
+
+       DBG_BUGON(!totalidx);
+       DBG_BUGON(inode->z_lclusterbits > 14);
+
+       if (inode->idata_type == EROFS_IDATA_TYPE_COMPRESSED_END_OF_2B) {
+               pack -= 32;
+               bitpos = 14 * (previous ? 14 : 15);
+               goto out;
+       }
+
+       /* Last compacted index pack is 4B */
+       pack -= 8;
+       if (!(totalidx & 1)) {
+               bitpos = previous ? 0 : 16;
+               goto out;
+       }
+
+       if (!previous) {
+               bitpos = 0;
+               goto out;
+       }
+
+       /* Second to last compacted index pack is 2B */
+       if (inode->idata_type == EROFS_IDATA_TYPE_COMPRESSED_4B1_PREV2B) {
+               pack -= 32;
+               bitpos = 14 * 15;
+       } else {
+               pack -= 8;
+               bitpos = 16;
+       }
+out:
+       DBG_BUGON(pack < base);
+       bitoff = bitpos & 7;
+       out = pack + bitpos / 8;
+       v = get_unaligned_le32(out);
+       v &= ~(Z_EROFS_LI_LCLUSTER_TYPE_MASK << (lobits + bitoff));
+       v |= type << (lobits + bitoff);
+       put_unaligned_le32(v, out);
+}
+
  void z_erofs_drop_inline_pcluster(struct erofs_inode *inode)
  {
         struct erofs_sb_info *sbi = inode->sbi;
         const unsigned int type = Z_EROFS_LCLUSTER_TYPE_PLAIN;
         struct z_erofs_map_header *h = inode->compressmeta;
+       erofs_off_t rawstart;
+       erofs_blk_t head_lcn, eof_lcn;

         h->h_advise = cpu_to_le16(le16_to_cpu(h->h_advise) &
                                   ~Z_EROFS_ADVISE_INLINE_PCLUSTER);
@@ -1218,38 +1278,27 @@ void z_erofs_drop_inline_pcluster(struct erofs_inode 
*inode)
         if (!inode->eof_tailraw)
                 return;
         DBG_BUGON(inode->idata_type == EROFS_IDATA_TYPE_RAW);
+       DBG_BUGON(!inode->i_size);
+       DBG_BUGON(inode->eof_tailrawsize > erofs_blksiz(sbi));
+       DBG_BUGON(inode->eof_tailrawsize > inode->i_size);
+
+       rawstart = inode->i_size - inode->eof_tailrawsize;
+       head_lcn = rawstart >> sbi->blkszbits;
+       eof_lcn = (inode->i_size - 1) >> sbi->blkszbits;
+       DBG_BUGON(head_lcn != eof_lcn && head_lcn + 1 != eof_lcn);

-       /* patch the EOF lcluster to uncompressed type first */
+       /* patch the tail pcluster head to uncompressed type first */
         if (inode->datalayout == EROFS_INODE_COMPRESSED_FULL) {
                 struct z_erofs_lcluster_index *di =
-                       (inode->compressmeta + inode->extent_isize) -
-                       sizeof(struct z_erofs_lcluster_index);
+                       (void *)((u8 *)inode->compressmeta +
+                                Z_EROFS_LEGACY_MAP_HEADER_SIZE +
+                                head_lcn *
+                                sizeof(struct z_erofs_lcluster_index));

                 di->di_advise = cpu_to_le16(type);
         } else if (inode->datalayout == EROFS_INODE_COMPRESSED_COMPACT) {
-               /* handle the last compacted 4B/2B pack */
-               unsigned int lclusterbits = inode->z_lclusterbits;
-               unsigned int lobits, eofs, base, pos, v;
-               u8 *out;
-
-               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 -
-                               sizeof(__le32) - sizeof(__le16);
-                       lobits = 16 - 14 /* encodebits */ + lobits;
-               }
-
-               v = (get_unaligned_le16(out) & (BIT(lobits) - 1)) |
-                       (type << lobits);
-               *out = v & 0xff;
-               *(out + 1) = v >> 8;
+               z_erofs_patch_tail_compacted_index(inode, head_lcn != eof_lcn,
+                                                  type);
         } else {
                 DBG_BUGON(1);
                 return;
--
2.47.3