| Message ID | 02633d43f29e1ba01865cd334216dc8efb8b4b11.1524587425.git.christophe.leroy@c-s.fr (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 45201c8794693d16e8e634188a92e9c0b075fd91 |
| Headers | show |
| Series | [1/3] powerpc/nohash: remove hash related code from nohash headers. | expand |
Hi Christophe,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on powerpc/next]
[also build test ERROR on v4.17-rc2 next-20180426]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Christophe-Leroy/powerpc-nohash-remove-hash-related-code-from-nohash-headers/20180425-182026
base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-ppc64e_defconfig (attached as .config)
compiler: powerpc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=powerpc
All errors (new ones prefixed by >>):
In file included from arch/powerpc/include/asm/nohash/pgtable.h:6:0,
from arch/powerpc/include/asm/pgtable.h:19,
from include/linux/memremap.h:8,
from include/linux/mm.h:27,
from include/linux/mman.h:5,
from arch/powerpc/kernel/asm-offsets.c:22:
arch/powerpc/include/asm/nohash/64/pgtable.h: In function '__ptep_test_and_clear_young':
>> arch/powerpc/include/asm/nohash/64/pgtable.h:214:6: error: implicit declaration of function 'pte_young'; did you mean 'pte_pud'? [-Werror=implicit-function-declaration]
if (pte_young(*ptep))
^~~~~~~~~
pte_pud
In file included from arch/powerpc/include/asm/pgtable.h:19:0,
from include/linux/memremap.h:8,
from include/linux/mm.h:27,
from include/linux/mman.h:5,
from arch/powerpc/kernel/asm-offsets.c:22:
arch/powerpc/include/asm/nohash/pgtable.h: At top level:
>> arch/powerpc/include/asm/nohash/pgtable.h:20:19: error: static declaration of 'pte_young' follows non-static declaration
static inline int pte_young(pte_t pte) { return pte_val(pte) & _PAGE_ACCESSED; }
^~~~~~~~~
In file included from arch/powerpc/include/asm/nohash/pgtable.h:6:0,
from arch/powerpc/include/asm/pgtable.h:19,
from include/linux/memremap.h:8,
from include/linux/mm.h:27,
from include/linux/mman.h:5,
from arch/powerpc/kernel/asm-offsets.c:22:
arch/powerpc/include/asm/nohash/64/pgtable.h:214:6: note: previous implicit declaration of 'pte_young' was here
if (pte_young(*ptep))
^~~~~~~~~
cc1: some warnings being treated as errors
make[2]: *** [arch/powerpc/kernel/asm-offsets.s] Error 1
make[2]: Target '__build' not remade because of errors.
make[1]: *** [prepare0] Error 2
make[1]: Target 'prepare' not remade because of errors.
make: *** [sub-make] Error 2
vim +214 arch/powerpc/include/asm/nohash/64/pgtable.h
208
209 static inline int __ptep_test_and_clear_young(struct mm_struct *mm,
210 unsigned long addr, pte_t *ptep)
211 {
212 unsigned long old;
213
> 214 if (pte_young(*ptep))
215 return 0;
216 old = pte_update(mm, addr, ptep, _PAGE_ACCESSED, 0, 0);
217 return (old & _PAGE_ACCESSED) != 0;
218 }
219 #define __HAVE_ARCH_PTEP_TEST_AND_CLEAR_YOUNG
220 #define ptep_test_and_clear_young(__vma, __addr, __ptep) \
221 ({ \
222 int __r; \
223 __r = __ptep_test_and_clear_young((__vma)->vm_mm, __addr, __ptep); \
224 __r; \
225 })
226
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
kbuild test robot <lkp@intel.com> writes: > Hi Christophe, > > Thank you for the patch! Yet something to improve: > > [auto build test ERROR on powerpc/next] > [also build test ERROR on v4.17-rc2 next-20180426] > [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] > > url: https://github.com/0day-ci/linux/commits/Christophe-Leroy/powerpc-nohash-remove-hash-related-code-from-nohash-headers/20180425-182026 > base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next > config: powerpc-ppc64e_defconfig (attached as .config) > compiler: powerpc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0 > reproduce: > wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross > chmod +x ~/bin/make.cross > # save the attached .config to linux build tree > make.cross ARCH=powerpc > > All errors (new ones prefixed by >>): > > In file included from arch/powerpc/include/asm/nohash/pgtable.h:6:0, > from arch/powerpc/include/asm/pgtable.h:19, > from include/linux/memremap.h:8, > from include/linux/mm.h:27, > from include/linux/mman.h:5, > from arch/powerpc/kernel/asm-offsets.c:22: > arch/powerpc/include/asm/nohash/64/pgtable.h: In function '__ptep_test_and_clear_young': >>> arch/powerpc/include/asm/nohash/64/pgtable.h:214:6: error: implicit declaration of function 'pte_young'; did you mean 'pte_pud'? [-Werror=implicit-function-declaration] > if (pte_young(*ptep)) > ^~~~~~~~~ > pte_pud Urk. There's a circular dependency here. I fixed it with the patch below, which seems to be the least worst solution. Possibly we can clean things up further in future. cheers diff --git a/arch/powerpc/include/asm/nohash/32/pgtable.h b/arch/powerpc/include/asm/nohash/32/pgtable.h index 140f8e74b478..987a658b18e1 100644 --- a/arch/powerpc/include/asm/nohash/32/pgtable.h +++ b/arch/powerpc/include/asm/nohash/32/pgtable.h @@ -267,6 +267,11 @@ static inline void __ptep_set_access_flags(struct mm_struct *mm, pte_update(ptep, clr, set); } +static inline int pte_young(pte_t pte) +{ + return pte_val(pte) & _PAGE_ACCESSED; +} + #define __HAVE_ARCH_PTE_SAME #define pte_same(A,B) ((pte_val(A) ^ pte_val(B)) == 0) diff --git a/arch/powerpc/include/asm/nohash/64/pgtable.h b/arch/powerpc/include/asm/nohash/64/pgtable.h index e8de7cb4d3fb..6ac8381f4c7a 100644 --- a/arch/powerpc/include/asm/nohash/64/pgtable.h +++ b/arch/powerpc/include/asm/nohash/64/pgtable.h @@ -204,6 +204,11 @@ static inline unsigned long pte_update(struct mm_struct *mm, return old; } +static inline int pte_young(pte_t pte) +{ + return pte_val(pte) & _PAGE_ACCESSED; +} + static inline int __ptep_test_and_clear_young(struct mm_struct *mm, unsigned long addr, pte_t *ptep) { diff --git a/arch/powerpc/include/asm/nohash/pgtable.h b/arch/powerpc/include/asm/nohash/pgtable.h index 077472640b35..2160be2e4339 100644 --- a/arch/powerpc/include/asm/nohash/pgtable.h +++ b/arch/powerpc/include/asm/nohash/pgtable.h @@ -17,7 +17,6 @@ static inline int pte_write(pte_t pte) } static inline int pte_read(pte_t pte) { return 1; } static inline int pte_dirty(pte_t pte) { return pte_val(pte) & _PAGE_DIRTY; } -static inline int pte_young(pte_t pte) { return pte_val(pte) & _PAGE_ACCESSED; } static inline int pte_special(pte_t pte) { return pte_val(pte) & _PAGE_SPECIAL; } static inline int pte_none(pte_t pte) { return (pte_val(pte) & ~_PTE_NONE_MASK) == 0; } static inline pgprot_t pte_pgprot(pte_t pte) { return __pgprot(pte_val(pte) & PAGE_PROT_BITS); }
Le 04/05/2018 à 13:17, Michael Ellerman a écrit : > kbuild test robot <lkp@intel.com> writes: > >> Hi Christophe, >> >> Thank you for the patch! Yet something to improve: >> >> [auto build test ERROR on powerpc/next] >> [also build test ERROR on v4.17-rc2 next-20180426] >> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] >> >> url: https://github.com/0day-ci/linux/commits/Christophe-Leroy/powerpc-nohash-remove-hash-related-code-from-nohash-headers/20180425-182026 >> base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next >> config: powerpc-ppc64e_defconfig (attached as .config) >> compiler: powerpc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0 >> reproduce: >> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross >> chmod +x ~/bin/make.cross >> # save the attached .config to linux build tree >> make.cross ARCH=powerpc >> >> All errors (new ones prefixed by >>): >> >> In file included from arch/powerpc/include/asm/nohash/pgtable.h:6:0, >> from arch/powerpc/include/asm/pgtable.h:19, >> from include/linux/memremap.h:8, >> from include/linux/mm.h:27, >> from include/linux/mman.h:5, >> from arch/powerpc/kernel/asm-offsets.c:22: >> arch/powerpc/include/asm/nohash/64/pgtable.h: In function '__ptep_test_and_clear_young': >>>> arch/powerpc/include/asm/nohash/64/pgtable.h:214:6: error: implicit declaration of function 'pte_young'; did you mean 'pte_pud'? [-Werror=implicit-function-declaration] >> if (pte_young(*ptep)) >> ^~~~~~~~~ >> pte_pud > > Urk. > > There's a circular dependency here. > > I fixed it with the patch below, which seems to be the least worst > solution. Possibly we can clean things up further in future. Oops, I just sent you a new version of the patch not using pte_young() anymore. I'll let you decide what to do. Christophe > > cheers > > diff --git a/arch/powerpc/include/asm/nohash/32/pgtable.h b/arch/powerpc/include/asm/nohash/32/pgtable.h > index 140f8e74b478..987a658b18e1 100644 > --- a/arch/powerpc/include/asm/nohash/32/pgtable.h > +++ b/arch/powerpc/include/asm/nohash/32/pgtable.h > @@ -267,6 +267,11 @@ static inline void __ptep_set_access_flags(struct mm_struct *mm, > pte_update(ptep, clr, set); > } > > +static inline int pte_young(pte_t pte) > +{ > + return pte_val(pte) & _PAGE_ACCESSED; > +} > + > #define __HAVE_ARCH_PTE_SAME > #define pte_same(A,B) ((pte_val(A) ^ pte_val(B)) == 0) > > diff --git a/arch/powerpc/include/asm/nohash/64/pgtable.h b/arch/powerpc/include/asm/nohash/64/pgtable.h > index e8de7cb4d3fb..6ac8381f4c7a 100644 > --- a/arch/powerpc/include/asm/nohash/64/pgtable.h > +++ b/arch/powerpc/include/asm/nohash/64/pgtable.h > @@ -204,6 +204,11 @@ static inline unsigned long pte_update(struct mm_struct *mm, > return old; > } > > +static inline int pte_young(pte_t pte) > +{ > + return pte_val(pte) & _PAGE_ACCESSED; > +} > + > static inline int __ptep_test_and_clear_young(struct mm_struct *mm, > unsigned long addr, pte_t *ptep) > { > diff --git a/arch/powerpc/include/asm/nohash/pgtable.h b/arch/powerpc/include/asm/nohash/pgtable.h > index 077472640b35..2160be2e4339 100644 > --- a/arch/powerpc/include/asm/nohash/pgtable.h > +++ b/arch/powerpc/include/asm/nohash/pgtable.h > @@ -17,7 +17,6 @@ static inline int pte_write(pte_t pte) > } > static inline int pte_read(pte_t pte) { return 1; } > static inline int pte_dirty(pte_t pte) { return pte_val(pte) & _PAGE_DIRTY; } > -static inline int pte_young(pte_t pte) { return pte_val(pte) & _PAGE_ACCESSED; } > static inline int pte_special(pte_t pte) { return pte_val(pte) & _PAGE_SPECIAL; } > static inline int pte_none(pte_t pte) { return (pte_val(pte) & ~_PTE_NONE_MASK) == 0; } > static inline pgprot_t pte_pgprot(pte_t pte) { return __pgprot(pte_val(pte) & PAGE_PROT_BITS); } >
On Tue, 2018-04-24 at 16:31:28 UTC, Christophe Leroy wrote: > When nohash and book3s header were split, some hash related stuff > remained in the nohash header. This patch removes them. > > Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr> Series applied to powerpc next, thanks. https://git.kernel.org/powerpc/c/45201c8794693d16e8e634188a92e9 cheers
diff --git a/arch/powerpc/include/asm/nohash/32/pgtable.h b/arch/powerpc/include/asm/nohash/32/pgtable.h index a717b5c39b9c..b413abcd5a09 100644 --- a/arch/powerpc/include/asm/nohash/32/pgtable.h +++ b/arch/powerpc/include/asm/nohash/32/pgtable.h @@ -129,7 +129,7 @@ extern int icache_44x_need_flush; #ifndef __ASSEMBLY__ #define pte_clear(mm, addr, ptep) \ - do { pte_update(ptep, ~_PAGE_HASHPTE, 0); } while (0) + do { pte_update(ptep, ~0, 0); } while (0) #define pmd_none(pmd) (!pmd_val(pmd)) #define pmd_bad(pmd) (pmd_val(pmd) & _PMD_BAD) @@ -142,21 +142,6 @@ static inline void pmd_clear(pmd_t *pmdp) /* - * When flushing the tlb entry for a page, we also need to flush the hash - * table entry. flush_hash_pages is assembler (for speed) in hashtable.S. - */ -extern int flush_hash_pages(unsigned context, unsigned long va, - unsigned long pmdval, int count); - -/* Add an HPTE to the hash table */ -extern void add_hash_page(unsigned context, unsigned long va, - unsigned long pmdval); - -/* Flush an entry from the TLB/hash table */ -extern void flush_hash_entry(struct mm_struct *mm, pte_t *ptep, - unsigned long address); - -/* * PTE updates. This function is called whenever an existing * valid PTE is updated. This does -not- include set_pte_at() * which nowadays only sets a new PTE. @@ -242,12 +227,6 @@ static inline int __ptep_test_and_clear_young(unsigned int context, unsigned lon { unsigned long old; old = pte_update(ptep, _PAGE_ACCESSED, 0); -#if _PAGE_HASHPTE != 0 - if (old & _PAGE_HASHPTE) { - unsigned long ptephys = __pa(ptep) & PAGE_MASK; - flush_hash_pages(context, addr, ptephys, 1); - } -#endif return (old & _PAGE_ACCESSED) != 0; } #define ptep_test_and_clear_young(__vma, __addr, __ptep) \ @@ -257,7 +236,7 @@ static inline int __ptep_test_and_clear_young(unsigned int context, unsigned lon static inline pte_t ptep_get_and_clear(struct mm_struct *mm, unsigned long addr, pte_t *ptep) { - return __pte(pte_update(ptep, ~_PAGE_HASHPTE, 0)); + return __pte(pte_update(ptep, ~0, 0)); } #define __HAVE_ARCH_PTEP_SET_WRPROTECT @@ -285,7 +264,7 @@ static inline void __ptep_set_access_flags(struct mm_struct *mm, } #define __HAVE_ARCH_PTE_SAME -#define pte_same(A,B) (((pte_val(A) ^ pte_val(B)) & ~_PAGE_HASHPTE) == 0) +#define pte_same(A,B) ((pte_val(A) ^ pte_val(B)) == 0) /* * Note that on Book E processors, the pmd contains the kernel virtual @@ -326,7 +305,7 @@ static inline void __ptep_set_access_flags(struct mm_struct *mm, /* * Encode and decode a swap entry. * Note that the bits we use in a PTE for representing a swap entry - * must not include the _PAGE_PRESENT bit or the _PAGE_HASHPTE bit (if used). + * must not include the _PAGE_PRESENT bit. * -- paulus */ #define __swp_type(entry) ((entry).val & 0x1f) diff --git a/arch/powerpc/include/asm/nohash/64/pgtable.h b/arch/powerpc/include/asm/nohash/64/pgtable.h index 5c5f75d005ad..251d74c9013e 100644 --- a/arch/powerpc/include/asm/nohash/64/pgtable.h +++ b/arch/powerpc/include/asm/nohash/64/pgtable.h @@ -173,8 +173,6 @@ static inline void pgd_set(pgd_t *pgdp, unsigned long val) /* to find an entry in a kernel page-table-directory */ /* This now only contains the vmalloc pages */ #define pgd_offset_k(address) pgd_offset(&init_mm, address) -extern void hpte_need_flush(struct mm_struct *mm, unsigned long addr, - pte_t *ptep, unsigned long pte, int huge); /* Atomic PTE updates */ static inline unsigned long pte_update(struct mm_struct *mm, @@ -205,11 +203,6 @@ static inline unsigned long pte_update(struct mm_struct *mm, if (!huge) assert_pte_locked(mm, addr); -#ifdef CONFIG_PPC_BOOK3S_64 - if (old & _PAGE_HASHPTE) - hpte_need_flush(mm, addr, ptep, old, huge); -#endif - return old; } @@ -218,7 +211,7 @@ static inline int __ptep_test_and_clear_young(struct mm_struct *mm, { unsigned long old; - if ((pte_val(*ptep) & (_PAGE_ACCESSED | _PAGE_HASHPTE)) == 0) + if (pte_young(*ptep)) return 0; old = pte_update(mm, addr, ptep, _PAGE_ACCESSED, 0, 0); return (old & _PAGE_ACCESSED) != 0; @@ -312,7 +305,7 @@ static inline void __ptep_set_access_flags(struct mm_struct *mm, } #define __HAVE_ARCH_PTE_SAME -#define pte_same(A,B) (((pte_val(A) ^ pte_val(B)) & ~_PAGE_HPTEFLAGS) == 0) +#define pte_same(A,B) ((pte_val(A) ^ pte_val(B)) == 0) #define pte_ERROR(e) \ pr_err("%s:%d: bad pte %08lx.\n", __FILE__, __LINE__, pte_val(e)) @@ -324,11 +317,6 @@ static inline void __ptep_set_access_flags(struct mm_struct *mm, /* Encode and de-code a swap entry */ #define MAX_SWAPFILES_CHECK() do { \ BUILD_BUG_ON(MAX_SWAPFILES_SHIFT > SWP_TYPE_BITS); \ - /* \ - * Don't have overlapping bits with _PAGE_HPTEFLAGS \ - * We filter HPTEFLAGS on set_pte. \ - */ \ - BUILD_BUG_ON(_PAGE_HPTEFLAGS & (0x1f << _PAGE_BIT_SWAP_TYPE)); \ } while (0) /* * on pte we don't need handle RADIX_TREE_EXCEPTIONAL_SHIFT; diff --git a/arch/powerpc/include/asm/nohash/pgtable.h b/arch/powerpc/include/asm/nohash/pgtable.h index c56de1e8026f..f2fe3cbe90af 100644 --- a/arch/powerpc/include/asm/nohash/pgtable.h +++ b/arch/powerpc/include/asm/nohash/pgtable.h @@ -148,37 +148,16 @@ extern void set_pte_at(struct mm_struct *mm, unsigned long addr, pte_t *ptep, static inline void __set_pte_at(struct mm_struct *mm, unsigned long addr, pte_t *ptep, pte_t pte, int percpu) { -#if defined(CONFIG_PPC_STD_MMU_32) && defined(CONFIG_SMP) && !defined(CONFIG_PTE_64BIT) - /* First case is 32-bit Hash MMU in SMP mode with 32-bit PTEs. We use the - * helper pte_update() which does an atomic update. We need to do that - * because a concurrent invalidation can clear _PAGE_HASHPTE. If it's a - * per-CPU PTE such as a kmap_atomic, we do a simple update preserving - * the hash bits instead (ie, same as the non-SMP case) - */ - if (percpu) - *ptep = __pte((pte_val(*ptep) & _PAGE_HASHPTE) - | (pte_val(pte) & ~_PAGE_HASHPTE)); - else - pte_update(ptep, ~_PAGE_HASHPTE, pte_val(pte)); - -#elif defined(CONFIG_PPC32) && defined(CONFIG_PTE_64BIT) +#if defined(CONFIG_PPC32) && defined(CONFIG_PTE_64BIT) /* Second case is 32-bit with 64-bit PTE. In this case, we * can just store as long as we do the two halves in the right order - * with a barrier in between. This is possible because we take care, - * in the hash code, to pre-invalidate if the PTE was already hashed, - * which synchronizes us with any concurrent invalidation. - * In the percpu case, we also fallback to the simple update preserving - * the hash bits + * with a barrier in between. + * In the percpu case, we also fallback to the simple update */ if (percpu) { - *ptep = __pte((pte_val(*ptep) & _PAGE_HASHPTE) - | (pte_val(pte) & ~_PAGE_HASHPTE)); + *ptep = pte; return; } -#if _PAGE_HASHPTE != 0 - if (pte_val(*ptep) & _PAGE_HASHPTE) - flush_hash_entry(mm, ptep, addr); -#endif __asm__ __volatile__("\ stw%U0%X0 %2,%0\n\ eieio\n\ @@ -186,15 +165,6 @@ static inline void __set_pte_at(struct mm_struct *mm, unsigned long addr, : "=m" (*ptep), "=m" (*((unsigned char *)ptep+4)) : "r" (pte) : "memory"); -#elif defined(CONFIG_PPC_STD_MMU_32) - /* Third case is 32-bit hash table in UP mode, we need to preserve - * the _PAGE_HASHPTE bit since we may not have invalidated the previous - * translation in the hash yet (done in a subsequent flush_tlb_xxx()) - * and see we need to keep track that this PTE needs invalidating - */ - *ptep = __pte((pte_val(*ptep) & _PAGE_HASHPTE) - | (pte_val(pte) & ~_PAGE_HASHPTE)); - #else /* Anything else just stores the PTE normally. That covers all 64-bit * cases, and 32-bit non-hash with 32-bit PTEs. diff --git a/arch/powerpc/include/asm/nohash/pte-book3e.h b/arch/powerpc/include/asm/nohash/pte-book3e.h index ccee8eb509bb..9ff51b4c0cac 100644 --- a/arch/powerpc/include/asm/nohash/pte-book3e.h +++ b/arch/powerpc/include/asm/nohash/pte-book3e.h @@ -57,7 +57,6 @@ #define _PAGE_USER (_PAGE_BAP_UR | _PAGE_BAP_SR) /* Can be read */ #define _PAGE_PRIVILEGED (_PAGE_BAP_SR) -#define _PAGE_HASHPTE 0 #define _PAGE_BUSY 0 #define _PAGE_SPECIAL _PAGE_SW0
When nohash and book3s header were split, some hash related stuff remained in the nohash header. This patch removes them. Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr> --- arch/powerpc/include/asm/nohash/32/pgtable.h | 29 +++------------------ arch/powerpc/include/asm/nohash/64/pgtable.h | 16 ++---------- arch/powerpc/include/asm/nohash/pgtable.h | 38 +++------------------------- arch/powerpc/include/asm/nohash/pte-book3e.h | 1 - 4 files changed, 10 insertions(+), 74 deletions(-)