Re: + x86-add-support-for-pud-sized-transparent-hugepages-checkpatch-fixes.patch added to -mm tree

[Andrea Arcangeli]

On Wed, Mar 09, 2016 at 03:03:45PM -0500, Matthew Wilcox wrote:
> On Wed, Mar 09, 2016 at 01:45:40PM -0500, Matthew Wilcox wrote:
> > > In fact even before starting to fix the comment, I would recommend to
> > > try again to call native_local_pmdp_get_and_clear and
> > > native_local_ptep_get_and_clear to verify if it still breaks, just in
> > > case the include ordering got fixed by accident in the meanwhile (that
> > > was a comment in 2.6.25 when arch/x86/include/asm didn't even exist
> > > yet, it was still in include/asm-x86). If it would manage to build
> > > without the manual expansion, the comment could go and the duplication
> > > as well.
> > 
> > The ordering problem is still there.  native_local_ptep_get_and_clear()
> > is declared at line 726 of asm/pgtable.h and asm/pgtable_64.h is included
> > at line 466 of asm/pgtable.h.
> > 
> > I'll have a little play; see if I can resolve this ...
> 
> asm/pgtable.h:466
> # include <asm/pgtable_64.h>
> 
> asm/pgtable.h:726
> static inline pte_t native_local_ptep_get_and_clear(pte_t *ptep)
> {
>         pte_t res = *ptep;
> 
>         /* Pure native function needs no input for mm, addr */
>         native_pte_clear(NULL, 0, ptep);
>         return res;
> }
> 
> asm/pgtable_64.h:47
> static inline void native_pte_clear(struct mm_struct *mm, unsigned long addr,
>                                     pte_t *ptep)
> {
>         *ptep = native_make_pte(0);
> }
> 
> asm/pgtable_64.h:73
> static inline pte_t native_ptep_get_and_clear(pte_t *xp)
> {
> #ifdef CONFIG_SMP
>         return native_make_pte(xchg(&xp->pte, 0));
> #else
>         /* native_local_ptep_get_and_clear,
>            but duplicated because of cyclic dependency */
>         pte_t ret = *xp;
>         native_pte_clear(NULL, 0, xp);
>         return ret;
> #endif
> }
> 
> Why don't we just convert native_ptep_get_and_clear to work the same way that
> pgtable-2level and pgtable-3level work?  ie:
> 
> #ifdef CONFIG_SMP
> static inline pte_t native_ptep_get_and_clear(pte_t *xp)
> {
>         return native_make_pte(xchg(&xp->pte, 0));
> }
> #else
> #define native_ptep_get_and_clear(xp) native_local_ptep_get_and_clear(xp)
> #endif
> 
> Or perhaps better, centralise the non-SMP definitions:
> 
>  arch/x86/include/asm/pgtable-2level.h |  6 ------
>  arch/x86/include/asm/pgtable-3level.h |  7 +------
>  arch/x86/include/asm/pgtable.h        |  5 +++++
>  arch/x86/include/asm/pgtable_64.h     | 18 ++----------------
>  4 files changed, 8 insertions(+), 28 deletions(-)

Reviewed-by: Andrea Arcangeli <aarcange@xxxxxxxxxx>