On 13.10.2026 15:53, [email protected] wrote:
> > selftests/bpf: Add tests for memcg kfuncs
>
> TL;DR:
> - Test issue: the "reclaim clamped" upper-bound assertion in test_reclaim()
> likely fails, since only the reclaim request is clamped, not the result;
> commit message is also inaccurate
> - Minor: silent test__skip() with no message
>
...
> So someone reading a SKIP for memcg_kfuncs/memory_events or
> memcg_kfuncs/reclaim cannot tell why it was skipped. The arena_memcg test
> that this helper resembles prints "%s:SKIP:no disk-backed O_TMPFILE in cwd"
> before calling test__skip().
>
> On the other side, this is minor: the skip is correctly a skip rather than a
> failure, and the commit message documents the skip condition.
>
> Could a similar message be printed here before test__skip()?
>
> [ ... ]
>
> > +static void test_reclaim(struct memcg_kfuncs *skel)
> > +{
> > + long ret = 0;
> > + int fd, i;
> > +
> > + fd = alloc_pagecache_or_skip(PAGECACHE_SIZE);
> > + if (fd < 0)
> > + return;
> > +
> > + /* Ask for more than one batch to check the clamp. */
> > + for (i = 0; i < RECLAIM_RETRIES && ret <= 0; i++)
> > + ret = do_reclaim(skel, 4 * MEMCG_CHARGE_BATCH * page_size, 0);
> > +
> > + ASSERT_GT(ret, 0, "reclaimed");
> > + ASSERT_LE(ret, MEMCG_CHARGE_BATCH * page_size, "reclaim clamped");
> ^^^^
Will do, for both the O_TMPFILE and the missing memory controller case.
>
> Does the "reclaim clamped" assertion hold? bpf_proactive_reclaim() only
> clamps the reclaim target:
>
> nr_pages = min(size / PAGE_SIZE, (unsigned long)MEMCG_CHARGE_BATCH);
> nr_reclaimed = try_to_free_mem_cgroup_pages(memcg, nr_pages, ...);
> return nr_reclaimed * PAGE_SIZE;
>
> Nothing limits what try_to_free_mem_cgroup_pages() actually returns to that
> target. do_try_to_free_pages() checks sc->nr_reclaimed >= sc->nr_to_reclaim
> only after each full priority pass:
>
> do {
> ...
> shrink_zones(zonelist, sc);
> if (sc->nr_reclaimed >= sc->nr_to_reclaim)
> break;
> ...
> } while (--sc->priority >= 0);
>
> In the classic LRU path, shrink_lruvec() checks a local nr_reclaimed
> (starting at 0) against the full sc->nr_to_reclaim, so one pass can add up
> to the whole target on top of what earlier passes already reclaimed.
>
> The MGLRU path is the same: try_to_shrink_lruvec() calls should_abort_scan()
> only after each evict_folios() batch of up to MIN_LRU_BATCH (64) pages.
> Both paths compute the per-pass scan target as lruvec_size >> priority.
>
> With the 32MB (8192 x 4K pages) of clean page cache that test_reclaim()
> creates, and swappiness 0 (SCAN_FILE), priorities 12 through 8 reclaim
> 2 + 4 + 8 + 16 + 32 = 62 pages. That is below nr_to_reclaim = 64, so the
> loop drops to priority 7. There, nr[LRU_INACTIVE_FILE] = 64 and the pass
> reclaims another 64 pages, for a total of about 126 pages (~504KB).
> ASSERT_LE(ret, 64 * page_size) then fails.
>
> For any page cache size around 32MB, the running total jumps from at most 63
> pages straight past 64, so this is not a rare edge case. Large page cache
> folios make it worse: isolate_lru_folios() takes a whole folio even when
> nr_to_scan is smaller, so a single folio bigger than 64 pages already
> exceeds the bound in the first pass. The same arithmetic applies with 64K
> pages.
>
> The commit message says "bpf_proactive_reclaim() reclaims clean page cache
> and the result is clamped to MEMCG_CHARGE_BATCH pages", but only the request
> is clamped, not the result. The kfunc's own kernel-doc describes @size as
> the amount to reclaim (clamped), not as an upper bound on the return value.
>
> Should the upper-bound assertion be dropped or relaxed, and the commit
> message fixed?
>
Right, only the request is clamped, and reclaim can overshoot it
(cgroup-v2.rst says as much for memory.reclaim). I'll drop the upper
bound and fix the commit message in v3.
> > + ASSERT_EQ(ret % page_size, 0, "reclaim page aligned");
> > +
> > + close(fd);
> > +}
>
>
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
>
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/38421980805
--
Regards,
listout
