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

[Gao Xiang <hsiangkao@linux.alibaba.com>]

Hi all,

On 2026/7/21 14:55, zhaoyifan (H) wrote:
> 
> On 2026/7/20 19:42, Zhiguo Niu wrote:
>> Yifan Zhao <zhaoyifan28@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 <es20490446e@gmail.com>
>>> Closes: https://github.com/erofs/erofs-utils/issues/51
>>> Assisted-by: Codex:GPT-5.5
>>> Signed-off-by: Yifan Zhao <zhaoyifan28@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?

This case has been resolved on the kernel side, so before we
get a cleaner way, let's keep this as-is since it's not a
new case for many year.

Thanks,
Gao Xiang

> 
> 
> Thanks,
> 
> Yifan
>