Re: [PATCH 2/5] mm,hugetlb: Document the reason to lock the folio in the faulting path

[David Hildenbrand <david@redhat.com>]

On 12.06.25 15:46, Oscar Salvador wrote:
> Recent conversations showed that there was a misunderstanding about why we
> were locking the folio prior to calling hugetlb_wp().
> Document explicitly why we need to take the lock, explaining on the way that
> although the timespan for the locking of anonymous and file folios is different,
> it would require a major surgery to represent that difference with the current
> code.
> 
> Signed-off-by: Oscar Salvador <osalvador@suse.de>
> ---
>   mm/hugetlb.c | 18 +++++++++++++++++-
>   1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 175edafeec67..dfa09fc3b2c6 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -6537,6 +6537,10 @@ static vm_fault_t hugetlb_no_page(struct address_space *mapping,
>   			}
>   			new_pagecache_folio = true;
>   		} else {
> +			/*
> +			 * hugetlb_wp() expects the folio to be locked in order to
> +			 * check whether we can re-use this page exclusively for us.
> +			 */
>   			folio_lock(folio);
>   			anon_rmap = 1;
>   		}
> @@ -6801,7 +6805,19 @@ vm_fault_t hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma,
>   		/* Fallthrough to CoW */
>   	}
>   
> -	/* hugetlb_wp() requires page locks of pte_page(vmf.orig_pte) */
> +	/*
> +	 * We need to lock the folio before calling hugetlb_wp().
> +	 * Either the folio is in the pagecache and we need to copy it over
> +	 * to another file, so it must remain stable throughout the operation,

But as discussed, why is that the case? We don't need that for ordinary 
pages, and existing folio mappings can already concurrently modify the page?

-- 
Cheers,

David / dhildenb