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

Yifan Zhao <[email protected]> 于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 <[email protected]>
> Closes: https://github.com/erofs/erofs-utils/issues/51
> Assisted-by: Codex:GPT-5.5
> Signed-off-by: Yifan Zhao <[email protected]>
> ---

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!
> +               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
>
>