Re: [PATCH v2 2/5] mm: avoid unnecessary flush on change_huge_pmd()

Dave Hansen
2021-10-26T19:40:13Z

On 10/26/21 12:06 PM, Nadav Amit wrote:
>
> To make it very clear - consider the following scenario, in which
> a volatile pointer p is mapped using a certain PTE, which is RW
> (i.e., *p is writable):
>
> CPU0 CPU1
> ---- ----
> x = *p
> [ PTE cached in TLB;
> PTE is not dirty ]
> clear_pte(PTE)
> *p = x
> [ needs to set dirty ]
>
> Note that there is no TLB flush in this scenario. The question
> is whether the write access to *p would succeed, setting the
> dirty bit on the clear, non-present entry.
>
> I was under the impression that the hardware AD-assist would
> recheck the PTE atomically as it sets the dirty bit. But, as I
> said, I am not sure anymore whether this is defined architecturally
> (or at least would work in practice on all CPUs modulo the
> Knights Landing thingy).

Practically, at "x=*p", he thing that gets cached in the TLB will
Dirty=0. At the "*p=x", the CPU will decide it needs to do a write,
find the Dirty=0 entry and will entirely discard it. In other words, it
*acts* roughly like this:

x = *p
INVLPG(p)
*p = x;

Where the INVLPG() and the "*p=x" are atomic. So, there's no
_practical_ problem with your scenario. This specific behavior isn't
architectural as far as I know, though.

Although it's pretty much just academic, as for the architecture, are
you getting hung up on the difference between the description of "Accessed":

Whenever the processor uses a paging-structure entry as part of
linear-address translation, it sets the accessed flag in that
entry

and "Dirty:"

Whenever there is a write to a linear address, the processor
sets the dirty flag (if it is not already set) in the paging-
structure entry...

Accessed says "as part of linear-address translation", which means that
the address must have a translation. But, the "Dirty" section doesn't
say that. It talks about "a write to a linear address" but not whether
there is a linear address *translation* involved.

If that's it, we could probably add a bit like:

In addition to setting the accessed flag, whenever there is a
write...

before the dirty rules in the SDM.

Or am I being dense and continuing to miss your point? :)