linux-kernel - Re: [PATCH -v2 1/2] mm: add spurious fault fixing support for huge pmd

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: <86bb70dd-2602-48f3-a683-5ec62b37fd46@lucifer.local>
Date: Tue, 14 Oct 2025 15:49:07 +0100
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: David Hildenbrand <david@...hat.com>
Cc: Huang Ying <ying.huang@...ux.alibaba.com>,
        Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will@...nel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        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: [PATCH -v2 1/2] mm: add spurious fault fixing support for huge
 pmd

On Tue, Oct 14, 2025 at 04:38:05PM +0200, David Hildenbrand wrote:
>
> >
> > 		/* 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.
> > 		 */
> > 		if (vmf->flags & FAULT_FLAG_WRITE)
> > 			flush_tlb_fix_spurious_fault(vmf->vma, vmf->address,
> > 						     vmf->pte);
> >
> >
> > So I don't see why it's so egregious to have the equivalent here, or actually
> > ideally to abstract the code entirely.
>
> Let's definitely not duplicate such comments whereby one instance will end
> up bitrotting.

We're duplicating the code in two places, how would that bitrot happen exactly?

I mean as I also say, probably better to just de-duplicate in general.

It's one thing referring to a comment _above_ another function, it's quite
another to refer to comments buried in a branch buried inside another function
and to hand wave about it.

And _those_ comments might very well be moved when the function is sensibly
refactored (as it needs to be tbh).

So yeah, in general I'd agree _if_ you were doing something
similar-but-not-the-same AND referencing a clearly identifiable comment
(e.g. above the function).

But this isn't that at all.

Anyway TL;DR is that we should just de-dupe the code.

>
> When talking about spurious faults I assume the educated reader will usually
> find the right comments -- like you easily did :P

Yeah but I'm familiar with the (kinda hideous) code there, it wasn't so much
'easily' found and for somebody else they'd maybe get confused as to what on
earth that's referring to.

>
> However I agree that ...
>
> >
> > In commit b22cc9a9c7ff ("mm/rmap: convert "enum rmap_level" to "enum
> > pgtable_level"") David introduced:
> >
> > 	enum pgtable_level {
> > 		PGTABLE_LEVEL_PTE = 0,
> > 		PGTABLE_LEVEL_PMD,
> > 		PGTABLE_LEVEL_PUD,
> > 		PGTABLE_LEVEL_P4D,
> > 		PGTABLE_LEVEL_PGD,
> > 	};
> >
> > Which allows for sensible abstraction.
>
> ... if there is an easier way to just unify the code and have the comments
> at a central place, even better.

Yup we're agreed on that :)

>
> --
> Cheers
>
> David / dhildenb
>

Cheers, Lorenzo