On Thu, Aug 08, 2024 at 12:22:38AM +0200, Thomas Gleixner wrote:
> On Wed, Aug 07 2024 at 15:48, Peter Xu wrote:
> > An entry should be reported as PUD leaf even if it's PROT_NONE, in which
> > case PRESENT bit isn't there. I hit bad pud without this when testing dax
> > 1G on zapping a PROT_NONE PUD.
>
> That does not qualify as a change log. What you hit is irrelevant unless
> you explain the actual underlying problem. See Documentation/process/
> including the TIP documentation.
Firstly, thanks a lot for the reviews.
I thought the commit message explained exactly what is the underlying
problem, no?
The problem is even if PROT_NONE, as long as the PSE bit is set on the PUD
it should be treated as a PUD leaf. Currently, the code will return
pud_leaf()==false for those PROT_NONE PUD entries, and IMHO that is wrong.
This patch wants to make it right. I still think that's mostly what I put
there in the commit message..
Would you please suggest something so I can try to make it better,
otherwise? Or it'll be helpful too if you could point out which part of
the two documentations I should reference.
>
> > diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h
> > index e39311a89bf4..a2a3bd4c1bda 100644
> > --- a/arch/x86/include/asm/pgtable.h
> > +++ b/arch/x86/include/asm/pgtable.h
> > @@ -1078,8 +1078,7 @@ static inline pmd_t *pud_pgtable(pud_t pud)
> > #define pud_leaf pud_leaf
> > static inline bool pud_leaf(pud_t pud)
> > {
> > - return (pud_val(pud) & (_PAGE_PSE | _PAGE_PRESENT)) ==
> > - (_PAGE_PSE | _PAGE_PRESENT);
> > + return pud_val(pud) & _PAGE_PSE;
> > }
>
> And the changelog does not explain why this change is not affecting any
> existing user of pud_leaf().
That's what I want to do: I want to affect them..
And IMHO it's mostly fine before because mprotect() is broken with 1g
anyway, and I guess nobody managed to populate any pud entry with PROT_NONE
on dax 1g before, and that's what this whole series is trying to fix.
Thanks,
--
Peter Xu
