Re: [PATCH 2/3] mm: add KUnit coverage for mappingless swapcache folios
Kairui Song <[email protected]>
| Newsgroups | gmane.linux.kernel,gmane.linux.kernel.mm |
|---|---|
| Message-ID | <CAMgjq7BW_n3HpM=mCGn=q90=4+oXcBLWwZewxvEcwqfzjuewOw@mail.gmail.com> |
On Wed, Aug 5, 2026 at 7:24 PM Shivam Kalra via B4 Relay <[email protected]> wrote: > > From: Shivam Kalra <[email protected]> > > Add focused coverage for the folio_check_splittable() eligibility checks. > Verify that an ordinary mappingless folio is rejected, a mappingless > swapcache folio is accepted, and unsupported target orders and split types > remain rejected. > > Build the test only when KUnit is built in because > folio_check_splittable() is intentionally not exported. > > Signed-off-by: Shivam Kalra <[email protected]> Hello, thanks for the patch. A few concerns on this... > --- > mm/Kconfig | 14 ++++++++++++ > mm/Makefile | 1 + > mm/tests/folio_split_kunit.c | 52 ++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 67 insertions(+) > > diff --git a/mm/Kconfig b/mm/Kconfig > index 331daf7fcfab..164dc1438900 100644 > --- a/mm/Kconfig > +++ b/mm/Kconfig > @@ -1503,6 +1503,20 @@ config LAZY_MMU_MODE_KUNIT_TEST > > If unsure, say N. > > +config FOLIO_SPLIT_KUNIT_TEST > + bool "KUnit tests for folio splitting" if !KUNIT_ALL_TESTS > + depends on KUNIT=y > + depends on TRANSPARENT_HUGEPAGE > + depends on SWAP Just BTW, we already have a tools/testing/selftests/mm/split_huge_page_test.c. > + help > + Enable this option to test folio split eligibility checks. The tests > + verify support for mappingless folios in the swap cache and ensure FOLIO_SPLIT_KUNIT_TEST seems a generic test and not limited to SWAP, but it relys on SWAP, looks a bit strange to me. ... > +static void folio_check_splittable_mappingless_swapcache(struct kunit *test) > +{ > + struct folio *folio; > + int ret; > + > + folio = folio_alloc(GFP_KERNEL, 2); > + KUNIT_ASSERT_NOT_NULL(test, folio); > + folio_lock(folio); > + > + KUNIT_EXPECT_PTR_EQ(test, folio->mapping, NULL); > + ret = folio_check_splittable(folio, 0, SPLIT_TYPE_UNIFORM); > + KUNIT_EXPECT_EQ(test, ret, -EBUSY); > + > + /* Only the eligibility check is exercised here. */ > + folio_set_swapbacked(folio); > + folio_set_swapcache(folio); Faking a swapcache with folio->swap == 0 looks hacky, could go very wrong if you do any actual work on it. > + > + ret = folio_check_splittable(folio, 0, SPLIT_TYPE_UNIFORM); > + KUNIT_EXPECT_EQ(test, ret, 0); > + ret = folio_check_splittable(folio, 1, SPLIT_TYPE_UNIFORM); > + KUNIT_EXPECT_EQ(test, ret, -EINVAL); > + ret = folio_check_splittable(folio, 0, SPLIT_TYPE_NON_UNIFORM); > + KUNIT_EXPECT_EQ(test, ret, -EINVAL); Hmm... you are just testing the gating, not the actual split, that makes the test not very meaningful I think. And we should not block non-uniform or high order split for swap cache after sorting things out, then this test will be doing nothing :)