On Sat, Aug 15, 2026 at 02:58:52AM +0100, Kiryl Shutsemau wrote:
> From: "Kiryl Shutsemau (Meta)" <[email protected]>
>
> Race and functional tests need to drive khugepaged in step: set up a
> layout, let one full scan pass over it, check the result. The khugepaged
> selftest already waits for full_scans to advance by two, but only makes
> progress if scan_sleep_millisecs happens to be short.
>
> Lift it into khugepaged_full_pass() and drive it through sysfs: any store
> to scan_sleep_millisecs wakes the daemon, so the barrier completes whatever
> the scan cadence. A store can be lost when the daemon is between scans, so
> it keeps storing until the pass lands; a store to an awake daemon costs
> nothing and queues no extra pass.
>
> One wake completes one pass only if the whole mm list fits in a scan batch,
> so callers need a large pages_to_scan.
>
> Settings pushes must not start passes either. A store to either sleep knob
> wakes the daemon, so thp_write_settings() now writes a khugepaged knob only
> when its value changes. The other knobs do not wake, but writing them
> uniformly costs nothing. thp_update_num() is exported for tests that want
> the same restraint.
>
> Assisted-by: Claude-Code:claude-opus-5
> Tested-by: Muhammad Usama Anjum <[email protected]>
> Signed-off-by: Kiryl Shutsemau (Meta) <[email protected]>
> ---
> .../testing/selftests/mm/hugepage_settings.c | 74 ++++++++++++++++---
> .../testing/selftests/mm/hugepage_settings.h | 3 +
> 2 files changed, 68 insertions(+), 9 deletions(-)
>
> diff --git a/tools/testing/selftests/mm/hugepage_settings.c
> b/tools/testing/selftests/mm/hugepage_settings.c
> index d7917dce3aba..992efee17b71 100644
> --- a/tools/testing/selftests/mm/hugepage_settings.c
> +++ b/tools/testing/selftests/mm/hugepage_settings.c
> @@ -183,6 +183,19 @@ void thp_read_settings(struct thp_settings *settings)
> }
> }
>
> +/*
> + * Write only on change: a store to either sleep knob wakes khugepaged --
> + * __sleep_millisecs_store() clears khugepaged_sleep_expire and wakes the
> + * queue -- and settings pushes/pops must not start scan passes nobody
> + * asked for; khugepaged_full_pass() is the only sanctioned wake. The
> + * other knobs do not wake, but writing them the same way costs nothing.
> + */
This seems quite schloppy and way too much information in a single dense
comment.
You/claude don't need to give your life story in every comment :)
Smaller, clearer, human-written please.
/*
> +void thp_update_num(const char *name, unsigned long num)
> +{
> + if (thp_read_num(name) != num)
> + thp_write_num(name, num);
> +}
> +
> void thp_write_settings(struct thp_settings *settings)
> {
> struct khugepaged_settings *khugepaged = &settings->khugepaged;
> @@ -198,15 +211,15 @@ void thp_write_settings(struct thp_settings *settings)
> shmem_enabled_strings[settings->shmem_enabled]);
> thp_write_num("use_zero_page", settings->use_zero_page);
>
> - thp_write_num("khugepaged/defrag", khugepaged->defrag);
> - thp_write_num("khugepaged/alloc_sleep_millisecs",
> - khugepaged->alloc_sleep_millisecs);
> - thp_write_num("khugepaged/scan_sleep_millisecs",
> - khugepaged->scan_sleep_millisecs);
> - thp_write_num("khugepaged/max_ptes_none", khugepaged->max_ptes_none);
> - thp_write_num("khugepaged/max_ptes_swap", khugepaged->max_ptes_swap);
> - thp_write_num("khugepaged/max_ptes_shared",
> khugepaged->max_ptes_shared);
> - thp_write_num("khugepaged/pages_to_scan", khugepaged->pages_to_scan);
> + thp_update_num("khugepaged/defrag", khugepaged->defrag);
> + thp_update_num("khugepaged/alloc_sleep_millisecs",
> + khugepaged->alloc_sleep_millisecs);
> + thp_update_num("khugepaged/scan_sleep_millisecs",
> + khugepaged->scan_sleep_millisecs);
> + thp_update_num("khugepaged/max_ptes_none", khugepaged->max_ptes_none);
> + thp_update_num("khugepaged/max_ptes_swap", khugepaged->max_ptes_swap);
> + thp_update_num("khugepaged/max_ptes_shared",
> khugepaged->max_ptes_shared);
> + thp_update_num("khugepaged/pages_to_scan", khugepaged->pages_to_scan);
>
> if (dev_queue_read_ahead_path[0])
> write_num(dev_queue_read_ahead_path, settings->read_ahead_kb);
> @@ -230,6 +243,49 @@ void thp_write_settings(struct thp_settings *settings)
> }
> }
>
> +/*
> + * Completion barrier for khugepaged: wait until a full scan pass that
> + * started after this call has finished. full_scans must advance by two;
> + * a +1 step may complete a pass that examined this mm before the
> + * caller's setup was in place.
> + *
> + * Any store to scan_sleep_millisecs wakes the daemon, so the barrier works
> + * whatever the configured scan cadence -- but a store can be lost.
> + * __sleep_millisecs_store() clears khugepaged_sleep_expire and wakes the
> + * queue; if the daemon is between scans rather than sleeping, it sets
> + * khugepaged_sleep_expire itself on the way into khugepaged_wait_work() and
> + * then sleeps for the full interval, having never seen the store. So keep
> + * storing until the pass lands; a store while the daemon is awake costs
> + * nothing and does not queue an extra pass.
> + *
> + * One wake completes one full pass only if the whole mm list fits in
> + * one scan batch, so callers must pair this with a large
> + * pages_to_scan.
> + */
Same comment about the comment. This is dense and schloppy. Clarity and
terseness please.
> +bool khugepaged_full_pass(unsigned int timeout_s)
> +{
> + unsigned long deadline_ms = timeout_s * 1000UL;
> + unsigned long sleep_ms =
> + thp_read_num("khugepaged/scan_sleep_millisecs");
> + unsigned long elapsed_ms = 0;
> + int pass;
> +
> + for (pass = 0; pass < 2; pass++) {
> + unsigned long target =
> + thp_read_num("khugepaged/full_scans") + 1;
> +
> + while (thp_read_num("khugepaged/full_scans") < target) {
> + if (elapsed_ms >= deadline_ms)
> + return false;
> + thp_write_num("khugepaged/scan_sleep_millisecs",
> + sleep_ms);
> + usleep(10 * 1000);
> + elapsed_ms += 10;
> + }
> + }
> + return true;
> +}
> +
> struct thp_settings *thp_current_settings(void)
> {
> if (!settings_index) {
> diff --git a/tools/testing/selftests/mm/hugepage_settings.h
> b/tools/testing/selftests/mm/hugepage_settings.h
> index 726c73c43c05..ba7d38370d43 100644
> --- a/tools/testing/selftests/mm/hugepage_settings.h
> +++ b/tools/testing/selftests/mm/hugepage_settings.h
> @@ -70,6 +70,7 @@ int thp_read_string(const char *name, const char * const
> strings[]);
> void thp_write_string(const char *name, const char *val);
> unsigned long thp_read_num(const char *name);
> void thp_write_num(const char *name, unsigned long num);
> +void thp_update_num(const char *name, unsigned long num);
>
> void thp_write_settings(struct thp_settings *settings);
> void thp_read_settings(struct thp_settings *settings);
> @@ -83,6 +84,8 @@ static inline void thp_save_settings(void)
> hugepage_save_settings(/* thp = */ true, /* hugetlb = */ false);
> }
>
> +bool khugepaged_full_pass(unsigned int timeout_s);
> +
> void thp_set_read_ahead_path(char *path);
> unsigned long thp_supported_orders(void);
> unsigned long thp_shmem_supported_orders(void);
> --
> 2.54.0
>
--
Cheers, Lorenzo
