Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933692AbcCIUEA (ORCPT ); Wed, 9 Mar 2016 15:04:00 -0500 Received: from mga09.intel.com ([134.134.136.24]:21848 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751994AbcCIUDy (ORCPT ); Wed, 9 Mar 2016 15:03:54 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,312,1455004800"; d="scan'208";a="933276485" Date: Wed, 9 Mar 2016 15:03:45 -0500 From: Matthew Wilcox To: Andrea Arcangeli Cc: Ingo Molnar , akpm@linux-foundation.org, hpa@zytor.com, mingo@elte.hu, tglx@linutronix.de, linux-kernel@vger.kernel.org, Jeremy Fitzhardinge Subject: Re: + x86-add-support-for-pud-sized-transparent-hugepages-checkpatch-fixes.patch added to -mm tree Message-ID: <20160309200345.GA26114@linux.intel.com> References: <56b13381.v0wS03ZQEKxwivVW%akpm@linux-foundation.org> <20160203074835.GB32652@gmail.com> <20160304203018.GC5530@linux.intel.com> <20160309174009.GR14882@redhat.com> <20160309184540.GC2464@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160309184540.GC2464@linux.intel.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5453 Lines: 179 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.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(-) diff --git a/arch/x86/include/asm/pgtable-2level.h b/arch/x86/include/asm/pgtable-2level.h index fd74a11..520318f 100644 --- a/arch/x86/include/asm/pgtable-2level.h +++ b/arch/x86/include/asm/pgtable-2level.h @@ -42,17 +42,11 @@ static inline pte_t native_ptep_get_and_clear(pte_t *xp) { return __pte(xchg(&xp->pte_low, 0)); } -#else -#define native_ptep_get_and_clear(xp) native_local_ptep_get_and_clear(xp) -#endif -#ifdef CONFIG_SMP static inline pmd_t native_pmdp_get_and_clear(pmd_t *xp) { return __pmd(xchg((pmdval_t *)xp, 0)); } -#else -#define native_pmdp_get_and_clear(xp) native_local_pmdp_get_and_clear(xp) #endif /* Bit manipulation helper on pte/pgoff entry */ diff --git a/arch/x86/include/asm/pgtable-3level.h b/arch/x86/include/asm/pgtable-3level.h index cdaa58c..b1b6412 100644 --- a/arch/x86/include/asm/pgtable-3level.h +++ b/arch/x86/include/asm/pgtable-3level.h @@ -149,11 +149,7 @@ static inline pte_t native_ptep_get_and_clear(pte_t *ptep) return res; } -#else -#define native_ptep_get_and_clear(xp) native_local_ptep_get_and_clear(xp) -#endif -#ifdef CONFIG_SMP union split_pmd { struct { u32 pmd_low; @@ -161,6 +157,7 @@ union split_pmd { }; pmd_t pmd; }; + static inline pmd_t native_pmdp_get_and_clear(pmd_t *pmdp) { union split_pmd res, *orig = (union split_pmd *)pmdp; @@ -172,8 +169,6 @@ static inline pmd_t native_pmdp_get_and_clear(pmd_t *pmdp) return res.pmd; } -#else -#define native_pmdp_get_and_clear(xp) native_local_pmdp_get_and_clear(xp) #endif /* Encode and de-code a swap entry */ diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h index 1ff49ec..35306ca 100644 --- a/arch/x86/include/asm/pgtable.h +++ b/arch/x86/include/asm/pgtable.h @@ -740,6 +740,11 @@ static inline pmd_t native_local_pmdp_get_and_clear(pmd_t *pmdp) return res; } +#ifndef CONFIG_SMP +#define native_ptep_get_and_clear(p) native_local_ptep_get_and_clear(p) +#define native_pmdp_get_and_clear(p) native_local_pmdp_get_and_clear(p) +#endif + static inline void native_set_pte_at(struct mm_struct *mm, unsigned long addr, pte_t *ptep , pte_t pte) { diff --git a/arch/x86/include/asm/pgtable_64.h b/arch/x86/include/asm/pgtable_64.h index 2ee7811..a0c0219 100644 --- a/arch/x86/include/asm/pgtable_64.h +++ b/arch/x86/include/asm/pgtable_64.h @@ -70,31 +70,17 @@ static inline void native_pmd_clear(pmd_t *pmd) native_set_pmd(pmd, native_make_pmd(0)); } +#ifdef CONFIG_SMP 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 } static inline pmd_t native_pmdp_get_and_clear(pmd_t *xp) { -#ifdef CONFIG_SMP return native_make_pmd(xchg(&xp->pmd, 0)); -#else - /* native_local_pmdp_get_and_clear, - but duplicated because of cyclic dependency */ - pmd_t ret = *xp; - native_pmd_clear(xp); - return ret; -#endif } +#endif static inline void native_set_pud(pud_t *pudp, pud_t pud) {