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: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <8c51da7a-7370-4678-96a3-7cd6eaf0db62@redhat.com>
Date: Mon, 15 Sep 2025 13:08:09 +0200
From: David Hildenbrand <david@...hat.com>
To: Huang Ying <ying.huang@...ux.alibaba.com>,
 Catalin Marinas <catalin.marinas@....com>, Will Deacon <will@...nel.org>,
 Andrew Morton <akpm@...ux-foundation.org>
Cc: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>,
 Vlastimil Babka <vbabka@...e.cz>, Zi Yan <ziy@...dia.com>,
 Baolin Wang <baolin.wang@...ux.alibaba.com>,
 Ryan Roberts <ryan.roberts@....com>, Yang Shi <yang@...amperecomputing.com>,
 "Christoph Lameter (Ampere)" <cl@...two.org>, Dev Jain <dev.jain@....com>,
 Barry Song <baohua@...nel.org>, Anshuman Khandual
 <anshuman.khandual@....com>, Yicong Yang <yangyicong@...ilicon.com>,
 Kefeng Wang <wangkefeng.wang@...wei.com>,
 Kevin Brodsky <kevin.brodsky@....com>,
 Yin Fengwei <fengwei_yin@...ux.alibaba.com>,
 linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
 linux-mm@...ck.org
Subject: Re: [RFC PATCH 1/2] mm: add spurious fault fixing support for huge
 pmd

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@...ux.alibaba.com>
> Cc: Catalin Marinas <catalin.marinas@....com>
> Cc: Will Deacon <will@...nel.org>
> Cc: Andrew Morton <akpm@...ux-foundation.org>
> Cc: David Hildenbrand <david@...hat.com>
> Cc: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
> Cc: Vlastimil Babka <vbabka@...e.cz>
> Cc: Zi Yan <ziy@...dia.com>
> Cc: Baolin Wang <baolin.wang@...ux.alibaba.com>
> Cc: Ryan Roberts <ryan.roberts@....com>
> Cc: Yang Shi <yang@...amperecomputing.com>
> Cc: "Christoph Lameter (Ampere)" <cl@...two.org>
> Cc: Dev Jain <dev.jain@....com>
> Cc: Barry Song <baohua@...nel.org>
> Cc: Anshuman Khandual <anshuman.khandual@....com>
> Cc: Yicong Yang <yangyicong@...ilicon.com>
> Cc: Kefeng Wang <wangkefeng.wang@...wei.com>
> Cc: Kevin Brodsky <kevin.brodsky@....com>
> Cc: Yin Fengwei <fengwei_yin@...ux.alibaba.com>
> Cc: linux-arm-kernel@...ts.infradead.org
> Cc: linux-kernel@...r.kernel.org
> Cc: linux-mm@...ck.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


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ