Peter Zijlstra <peterz@xxxxxxxxxxxxx> writes: > On Thu, Jun 18, 2020 at 12:21:22AM +1000, Michael Ellerman wrote: >> Peter Zijlstra <peterz@xxxxxxxxxxxxx> writes: >> > On Mon, Jun 15, 2020 at 12:57:59PM +0000, Christophe Leroy wrote: > >> >> +#if defined(CONFIG_PPC_8xx) && defined(CONFIG_PPC_16K_PAGES) >> >> +#define __HAVE_ARCH_PTEP_GET >> >> +static inline pte_t ptep_get(pte_t *ptep) >> >> +{ >> >> + pte_t pte = {READ_ONCE(ptep->pte), 0, 0, 0}; >> >> + >> >> + return pte; >> >> +} >> >> +#endif >> > >> > Would it make sense to have a comment with this magic? The casual reader >> > might wonder WTH just happened when he stumbles on this :-) >> >> I tried writing a helpful comment but it's too late for my brain to form >> sensible sentences. >> >> Christophe can you send a follow-up with a comment explaining it? In >> particular the zero entries stand out, it's kind of subtle that those >> entries are only populated with the right value when we write to the >> page table. > > static inline pte_t ptep_get(pte_t *ptep) > { > unsigned long val = READ_ONCE(ptep->pte); > /* 16K pages have 4 identical value 4K entries */ > pte_t pte = {val, val, val, val); > return pte; > } > > Maybe something like that? I think val wants to be pte_basic_t, but otherwise yeah I like that much better. cheers