Re: [PATCH 25/27] ext4: add KUnit tests for fs/ext4/move_extent.c

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Medium] ext4: 64KB page failure in test_mext_check_adjust_range()
- [Medium] ext4: sub-folio dirty buffer -EBUSY in move_extent-test.c on
  >4KB pages
--

commit abdff1b8eb818ad1bc77ee7f502a090b450df395
Author: Theodore Ts'o <tytso@mit.edu>

ext4: add KUnit tests for fs/ext4/move_extent.c

This commit adds a KUnit test suite covering lock ordering, validity and
range checks, and extent swapping in fs/ext4/move_extent.c.

> diff --git a/fs/ext4/move_extent-test.c b/fs/ext4/move_extent-test.c
> new file mode 100644
> index 0000000000000..e29a392eeebe3
> --- /dev/null
> +++ b/fs/ext4/move_extent-test.c
[ ... ]
> +static void test_mext_check_adjust_range(struct kunit *test)
> +{
> +	struct mext_test_ctx *ctx = mext_setup_ctx(test);
> +	struct inode *orig = ctx->orig_inode;
> +	struct inode *donor = ctx->donor_inode;
> +	__u64 len;
> +	int ret;
> +
> +	/* Sub-page block alignment check when blocksize (1024) < PAGE_SIZE */
> +	orig->i_blkbits = 10;
> +	donor->i_blkbits = 10;
> +	i_size_write(orig, 32 * 1024);
> +	i_size_write(donor, 32 * 1024);
> +
> +	len = 4;
> +	ret = ext4_mext_check_adjust_range_test(orig, donor, 1, 2, &len);
> +	KUNIT_EXPECT_EQ(test, ret, -EINVAL);
> +
> +	len = 4;
> +	ret = ext4_mext_check_adjust_range_test(orig, donor, 1,
> +						1 + (PAGE_SIZE >> 10), &len);
> +	KUNIT_EXPECT_EQ(test, ret, 0);
> +	KUNIT_EXPECT_EQ(test, len, 4ULL);

[Severity: Medium]
Does this sub-page alignment check fail on kernels with 64KB pages?

With i_blkbits set to 10 and i_size set to 32KB, donor_eof is 32 blocks,
while donor_start = 1 + (PAGE_SIZE >> 10) evaluates to 65 when PAGE_SIZE
is 65536.  mext_check_adjust_range() sees donor_eof <= donor_start, clamps
*len to 0, and returns -EINVAL instead of 0.  Sizing orig and donor
relative to PAGE_SIZE (for example, max_t(loff_t, 32 * 1024, 4 * PAGE_SIZE))
would avoid this.

[ ... ]

> +static void test_move_extents_written_copy_data_and_split(struct kunit *test)
> +{
> +	struct mext_test_ctx *ctx = mext_setup_ctx(test);
> +	struct inode *orig = ctx->orig_inode;
> +	struct inode *donor = ctx->donor_inode;
> +	ext4_lblk_t lblk;
> +	__u64 moved_len = 0;
> +	int ret;
> +
> +	/*
> +	 * orig:  [0, 6) written -> pblk 400..405, uptodate folios pre-filled
> +	 * donor: [1, 2) written -> pblk 501, [2, 3) hole,
> +	 *        [3, 5) unwritten -> pblk 503..504
> +	 *
> +	 * Moving [1, 5) exercises MEXT_COPY_DATA, left/right extent splits on
> +	 * orig, donor hole skipping at lblk 2, written preservation on orig,
> +	 * mark_unwritten=1 on donor, and page cache data preservation.
> +	 */
> +	i_size_write(orig, 6 * MEXT_TEST_BLK_SIZE);
> +	EXT4_I(orig)->i_disksize = 6 * MEXT_TEST_BLK_SIZE;
> +	i_size_write(donor, 6 * MEXT_TEST_BLK_SIZE);
> +	EXT4_I(donor)->i_disksize = 6 * MEXT_TEST_BLK_SIZE;
> +
> +	mext_insert_extent(test, orig, 0, 6, 400, false);
> +	mext_insert_extent(test, donor, 1, 1, 501, false);
> +	mext_insert_extent(test, donor, 3, 2, 503, true);
> +
> +	for (lblk = 0; lblk < 6; lblk++)
> +		mext_populate_folio_block(test, orig, lblk, 0xA0 + lblk);
> +
> +	ret = ext4_move_extents(&ctx->orig_file, &ctx->donor_file, 1, 1, 4,
> +				&moved_len);
> +	KUNIT_EXPECT_EQ(test, ret, 0);
> +	KUNIT_EXPECT_EQ(test, moved_len, 3ULL);

[Severity: Medium]
Does ext4_move_extents() fail with -EBUSY here when PAGE_SIZE > 4096?

With a 4KB block size on 16KB or 64KB page kernels, logical blocks 1 and 3
share folio 0 in orig->i_mapping.  Moving lblk 1 via MEXT_COPY_DATA calls
mext_folio_mkwrite() -> block_commit_write(), which attaches buffer_heads
to orig's folio 0 and marks lblk 1's buffer dirty.  After skipping the
donor hole at lblk 2, mext_move_extent() tries to move lblk 3..4 in the
same folio and calls filemap_release_folio(folio[0], 0) ->
ext4_release_folio() -> try_to_free_buffers(), which refuses to drop the
dirty buffer from lblk 1 and aborts the move with -EBUSY and moved_len = 1.

Similarly, in test_move_extents_unwritten_orig_and_ebusy(), moving
lblk 0..1 leaves dirty buffers on orig's folio 0 when PAGE_SIZE >= 16384,
so the subsequent move of lblk 2 fails filemap_release_folio() on orig
before reaching the PG_checked donor_folio under test.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112040638.1576836-1-tytso@mit.edu?part=25