Re: [PATCH v2] f2fs: compress: don't redirty sparse cluster during {,de}compress

[Chao Yu]

On 2024/12/13 0:11, Jaegeuk Kim wrote:
> On 12/12, Dan Carpenter wrote:
> > On Mon, Aug 19, 2024 at 05:34:30PM +0900, Yeongjin Gil wrote:
> > > In f2fs_do_write_data_page, when the data block is NULL_ADDR, it skips
> > > writepage considering that it has been already truncated.
> > > This results in an infinite loop as the PAGECACHE_TAG_TOWRITE tag is not
> > > cleared during the writeback process for a compressed file including
> > > NULL_ADDR in compress_mode=user.
> > >
> > > This is the reproduction process:
> > >
> > > 1. dd if=/dev/zero bs=4096 count=1024 seek=1024 of=testfile
> > > 2. f2fs_io compress testfile
> > > 3. dd if=/dev/zero bs=4096 count=1 conv=notrunc of=testfile
> > > 4. f2fs_io decompress testfile
> > >
> > > To prevent the problem, let's check whether the cluster is fully
> > > allocated before redirty its pages.
> >
> > We were discussing how to detect these sorts of things in the future.
> > Presumably a user found this by chance?  Xfstests has two tests which deal
> > with compression tests/f2fs/002 and tests/f2fs/007.  But it feels like
> > xfstests is not really the right place for this sort of thing, it would
> > be better as part of some sort of fuzz testing.
> >
> > What do you think?
>
> Yeah, agreed that we must have tests to catch this. One way may be creating
> some basic disk images having some possible valid layout to see f2fs can
> work as intended. I feel we can put it in xfstests as wel?
> > Chao, thoughts?

Hi Jaegeuk, Dan,

I'd like to continue to add regression testcases like f2fs/[003-008]
to xfstests, so that, we can make sure last change of f2fs won't cause
any regression by checking w/ those testcases.

Recently, Sheng Yong has introduced a new tool named inject.f2fs [1], and
then, built an auto testsuit in f2fs-tools based on inject.f2fs [2], w/
this testsuit, we can construct image by injecting specified fields, and
check the image w/ fsck to see whether result is as expected.

IMO, it's fine to add new testcases into testsuit to check whether f2fs'
behavior is as expected on constructed image w/ valid data layout.

So, regression testcase goes into xfstests, and fuzz/common image testcase
goes into testsuit of f2fs-tools? what do you think?

[1] "f2fs-tools: introduce inject.f2fs" https://lore.kernel.org/linux-f2fs-devel/20240704025740.527171-1-shengyong@xxxxxxxx/
[2] "f2fs-tools: add testcases" https://lore.kernel.org/linux-f2fs-devel/20241029120956.4186731-1-shengyong@xxxxxxxx/

Thanks,

> > regards,
> > dan carpenter