Message-ID: <150ffcb7-2df2-4f3a-a12e-9807f13c6ab9@arm.com>
Date: Wed, 26 Nov 2025 16:34:51 +0000
From: Ryan Roberts <ryan.roberts@....com>
To: "David Hildenbrand (Red Hat)" <david@...nel.org>,
 Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
Cc: Wei Yang <richard.weiyang@...il.com>,
 Samuel Holland <samuel.holland@...ive.com>,
 Palmer Dabbelt <palmer@...belt.com>, Paul Walmsley <pjw@...nel.org>,
 linux-riscv@...ts.infradead.org, Andrew Morton <akpm@...ux-foundation.org>,
 linux-mm@...ck.org, devicetree@...r.kernel.org,
 Suren Baghdasaryan <surenb@...gle.com>, linux-kernel@...r.kernel.org,
 Mike Rapoport <rppt@...nel.org>, Michal Hocko <mhocko@...e.com>,
 Conor Dooley <conor@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Alexandre Ghiti <alex@...ti.fr>, Emil Renner Berthing <kernel@...il.dk>,
 Rob Herring <robh+dt@...nel.org>, Vlastimil Babka <vbabka@...e.cz>,
 "Liam R . Howlett" <Liam.Howlett@...cle.com>,
 Julia Lawall <Julia.Lawall@...ia.fr>, Nicolas Palix <nicolas.palix@...g.fr>,
 Anshuman Khandual <anshuman.khandual@....com>
Subject: Re: [PATCH v3 06/22] mm: Always use page table accessor functions

On 26/11/2025 16:07, Ryan Roberts wrote:
> On 26/11/2025 15:12, David Hildenbrand (Red Hat) wrote:
>> On 11/26/25 16:08, Lorenzo Stoakes wrote:
>>> On Wed, Nov 26, 2025 at 03:56:13PM +0100, David Hildenbrand (Red Hat) wrote:
>>>> On 11/26/25 15:52, Lorenzo Stoakes wrote:
>>>>>
>>>>> Would the pmdp_get() never get invoked then? Or otherwise wouldn't that end up
>>>>> requiring a READ_ONCE() further up the stack?
>>>>
>>>> See my other reply, I think the pmdp_get() is required because all pud_*
>>>> functions are just simple stubs.
>>>
>>> OK, thought you were saying we should push further down the stack? Or up
>>> depending on how you view these things :P as in READ_ONCE at leaf?
>>
>> I think at leaf because I think the previous ones should essentially be only
>> used by stubs.
>>
>> But I haven't fully digested how this is all working. Or supposed to work.
>>
>> I'm trying to chew through the arch/arm/include/asm/pgtable-2level.h example to
>> see if I can make sense of it,
> 
> I wonder if we can think about this slightly differently;
> 
> READ_ONCE() has two important properties:
> 
>  - It guarrantees that a load will be issued, *even if output is unused*
>  - It guarrantees that the read will be single-copy-atomic (no tearing)
> 
> I think for the existing places where READ_ONCE() is used for pagetable reads we
> only care about:
> 
>  - It guarrantees that a load will be issued, *if output is used*
>  - It guarrantees that the read will be single-copy-atomic (no tearing)
> 
> I think if we can weaken to the "if output is used" property, then the compiler
> will optimize out all the unneccessary reads.
> 
> AIUI, a C dereference provides neither of the guarrantees so that's no good.
> 
> What about non-volatile asm? I'm told (thought need to verify) that for
> non-volatile asm, the compiler will emit it if the output is used and remove it
> otherwise. So if the asm contains the required single-copy-atomic, perhaps we
> are in business?
> 
> So we would need a new READ_SCA() macro that could default to READ_ONCE() (which
> is stronger) and arches could opt in to providing a weaker asm version. Then the
> default pXdp_get() could be READ_SCA(). And this should work for all cases.
> 
> I think.

I'm not sure this works. It looks like the compiler is free to move non-volatile
asm sections which might be problematic for places where we are currently using
READ_ONCE() in lockless algorithms, (e.g. GUP?). We wouldn't want to end up with
a stale value.

Another idea:

Given the main pattern where we are aiming to optimize out the read is something
like:

if (!pud_present(*pud))

where for a folded pmd:

static inline int pud_present(pud_t pud)	{ return 1; }

And we will change it to this:

if (!pud_present(pudp_get(pud)))

...

perhaps we can just define the folded pXd_present(), pXd_none(), pXd_bad(),
pXd_user() and pXd_leaf() as macros:

#define pud_present(pud)	1

Then the compiler will never even see the pudp_get().

Thanks,
Ryan


> 
>>
>>>
>>> Anyway. I am now designating you the expert at this ;)
>>
>> Oh no. :)
>>
>>>
>>>>
>>>>>
>>>>>>
>>>>>> IOW, push the READ_ONCE() down to the lowest level so the previous ones
>>>>>> (that will get essentially ignore?) will get folded into the last
>>>>>> READ_ONCE()?
>>>>>>
>>>>>> But my head still hurts and I am focusing on something else concurrently :)
>>>>>
>>>>> Even if we could make this work, I don't love that there's some implicit
>>>>> assumption there that could easily break later on.
>>>>>
>>>>> I'd rather we kept it as stupid/obvious as possible...
>>>>
>>>> Looking at include/asm-generic/pgtable-nopmd.h I am not sure we are talking
>>>> about implicit assumptions here. It's kind-of the design that the pud_t
>>>> values are dummies, so why shoul the pudp_get() give you any guarantees.
>>>>
>>>> At least that's my current understanding, which might be very flawed :)
>>>
>>> I mean I'm waving my hands around like I'm working on an aircraft carrier here
>>> so if you're _sure_ it's _absolutely_ safe then fine :)
>>
>> Well, not yet ... :)
>>
> 

Powered by blists - more mailing lists