Re: [RFC PATCH 1/2] mm: add spurious fault fixing support for huge pmd

[David Hildenbrand <david@redhat.com>]

On 15.09.25 05:29, Huang Ying wrote:
> In the current kernel, there is spurious fault fixing support for pte,
> but not for huge pmd because no architectures need it. But in the
> next patch in the series, we will change the write protection fault
> handling logic on arm64, so that some stale huge pmd entries may
> remain in the TLB. These entries need to be flushed via the huge pmd
> spurious fault fixing mechanism.
> 
> Signed-off-by: Huang Ying <ying.huang@linux.alibaba.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: Zi Yan <ziy@nvidia.com>
> Cc: Baolin Wang <baolin.wang@linux.alibaba.com>
> Cc: Ryan Roberts <ryan.roberts@arm.com>
> Cc: Yang Shi <yang@os.amperecomputing.com>
> Cc: "Christoph Lameter (Ampere)" <cl@gentwo.org>
> Cc: Dev Jain <dev.jain@arm.com>
> Cc: Barry Song <baohua@kernel.org>
> Cc: Anshuman Khandual <anshuman.khandual@arm.com>
> Cc: Yicong Yang <yangyicong@hisilicon.com>
> Cc: Kefeng Wang <wangkefeng.wang@huawei.com>
> Cc: Kevin Brodsky <kevin.brodsky@arm.com>
> Cc: Yin Fengwei <fengwei_yin@linux.alibaba.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-mm@kvack.org
> ---

[...]

>   
>   int copy_huge_pmd(struct mm_struct *dst_mm, struct mm_struct *src_mm,
> @@ -1857,7 +1861,20 @@ void huge_pmd_set_accessed(struct vm_fault *vmf)
>   	if (unlikely(!pmd_same(*vmf->pmd, vmf->orig_pmd)))
>   		goto unlock;
>   
> -	touch_pmd(vmf->vma, vmf->address, vmf->pmd, write);
> +	if (!touch_pmd(vmf->vma, vmf->address, vmf->pmd, write)) {
> +		/* Skip spurious TLB flush for retried page fault */
> +		if (vmf->flags & FAULT_FLAG_TRIED)
> +			goto unlock;
> +		/*
> +		 * This is needed only for protection faults but the arch code
> +		 * is not yet telling us if this is a protection fault or not.
> +		 * This still avoids useless tlb flushes for .text page faults
> +		 * with threads.
> +		 */

Can we instead just remove these comments and simplly say "see 
handle_pte_fault()"

> +		if (vmf->flags & FAULT_FLAG_WRITE)
> +			flush_tlb_fix_spurious_fault_pmd(vmf->vma, vmf->address,
> +							 vmf->pmd);
> +	}

Okay, In the PTE case, we call flush_tlb_fix_spurious_fault() during 
write faults if ptep_set_access_flags() returned "0".

You are calling flush_tlb_fix_spurious_fault_pmd() during a write fault 
when pmdp_set_access_flags() returned "0" as well.

In general, LGTM, but I would just let touch_pmd() return the value of 
pmdp_set_access_flags() instead and add a quick comment for touch_pmd() 
what the return value means.

-- 
Cheers

David / dhildenb