On Sat, Oct 22, 2022 at 4:48 AM Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
>
> static inline void native_set_pte(pte_t *ptep, pte_t pte)
> {
> - ptep->pte_high = pte.pte_high;
> + WRITE_ONCE(ptep->pte_high, pte.pte_high);
> smp_wmb();
> - ptep->pte_low = pte.pte_low;
> + WRITE_ONCE(ptep->pte_low, pte.pte_low);
With this, the smp_wmb() should just go away too. It was really only
ever there as a compiler barrier.
Two WRITE_ONCE() statements are inherently ordered for the compiler
(due to volatile rules), and x86 doesn't re-order writes.
It's not a big deal, since smp_wmb() is just a barrier() on x86-64
anyway, but it might make some improvement to code generation to
remove it, and the smp_wmb() really isn't adding anything.
If somebody likes the smp_wmb() as a comment, I think it would be
better to actually _make_ it a comment, and have these functions turn
into just
/* Force ordered word-sized writes, set low word with present bit last */
static inline void native_set_pte(pte_t *ptep, pte_t pte)
{
WRITE_ONCE(ptep->pte_high, pte.pte_high);
WRITE_ONCE(ptep->pte_low, pte.pte_low);
}
or similar. I think that kind of one-liner comment is much more
informative than a "smp_wmb()".
Or do we already have a comment elsewhere about why the ordering is
important (and how *clearing* clears the low word with the present bit
first, but setting a *new* entry sets the high word first so that the
64-bit entry is complete when the present bit is set?)
Linus
