From: Gao Xiang <xiang@kernel.org>
To: seula2.lee@lge.com
Cc: linux-erofs@lists.ozlabs.org, Gao Xiang <hsiangkao@linux.alibaba.com>
Subject: Re: [PATCH] erofs-utils: lib: fix erofs_is_packed_inode() on the read path
Date: Thu, 13 Aug 2026 20:08:43 +0800 [thread overview]
Message-ID: <an2zy_VyU3rP47vx@XiangdeMacBook-Pro.local> (raw)
In-Reply-To: <20260813093849.3607625-1-seula2.lee@lge.com>
Hi Seula,
On Thu, Aug 13, 2026 at 06:38:49PM +0900, seula2.lee@lge.com wrote:
> From: Seula Lee <seula2.lee@lge.com>
>
> erofs_is_packed_inode() compares i_srcpath against the global
> EROFS_PACKED_INODE identifier. i_srcpath is only assigned by
> mkfs.erofs and has no on-disk counterpart, so it is never set for
> inodes filled in by erofs_read_inode_from_disk(), and the helper always
> returns false on the read path.
>
> fsck.erofs therefore treats the packed inode as an ordinary regular
> file at all four of its call sites, for any image with fragments and a
> non-zero packed_nid:
>
> - erofsfsck_extract_inode(): the guard never fires, so
> erofs_extract_file() writes the packed data to fsckcfg.extract_path
> itself, which is still the top-level directory because main()
> checks the packed inode before the root inode. Extracting the root
> then fails in erofs_extract_dir():
>
> $ mkfs.erofs -zlz4hc -Efragments img.erofs dir
> $ fsck.erofs --extract=out img.erofs
> <E> erofs: erofs_extract_dir() Line[697] path is not a directory: out
> <E> erofs: main() Line[1256] Failed to extract filesystem
> $ file out
> out: ASCII text # 603720 bytes, the packed inode itself
>
> - erofsfsck_set_attributes(): the packed inode's mode, timestamps and
> xattrs are applied to the extraction root directory.
>
> - erofs_verify_inode_data(): the packed inode's own compressed data is
> decoded and hashed as if it were file data.
>
> - the -p accounting: fragment bytes are counted both as file data and
> as packed inode data, so the reported ratio is too low.
>
> Restore the nid-based test, which is how the packed inode is already
> identified elsewhere on the read path: z_erofs_read_one_data() compares
> inode->nid against sbi->packed_nid directly, and the
> erofs_sb_has_fragments(sbi) && sbi->packed_nid > 0 idiom used here is
> the same one in fuse/main.c, dump/main.c and fsck.erofs' own
> erofs_packedfile_init() call.
>
> The i_srcpath comparison has to stay first rather than be replaced:
> while mkfs.erofs builds the packed inode its nid is still
> EROFS_NID_UNALLOCATED and sbi->packed_nid is 0, so a nid-only test
> would return false there and break fragment dedupe and
> pclusterblks_packed handling in lib/compress.c.
>
> This restores the v1.8.x behaviour at every call site, where the helper
> was nid-based; in particular the packed inode is again excluded from the
> decompression check that --extract performs, as it was before v1.9.
>
> dump.erofs is affected the same way and prints "Path : (packed file)"
> again as intended.
>
> erofs_is_metabox_inode() shares the same idiom, but it has no read-path
> caller (all callers are in lib/compress.c and lib/inode.c), so it is
> left alone.
>
> Fixes: 7928074b7643 ("erofs-utils: introduce metadata compression [metabox]")
> Signed-off-by: Seula Lee <seula2.lee@lge.com>
Thanks for the patch, that is indeed a bug and the fix looks fine,
but the commit message is too long to understand the impacts (it seems
in a LLM-generated-style.)
I will rephrase a short commit message instead.
Thanks,
Gao Xiang
prev parent reply other threads:[~2026-08-13 12:08 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-13 9:38 [PATCH] erofs-utils: lib: fix erofs_is_packed_inode() on the read path seula2.lee
2026-08-13 12:08 ` Gao Xiang [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=an2zy_VyU3rP47vx@XiangdeMacBook-Pro.local \
--to=xiang@kernel.org \
--cc=hsiangkao@linux.alibaba.com \
--cc=linux-erofs@lists.ozlabs.org \
--cc=seula2.lee@lge.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.