lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1d82c930-be52-4983-9fd7-099df487eb48@redhat.com>
Date: Thu, 3 Jul 2025 11:25:52 +0200
From: David Hildenbrand <david@...hat.com>
To: Dev Jain <dev.jain@....com>, akpm@...ux-foundation.org
Cc: ziy@...dia.com, baolin.wang@...ux.alibaba.com,
 lorenzo.stoakes@...cle.com, Liam.Howlett@...cle.com, npache@...hat.com,
 ryan.roberts@....com, baohua@...nel.org, linux-mm@...ck.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] khugepaged: Reduce race probability between migration
 and khugepaged

On 03.07.25 07:48, Dev Jain wrote:
> Suppose a folio is under migration, and khugepaged is also trying to
> collapse it. collapse_pte_mapped_thp() will retrieve the folio from the
> page cache via filemap_lock_folio(), thus taking a reference on the folio
> and sleeping on the folio lock, since the lock is held by the migration
> path. Migration will then fail in
> __folio_migrate_mapping -> folio_ref_freeze. Reduce the probability of
> such a race happening (leading to migration failure) by bailing out
> if we detect a PMD is marked with a migration entry.
> 
> This fixes the migration-shared-anon-thp testcase failure on Apple M3.
> 
> Note that, this is not a "fix" since it only reduces the chance of
> interference of khugepaged with migration, wherein both the kernel
> functionalities are deemed "best-effort".
> 
> Signed-off-by: Dev Jain <dev.jain@....com>
> ---
> 
> v1->v2:
>   - Remove SCAN_PMD_MIGRATION, merge into SCAN_PMD_MAPPED (David, Anshuman)
>   - Add a comment (Lorenzo)
> 
> v1:
>   - https://lore.kernel.org/all/20250630044837.4675-1-dev.jain@arm.com/
> 
>   mm/khugepaged.c | 9 +++++++++
>   1 file changed, 9 insertions(+)
> 
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index 1aa7ca67c756..3fdefc4f4984 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -941,6 +941,15 @@ static inline int check_pmd_state(pmd_t *pmd)
>   
>   	if (pmd_none(pmde))
>   		return SCAN_PMD_NONE;
> +
> +	/*
> +	 * The folio may be under migration when khugepaged is trying to
> +	 * collapse it. Migration success or failure will eventually end
> +	 * up with the PMD still pointing to a PMD-order folio, so return
> +	 * SCAN_PMD_MAPPED.

Nit: the last part (, so return ..) is obvious from the code.

I would have written

/*
  * The folio may be under migration when khugepaged is trying to
  * collapse it. Migration success or failure will eventually end
  * up with a present PMD entry again.
  */


Acked-by: David Hildenbrand <david@...hat.com>

-- 
Cheers,

David / dhildenb


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ