Re: [PATCH v10 10/13] khugepaged: kick khugepaged for enabling none-PMD-sized mTHPs

[Lorenzo Stoakes <lorenzo.stoakes@oracle.com>]

On Tue, Aug 19, 2025 at 07:42:02AM -0600, Nico Pache wrote:
> From: Baolin Wang <baolin.wang@linux.alibaba.com>
>
> When only non-PMD-sized mTHP is enabled (such as only 64K mTHP enabled),

I don't think this example is very useful, probably just remove it.

Also 'non-PMD-sized mTHP' implies there is such a thing as PMD-sized mTP :)

> we should also allow kicking khugepaged to attempt scanning and collapsing

What is kicking? I think this should be rephrased to something like 'we should
also allow khugepaged to attempt scanning...'

> 64K mTHP. Modify hugepage_pmd_enabled() to support mTHP collapse, and

64K mTHP -> "of mTHP ranges". Put the 'Modify...' bit in a new paragraph to
be clear.

> while we are at it, rename it to make the function name more clear.

To make this clearer let me suggest:

	In order for khugepaged to operate when only mTHP sizes are
	specified in sysfs, we must modify the predicate function that
	determines whether it ought to run to do so.

	This function is currently called hugepage_pmd_enabled(), this
	patch renames it to hugepage_enabled() and updates the logic to
	check to determine whether any valid orders may exist which would
	justify khugepaged running.

>
> Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>
> Signed-off-by: Nico Pache <npache@redhat.com>

> ---
>  mm/khugepaged.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index 2cadd07341de..81d2ffd56ab9 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -430,7 +430,7 @@ static inline int collapse_test_exit_or_disable(struct mm_struct *mm)
>  		mm_flags_test(MMF_DISABLE_THP_COMPLETELY, mm);
>  }
>
> -static bool hugepage_pmd_enabled(void)
> +static bool hugepage_enabled(void)
>  {
>  	/*
>  	 * We cover the anon, shmem and the file-backed case here; file-backed
> @@ -442,11 +442,11 @@ static bool hugepage_pmd_enabled(void)

The comment above this still references PMD-sized, please make sure to update
comments when you change the described behaviour, as it is now incorrect:

	/*
	 * We cover the anon, shmem and the file-backed case here; file-backed
	 * hugepages, when configured in, are determined by the global control.
	 * Anon pmd-sized hugepages are determined by the pmd-size control.
	 * Shmem pmd-sized hugepages are also determined by its pmd-size control,
	 * except when the global shmem_huge is set to SHMEM_HUGE_DENY.
	 */

Please correct this.

>  	if (IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS) &&
>  	    hugepage_global_enabled())
>  		return true;
> -	if (test_bit(PMD_ORDER, &huge_anon_orders_always))
> +	if (READ_ONCE(huge_anon_orders_always))
>  		return true;
> -	if (test_bit(PMD_ORDER, &huge_anon_orders_madvise))
> +	if (READ_ONCE(huge_anon_orders_madvise))
>  		return true;
> -	if (test_bit(PMD_ORDER, &huge_anon_orders_inherit) &&
> +	if (READ_ONCE(huge_anon_orders_inherit) &&
>  	    hugepage_global_enabled())

I guess READ_ONCE() is probably sufficient here as memory ordering isn't
important here, right?

>  		return true;
>  	if (IS_ENABLED(CONFIG_SHMEM) && shmem_hpage_pmd_enabled())
> @@ -490,7 +490,7 @@ void khugepaged_enter_vma(struct vm_area_struct *vma,
>  			  vm_flags_t vm_flags)
>  {
>  	if (!mm_flags_test(MMF_VM_HUGEPAGE, vma->vm_mm) &&
> -	    hugepage_pmd_enabled()) {
> +	    hugepage_enabled()) {
>  		unsigned long orders = vma_is_anonymous(vma) ?
>  					THP_ORDERS_ALL_ANON : BIT(PMD_ORDER);
>
> @@ -2762,7 +2762,7 @@ static unsigned int collapse_scan_mm_slot(unsigned int pages, int *result,
>
>  static int khugepaged_has_work(void)
>  {
> -	return !list_empty(&khugepaged_scan.mm_head) && hugepage_pmd_enabled();
> +	return !list_empty(&khugepaged_scan.mm_head) && hugepage_enabled();
>  }
>
>  static int khugepaged_wait_event(void)
> @@ -2835,7 +2835,7 @@ static void khugepaged_wait_work(void)
>  		return;
>  	}
>
> -	if (hugepage_pmd_enabled())
> +	if (hugepage_enabled())
>  		wait_event_freezable(khugepaged_wait, khugepaged_wait_event());
>  }
>
> @@ -2866,7 +2866,7 @@ static void set_recommended_min_free_kbytes(void)
>  	int nr_zones = 0;
>  	unsigned long recommended_min;
>
> -	if (!hugepage_pmd_enabled()) {
> +	if (!hugepage_enabled()) {
>  		calculate_min_free_kbytes();
>  		goto update_wmarks;
>  	}
> @@ -2916,7 +2916,7 @@ int start_stop_khugepaged(void)
>  	int err = 0;
>
>  	mutex_lock(&khugepaged_mutex);
> -	if (hugepage_pmd_enabled()) {
> +	if (hugepage_enabled()) {
>  		if (!khugepaged_thread)
>  			khugepaged_thread = kthread_run(khugepaged, NULL,
>  							"khugepaged");
> @@ -2942,7 +2942,7 @@ int start_stop_khugepaged(void)
>  void khugepaged_min_free_kbytes_update(void)
>  {
>  	mutex_lock(&khugepaged_mutex);
> -	if (hugepage_pmd_enabled() && khugepaged_thread)
> +	if (hugepage_enabled() && khugepaged_thread)
>  		set_recommended_min_free_kbytes();
>  	mutex_unlock(&khugepaged_mutex);
>  }
> --
> 2.50.1
>