From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A7E5345A286; Tue, 18 Aug 2026 10:34:49 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787049291; cv=none; b=Ea/+dRMKEQLZvcVkI/nywDisfY14YBPaB7v6DkzIcz++J1hS8a1VYjEdzqmXs0gT2NHcfzu/zQ73k5Sjv6R6uTnavj4/MrSy3XYlUvDSB6wgM2meUTobNMUQm7pLTkRnDuT3qo0ynYhn8/c2RdpvFa8ppGLCt8NfgbmwokxUl/E= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787049291; c=relaxed/simple; bh=VG/5Fsf7AaD1TfIKEsHLmaWqEfDYir/xuqp3+79X210=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=VzU20cJ71K8MPeamCRA/KIJLEpmoDhi02CYKLk7OgWEtaNnK6+IhCGVqWuFFNVqV4tjnHPC56MwCI/39XuiLv1HXHGiHdRBbh3FL5zim7ymXGQnTteTnGU4Eruq4XzseDvU/awzfmFIECENUGVcLCCxXT/qFxJSL+AP1/MCFTnw= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=lr8/Pdix; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="lr8/Pdix" Received: by smtp.kernel.org (Postfix) with ESMTPSA id EF1DE1F000E9; Tue, 18 Aug 2026 10:34:43 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1787049289; bh=RabI4J/BrtvdoCZ9vo+KK3MSA7ivPJAPsrCnENerses=; h=Date:From:To:Cc:Subject:References:In-Reply-To; b=lr8/PdixHC0b0cHQYGs/rrFYXrnRpgGR09yIMWK3/sFkEnbxp9wEKabBlzX1lDuEY QbHSdVMeGPHP24nwk3gEpkJ65/V7QPVzglAuTvteq17oUcWZKIyJvQI3H9u0Um3EJv 7Bp35cQCgv44NRiAD55dqm+F0QC3ia+2vzitjR/jVPMSXDsZe1y1uRPm45CPTcoRR7 Sglqkyaw4yMcPKoXdKslL9kAOEqF75H2E0uv1TA55/nFqDztxUV6ThmrWaM/nxCoqs +BMV0sTBqAUqaETN+y56Xc+MLRm2iqnpBU5ZB3gZKdlPHtTM9tnlw6NLg+ocFYOXrz JX+SkkhyCpejw== Date: Tue, 18 Aug 2026 11:34:27 +0100 From: "Lorenzo Stoakes (ARM)" To: Kiryl Shutsemau Cc: akpm@linux-foundation.org, david@kernel.org, nico.pache@linux.dev, baolin.wang@linux.alibaba.com, baohua@kernel.org, dev.jain@arm.com, hughd@google.com, lance.yang@linux.dev, liam@infradead.org, mhocko@suse.com, rppt@kernel.org, ryan.roberts@arm.com, shuah@kernel.org, surenb@google.com, usama.arif@linux.dev, vbabka@kernel.org, ziy@nvidia.com, usama.anjum@arm.com, agordeev@linux.ibm.com, linux-mm@kvack.org, linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org, kas@kernel.org Subject: Re: [PATCH v4 10/19] selftests/mm: add khugepaged completion barrier helper Message-ID: References: <20260815015901.1236937-1-kirill@shutemov.name> <20260815015901.1236937-11-kirill@shutemov.name> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260815015901.1236937-11-kirill@shutemov.name> On Sat, Aug 15, 2026 at 02:58:52AM +0100, Kiryl Shutsemau wrote: > From: "Kiryl Shutsemau (Meta)" > > 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 > Signed-off-by: Kiryl Shutsemau (Meta) > --- > .../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