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

Gao Xiang <[email protected]> 于2026年5月18日周一 12:18写道：
>
> 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?
Hi Xiang,
Just confirming a few minor issues：
>
>
> 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;
if  compacted_2b && compacted_4b_end both are zero,  should not set this???
>
>         /* 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);
 DBG_BUGON(inode->idata_type != EROFS_IDATA_TYPE_COMPRESSED_DEFAULT); ???
>
>         /* 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;
if  compacted 2B, lobits=2+12=14, but encodebis =14, clusterofbits
should be 12??
Or is there something I'm misunderstanding?

> +               }
> +               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,
>
>
>