[PATCH v2 1/4] powerpc/mm: enable the use of page table cache of order 0

[PATCH v2 1/4] powerpc/mm: enable the use of page table cache of order 0

[Christophe Leroy]

hugepages uses a cache of order 0. Lets allow page tables
of order 0 in the common part in order to avoid open coding
in hugetlb

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 arch/powerpc/include/asm/book3s/32/pgalloc.h | 5 +----
 arch/powerpc/include/asm/book3s/64/pgalloc.h | 5 +----
 arch/powerpc/include/asm/nohash/32/pgalloc.h | 5 +----
 arch/powerpc/include/asm/nohash/64/pgalloc.h | 5 +----
 arch/powerpc/mm/init-common.c                | 6 +++---
 5 files changed, 7 insertions(+), 19 deletions(-)

diff --git a/arch/powerpc/include/asm/book3s/32/pgalloc.h b/arch/powerpc/include/asm/book3s/32/pgalloc.h
index 82e44b1a00ae..96138ab3ddd6 100644
--- a/arch/powerpc/include/asm/book3s/32/pgalloc.h
+++ b/arch/powerpc/include/asm/book3s/32/pgalloc.h
@@ -25,10 +25,7 @@
 extern void __bad_pte(pmd_t *pmd);
 
 extern struct kmem_cache *pgtable_cache[];
-#define PGT_CACHE(shift) ({				\
-			BUG_ON(!(shift));		\
-			pgtable_cache[(shift) - 1];	\
-		})
+#define PGT_CACHE(shift) pgtable_cache[shift]
 
 static inline pgd_t *pgd_alloc(struct mm_struct *mm)
 {
diff --git a/arch/powerpc/include/asm/book3s/64/pgalloc.h b/arch/powerpc/include/asm/book3s/64/pgalloc.h
index 391ed2c3b697..bfed4cf3b2f3 100644
--- a/arch/powerpc/include/asm/book3s/64/pgalloc.h
+++ b/arch/powerpc/include/asm/book3s/64/pgalloc.h
@@ -37,10 +37,7 @@ extern struct vmemmap_backing *vmemmap_list;
 #define MAX_PGTABLE_INDEX_SIZE	0xf
 
 extern struct kmem_cache *pgtable_cache[];
-#define PGT_CACHE(shift) ({				\
-			BUG_ON(!(shift));		\
-			pgtable_cache[(shift) - 1];	\
-		})
+#define PGT_CACHE(shift) pgtable_cache[shift]
 
 extern pte_t *pte_fragment_alloc(struct mm_struct *, unsigned long, int);
 extern pmd_t *pmd_fragment_alloc(struct mm_struct *, unsigned long);
diff --git a/arch/powerpc/include/asm/nohash/32/pgalloc.h b/arch/powerpc/include/asm/nohash/32/pgalloc.h
index 8825953c225b..6fbbb90043c0 100644
--- a/arch/powerpc/include/asm/nohash/32/pgalloc.h
+++ b/arch/powerpc/include/asm/nohash/32/pgalloc.h
@@ -25,10 +25,7 @@
 extern void __bad_pte(pmd_t *pmd);
 
 extern struct kmem_cache *pgtable_cache[];
-#define PGT_CACHE(shift) ({				\
-			BUG_ON(!(shift));		\
-			pgtable_cache[(shift) - 1];	\
-		})
+#define PGT_CACHE(shift) pgtable_cache[shift]
 
 static inline pgd_t *pgd_alloc(struct mm_struct *mm)
 {
diff --git a/arch/powerpc/include/asm/nohash/64/pgalloc.h b/arch/powerpc/include/asm/nohash/64/pgalloc.h
index e2d62d033708..e95eb499a174 100644
--- a/arch/powerpc/include/asm/nohash/64/pgalloc.h
+++ b/arch/powerpc/include/asm/nohash/64/pgalloc.h
@@ -36,10 +36,7 @@ extern struct vmemmap_backing *vmemmap_list;
 #define MAX_PGTABLE_INDEX_SIZE	0xf
 
 extern struct kmem_cache *pgtable_cache[];
-#define PGT_CACHE(shift) ({				\
-			BUG_ON(!(shift));		\
-			pgtable_cache[(shift) - 1];	\
-		})
+#define PGT_CACHE(shift) pgtable_cache[shift]
 
 static inline pgd_t *pgd_alloc(struct mm_struct *mm)
 {
diff --git a/arch/powerpc/mm/init-common.c b/arch/powerpc/mm/init-common.c
index 2b656e67f2ea..41190f2b60c2 100644
--- a/arch/powerpc/mm/init-common.c
+++ b/arch/powerpc/mm/init-common.c
@@ -40,7 +40,7 @@ static void pmd_ctor(void *addr)
 	memset(addr, 0, PMD_TABLE_SIZE);
 }
 
-struct kmem_cache *pgtable_cache[MAX_PGTABLE_INDEX_SIZE];
+struct kmem_cache *pgtable_cache[MAX_PGTABLE_INDEX_SIZE + 1];
 EXPORT_SYMBOL_GPL(pgtable_cache);	/* used by kvm_hv module */
 
 /*
@@ -71,7 +71,7 @@ void pgtable_cache_add(unsigned shift, void (*ctor)(void *))
 	 * moment, gcc doesn't seem to recognize is_power_of_2 as a
 	 * constant expression, so so much for that. */
 	BUG_ON(!is_power_of_2(minalign));
-	BUG_ON((shift < 1) || (shift > MAX_PGTABLE_INDEX_SIZE));
+	BUG_ON(shift > MAX_PGTABLE_INDEX_SIZE);
 
 	if (PGT_CACHE(shift))
 		return; /* Already have a cache of this size */
@@ -83,7 +83,7 @@ void pgtable_cache_add(unsigned shift, void (*ctor)(void *))
 		panic("Could not allocate pgtable cache for order %d", shift);
 
 	kfree(name);
-	pgtable_cache[shift - 1] = new;
+	pgtable_cache[shift] = new;
 
 	pr_debug("Allocated pgtable cache for order %d\n", shift);
 }
-- 
2.13.3

[PATCH v2 2/4] powerpc/mm: replace hugetlb_cache by PGT_CACHE(PTE_T_ORDER)

[Christophe Leroy]

Instead of opencoding cache handling for the special case
of hugepage tables having a single pte_t element, this
patch makes use of the common pgtable_cache helpers

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 arch/powerpc/include/asm/hugetlb.h |  2 --
 arch/powerpc/mm/hugetlbpage.c      | 26 +++++++-------------------
 2 files changed, 7 insertions(+), 21 deletions(-)

diff --git a/arch/powerpc/include/asm/hugetlb.h b/arch/powerpc/include/asm/hugetlb.h
index 2d00cc530083..e13843556414 100644
--- a/arch/powerpc/include/asm/hugetlb.h
+++ b/arch/powerpc/include/asm/hugetlb.h
@@ -6,8 +6,6 @@
 #include <asm/page.h>
 #include <asm-generic/hugetlb.h>
 
-extern struct kmem_cache *hugepte_cache;
-
 #ifdef CONFIG_PPC_BOOK3S_64
 
 #include <asm/book3s/64/hugetlb.h>
diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpage.c
index 7296a42eb62e..53b7a605c3a8 100644
--- a/arch/powerpc/mm/hugetlbpage.c
+++ b/arch/powerpc/mm/hugetlbpage.c
@@ -43,6 +43,8 @@ EXPORT_SYMBOL(HPAGE_SHIFT);
 
 #define hugepd_none(hpd)	(hpd_val(hpd) == 0)
 
+#define PTE_T_ORDER	(__builtin_ffs(sizeof(pte_t)) - __builtin_ffs(sizeof(void *)))
+
 pte_t *huge_pte_offset(struct mm_struct *mm, unsigned long addr, unsigned long sz)
 {
 	/*
@@ -62,7 +64,7 @@ static int __hugepte_alloc(struct mm_struct *mm, hugepd_t *hpdp,
 	int num_hugepd;
 
 	if (pshift >= pdshift) {
-		cachep = hugepte_cache;
+		cachep = PGT_CACHE(PTE_T_ORDER);
 		num_hugepd = 1 << (pshift - pdshift);
 	} else {
 		cachep = PGT_CACHE(pdshift - pshift);
@@ -265,7 +267,7 @@ static void hugepd_free_rcu_callback(struct rcu_head *head)
 	unsigned int i;
 
 	for (i = 0; i < batch->index; i++)
-		kmem_cache_free(hugepte_cache, batch->ptes[i]);
+		kmem_cache_free(PGT_CACHE(PTE_T_ORDER), batch->ptes[i]);
 
 	free_page((unsigned long)batch);
 }
@@ -278,7 +280,7 @@ static void hugepd_free(struct mmu_gather *tlb, void *hugepte)
 
 	if (atomic_read(&tlb->mm->mm_users) < 2 ||
 	    mm_is_thread_local(tlb->mm)) {
-		kmem_cache_free(hugepte_cache, hugepte);
+		kmem_cache_free(PGT_CACHE(PTE_T_ORDER), hugepte);
 		put_cpu_var(hugepd_freelist_cur);
 		return;
 	}
@@ -653,7 +655,6 @@ static int __init hugepage_setup_sz(char *str)
 }
 __setup("hugepagesz=", hugepage_setup_sz);
 
-struct kmem_cache *hugepte_cache;
 static int __init hugetlbpage_init(void)
 {
 	int psize;
@@ -703,21 +704,8 @@ static int __init hugetlbpage_init(void)
 		if (pdshift > shift)
 			pgtable_cache_add(pdshift - shift, NULL);
 #if defined(CONFIG_PPC_FSL_BOOK3E) || defined(CONFIG_PPC_8xx)
-		else if (!hugepte_cache) {
-			/*
-			 * Create a kmem cache for hugeptes.  The bottom bits in
-			 * the pte have size information encoded in them, so
-			 * align them to allow this
-			 */
-			hugepte_cache = kmem_cache_create("hugepte-cache",
-							  sizeof(pte_t),
-							  HUGEPD_SHIFT_MASK + 1,
-							  0, NULL);
-			if (hugepte_cache == NULL)
-				panic("%s: Unable to create kmem cache "
-				      "for hugeptes\n", __func__);
-
-		}
+		else
+			pgtable_cache_add(PTE_T_ORDER, NULL);
 #endif
 	}
 
-- 
2.13.3

[PATCH v2 3/4] powerpc/mm: fix a warning when a cache is common to PGD and hugepages

[Christophe Leroy]

While implementing TLB miss HW assistance on the 8xx, the following
warning was encountered:

[  423.732965] WARNING: CPU: 0 PID: 345 at mm/slub.c:2412 ___slab_alloc.constprop.30+0x26c/0x46c
[  423.733033] CPU: 0 PID: 345 Comm: mmap Not tainted 4.18.0-rc8-00664-g2dfff9121c55 #671
[  423.733075] NIP:  c0108f90 LR: c0109ad0 CTR: 00000004
[  423.733121] REGS: c455bba0 TRAP: 0700   Not tainted  (4.18.0-rc8-00664-g2dfff9121c55)
[  423.733147] MSR:  00021032 <ME,IR,DR,RI>  CR: 24224848  XER: 20000000
[  423.733319]
[  423.733319] GPR00: c0109ad0 c455bc50 c4521910 c60053c0 007080c0 c0011b34 c7fa41e0 c455be30
[  423.733319] GPR08: 00000001 c00103a0 c7fa41e0 c49afcc4 24282842 10018840 c079b37c 00000040
[  423.733319] GPR16: 73f00000 00210d00 00000000 00000001 c455a000 00000100 00000200 c455a000
[  423.733319] GPR24: c60053c0 c0011b34 007080c0 c455a000 c455a000 c7fa41e0 00000000 00009032
[  423.734190] NIP [c0108f90] ___slab_alloc.constprop.30+0x26c/0x46c
[  423.734257] LR [c0109ad0] kmem_cache_alloc+0x210/0x23c
[  423.734283] Call Trace:
[  423.734326] [c455bc50] [00000100] 0x100 (unreliable)
[  423.734430] [c455bcc0] [c0109ad0] kmem_cache_alloc+0x210/0x23c
[  423.734543] [c455bcf0] [c0011b34] huge_pte_alloc+0xc0/0x1dc
[  423.734633] [c455bd20] [c01044dc] hugetlb_fault+0x408/0x48c
[  423.734720] [c455bdb0] [c0104b20] follow_hugetlb_page+0x14c/0x44c
[  423.734826] [c455be10] [c00e8e54] __get_user_pages+0x1c4/0x3dc
[  423.734919] [c455be80] [c00e9924] __mm_populate+0xac/0x140
[  423.735020] [c455bec0] [c00db14c] vm_mmap_pgoff+0xb4/0xb8
[  423.735127] [c455bf00] [c00f27c0] ksys_mmap_pgoff+0xcc/0x1fc
[  423.735222] [c455bf40] [c000e0f8] ret_from_syscall+0x0/0x38
[  423.735271] Instruction dump:
[  423.735321] 7cbf482e 38fd0008 7fa6eb78 7fc4f378 4bfff5dd 7fe3fb78 4bfffe24 81370010
[  423.735536] 71280004 41a2ff88 4840c571 4bffff80 <0fe00000> 4bfffeb8 81340010 712a0004
[  423.735757] ---[ end trace e9b222919a470790 ]---

This warning occurs when calling kmem_cache_zalloc() on a
cache having a constructor.

In this case it happens because PGD cache and 512k hugepte cache are
the same size (4k). While a cache with constructor is created for
the PGD, hugepages create cache without constructor and uses
kmem_cache_zalloc(). As both expect a cache with the same size,
the hugepages reuse the cache created for PGD, hence the conflict.

In order to avoid this conflict, this patch:
- modifies pgtable_cache_add() so that a zeroising constructor is
added for any cache size.
- replaces calls to kmem_cache_zalloc() by kmem_cache_alloc()

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 arch/powerpc/include/asm/pgtable.h |  2 +-
 arch/powerpc/mm/hugetlbpage.c      |  6 ++---
 arch/powerpc/mm/init-common.c      | 46 ++++++++++++++++++++++++++------------
 3 files changed, 36 insertions(+), 18 deletions(-)

diff --git a/arch/powerpc/include/asm/pgtable.h b/arch/powerpc/include/asm/pgtable.h
index 14c79a7dc855..1e6265dc6697 100644
--- a/arch/powerpc/include/asm/pgtable.h
+++ b/arch/powerpc/include/asm/pgtable.h
@@ -72,7 +72,7 @@ extern int gup_hugepte(pte_t *ptep, unsigned long sz, unsigned long addr,
 /* can we use this in kvm */
 unsigned long vmalloc_to_phys(void *vmalloc_addr);
 
-void pgtable_cache_add(unsigned shift, void (*ctor)(void *));
+void pgtable_cache_add(unsigned int shift);
 void pgtable_cache_init(void);
 
 #if defined(CONFIG_STRICT_KERNEL_RWX) || defined(CONFIG_PPC32)
diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpage.c
index 53b7a605c3a8..6cd90445b1f5 100644
--- a/arch/powerpc/mm/hugetlbpage.c
+++ b/arch/powerpc/mm/hugetlbpage.c
@@ -71,7 +71,7 @@ static int __hugepte_alloc(struct mm_struct *mm, hugepd_t *hpdp,
 		num_hugepd = 1;
 	}
 
-	new = kmem_cache_zalloc(cachep, pgtable_gfp_flags(mm, GFP_KERNEL));
+	new = kmem_cache_alloc(cachep, pgtable_gfp_flags(mm, GFP_KERNEL));
 
 	BUG_ON(pshift > HUGEPD_SHIFT_MASK);
 	BUG_ON((unsigned long)new & HUGEPD_SHIFT_MASK);
@@ -702,10 +702,10 @@ static int __init hugetlbpage_init(void)
 		 * use pgt cache for hugepd.
 		 */
 		if (pdshift > shift)
-			pgtable_cache_add(pdshift - shift, NULL);
+			pgtable_cache_add(pdshift - shift);
 #if defined(CONFIG_PPC_FSL_BOOK3E) || defined(CONFIG_PPC_8xx)
 		else
-			pgtable_cache_add(PTE_T_ORDER, NULL);
+			pgtable_cache_add(PTE_T_ORDER);
 #endif
 	}
 
diff --git a/arch/powerpc/mm/init-common.c b/arch/powerpc/mm/init-common.c
index 41190f2b60c2..b7ca03643d0b 100644
--- a/arch/powerpc/mm/init-common.c
+++ b/arch/powerpc/mm/init-common.c
@@ -25,19 +25,37 @@
 #include <asm/pgalloc.h>
 #include <asm/pgtable.h>
 
-static void pgd_ctor(void *addr)
-{
-	memset(addr, 0, PGD_TABLE_SIZE);
+#define CTOR(shift) static void ctor_##shift(void *addr) \
+{							\
+	memset(addr, 0, sizeof(void *) << (shift));	\
 }
 
-static void pud_ctor(void *addr)
-{
-	memset(addr, 0, PUD_TABLE_SIZE);
-}
+CTOR(0); CTOR(1); CTOR(2); CTOR(3); CTOR(4); CTOR(5); CTOR(6); CTOR(7);
+CTOR(8); CTOR(9); CTOR(10); CTOR(11); CTOR(12); CTOR(13); CTOR(14); CTOR(15);
 
-static void pmd_ctor(void *addr)
+static inline void (*ctor(int shift))(void *)
 {
-	memset(addr, 0, PMD_TABLE_SIZE);
+	BUILD_BUG_ON(MAX_PGTABLE_INDEX_SIZE != 15);
+
+	switch (shift) {
+	case 0: return ctor_0;
+	case 1: return ctor_1;
+	case 2: return ctor_2;
+	case 3: return ctor_3;
+	case 4: return ctor_4;
+	case 5: return ctor_5;
+	case 6: return ctor_6;
+	case 7: return ctor_7;
+	case 8: return ctor_8;
+	case 9: return ctor_9;
+	case 10: return ctor_10;
+	case 11: return ctor_11;
+	case 12: return ctor_12;
+	case 13: return ctor_13;
+	case 14: return ctor_14;
+	case 15: return ctor_15;
+	}
+	return NULL;
 }
 
 struct kmem_cache *pgtable_cache[MAX_PGTABLE_INDEX_SIZE + 1];
@@ -50,7 +68,7 @@ EXPORT_SYMBOL_GPL(pgtable_cache);	/* used by kvm_hv module */
  * everything else.  Caches created by this function are used for all
  * the higher level pagetables, and for hugepage pagetables.
  */
-void pgtable_cache_add(unsigned shift, void (*ctor)(void *))
+void pgtable_cache_add(unsigned int shift)
 {
 	char *name;
 	unsigned long table_size = sizeof(void *) << shift;
@@ -78,7 +96,7 @@ void pgtable_cache_add(unsigned shift, void (*ctor)(void *))
 
 	align = max_t(unsigned long, align, minalign);
 	name = kasprintf(GFP_KERNEL, "pgtable-2^%d", shift);
-	new = kmem_cache_create(name, table_size, align, 0, ctor);
+	new = kmem_cache_create(name, table_size, align, 0, ctor(shift));
 	if (!new)
 		panic("Could not allocate pgtable cache for order %d", shift);
 
@@ -91,15 +109,15 @@ EXPORT_SYMBOL_GPL(pgtable_cache_add);	/* used by kvm_hv module */
 
 void pgtable_cache_init(void)
 {
-	pgtable_cache_add(PGD_INDEX_SIZE, pgd_ctor);
+	pgtable_cache_add(PGD_INDEX_SIZE);
 
 	if (PMD_CACHE_INDEX && !PGT_CACHE(PMD_CACHE_INDEX))
-		pgtable_cache_add(PMD_CACHE_INDEX, pmd_ctor);
+		pgtable_cache_add(PMD_CACHE_INDEX);
 	/*
 	 * In all current configs, when the PUD index exists it's the
 	 * same size as either the pgd or pmd index except with THP enabled
 	 * on book3s 64
 	 */
 	if (PUD_CACHE_INDEX && !PGT_CACHE(PUD_CACHE_INDEX))
-		pgtable_cache_add(PUD_CACHE_INDEX, pud_ctor);
+		pgtable_cache_add(PUD_CACHE_INDEX);
 }
-- 
2.13.3

Re: [PATCH v2 3/4] powerpc/mm: fix a warning when a cache is common to PGD and hugepages

[Aneesh Kumar K.V]

On 08/14/2018 08:24 PM, Christophe Leroy wrote:
> While implementing TLB miss HW assistance on the 8xx, the following
> warning was encountered:
> 
> [  423.732965] WARNING: CPU: 0 PID: 345 at mm/slub.c:2412 ___slab_alloc.constprop.30+0x26c/0x46c
> [  423.733033] CPU: 0 PID: 345 Comm: mmap Not tainted 4.18.0-rc8-00664-g2dfff9121c55 #671
> [  423.733075] NIP:  c0108f90 LR: c0109ad0 CTR: 00000004
> [  423.733121] REGS: c455bba0 TRAP: 0700   Not tainted  (4.18.0-rc8-00664-g2dfff9121c55)
> [  423.733147] MSR:  00021032 <ME,IR,DR,RI>  CR: 24224848  XER: 20000000
> [  423.733319]
> [  423.733319] GPR00: c0109ad0 c455bc50 c4521910 c60053c0 007080c0 c0011b34 c7fa41e0 c455be30
> [  423.733319] GPR08: 00000001 c00103a0 c7fa41e0 c49afcc4 24282842 10018840 c079b37c 00000040
> [  423.733319] GPR16: 73f00000 00210d00 00000000 00000001 c455a000 00000100 00000200 c455a000
> [  423.733319] GPR24: c60053c0 c0011b34 007080c0 c455a000 c455a000 c7fa41e0 00000000 00009032
> [  423.734190] NIP [c0108f90] ___slab_alloc.constprop.30+0x26c/0x46c
> [  423.734257] LR [c0109ad0] kmem_cache_alloc+0x210/0x23c
> [  423.734283] Call Trace:
> [  423.734326] [c455bc50] [00000100] 0x100 (unreliable)
> [  423.734430] [c455bcc0] [c0109ad0] kmem_cache_alloc+0x210/0x23c
> [  423.734543] [c455bcf0] [c0011b34] huge_pte_alloc+0xc0/0x1dc
> [  423.734633] [c455bd20] [c01044dc] hugetlb_fault+0x408/0x48c
> [  423.734720] [c455bdb0] [c0104b20] follow_hugetlb_page+0x14c/0x44c
> [  423.734826] [c455be10] [c00e8e54] __get_user_pages+0x1c4/0x3dc
> [  423.734919] [c455be80] [c00e9924] __mm_populate+0xac/0x140
> [  423.735020] [c455bec0] [c00db14c] vm_mmap_pgoff+0xb4/0xb8
> [  423.735127] [c455bf00] [c00f27c0] ksys_mmap_pgoff+0xcc/0x1fc
> [  423.735222] [c455bf40] [c000e0f8] ret_from_syscall+0x0/0x38
> [  423.735271] Instruction dump:
> [  423.735321] 7cbf482e 38fd0008 7fa6eb78 7fc4f378 4bfff5dd 7fe3fb78 4bfffe24 81370010
> [  423.735536] 71280004 41a2ff88 4840c571 4bffff80 <0fe00000> 4bfffeb8 81340010 712a0004
> [  423.735757] ---[ end trace e9b222919a470790 ]---
> 
> This warning occurs when calling kmem_cache_zalloc() on a
> cache having a constructor.
> 
> In this case it happens because PGD cache and 512k hugepte cache are
> the same size (4k). While a cache with constructor is created for
> the PGD, hugepages create cache without constructor and uses
> kmem_cache_zalloc(). As both expect a cache with the same size,
> the hugepages reuse the cache created for PGD, hence the conflict.
> 
> In order to avoid this conflict, this patch:
> - modifies pgtable_cache_add() so that a zeroising constructor is
> added for any cache size.
> - replaces calls to kmem_cache_zalloc() by kmem_cache_alloc()
> 

Can't we just do kmem_cache_alloc with gfp flags __GFP_ZERO? and remove 
the constructor completely?


-aneesh

Re: [PATCH v2 3/4] powerpc/mm: fix a warning when a cache is common to PGD and hugepages

[Christophe LEROY]

Le 17/08/2018 à 05:32, Aneesh Kumar K.V a écrit :
> On 08/14/2018 08:24 PM, Christophe Leroy wrote:
>> While implementing TLB miss HW assistance on the 8xx, the following
>> warning was encountered:
>>
>> [  423.732965] WARNING: CPU: 0 PID: 345 at mm/slub.c:2412 
>> ___slab_alloc.constprop.30+0x26c/0x46c
>> [  423.733033] CPU: 0 PID: 345 Comm: mmap Not tainted 
>> 4.18.0-rc8-00664-g2dfff9121c55 #671
>> [  423.733075] NIP:  c0108f90 LR: c0109ad0 CTR: 00000004
>> [  423.733121] REGS: c455bba0 TRAP: 0700   Not tainted  
>> (4.18.0-rc8-00664-g2dfff9121c55)
>> [  423.733147] MSR:  00021032 <ME,IR,DR,RI>  CR: 24224848  XER: 20000000
>> [  423.733319]
>> [  423.733319] GPR00: c0109ad0 c455bc50 c4521910 c60053c0 007080c0 
>> c0011b34 c7fa41e0 c455be30
>> [  423.733319] GPR08: 00000001 c00103a0 c7fa41e0 c49afcc4 24282842 
>> 10018840 c079b37c 00000040
>> [  423.733319] GPR16: 73f00000 00210d00 00000000 00000001 c455a000 
>> 00000100 00000200 c455a000
>> [  423.733319] GPR24: c60053c0 c0011b34 007080c0 c455a000 c455a000 
>> c7fa41e0 00000000 00009032
>> [  423.734190] NIP [c0108f90] ___slab_alloc.constprop.30+0x26c/0x46c
>> [  423.734257] LR [c0109ad0] kmem_cache_alloc+0x210/0x23c
>> [  423.734283] Call Trace:
>> [  423.734326] [c455bc50] [00000100] 0x100 (unreliable)
>> [  423.734430] [c455bcc0] [c0109ad0] kmem_cache_alloc+0x210/0x23c
>> [  423.734543] [c455bcf0] [c0011b34] huge_pte_alloc+0xc0/0x1dc
>> [  423.734633] [c455bd20] [c01044dc] hugetlb_fault+0x408/0x48c
>> [  423.734720] [c455bdb0] [c0104b20] follow_hugetlb_page+0x14c/0x44c
>> [  423.734826] [c455be10] [c00e8e54] __get_user_pages+0x1c4/0x3dc
>> [  423.734919] [c455be80] [c00e9924] __mm_populate+0xac/0x140
>> [  423.735020] [c455bec0] [c00db14c] vm_mmap_pgoff+0xb4/0xb8
>> [  423.735127] [c455bf00] [c00f27c0] ksys_mmap_pgoff+0xcc/0x1fc
>> [  423.735222] [c455bf40] [c000e0f8] ret_from_syscall+0x0/0x38
>> [  423.735271] Instruction dump:
>> [  423.735321] 7cbf482e 38fd0008 7fa6eb78 7fc4f378 4bfff5dd 7fe3fb78 
>> 4bfffe24 81370010
>> [  423.735536] 71280004 41a2ff88 4840c571 4bffff80 <0fe00000> 4bfffeb8 
>> 81340010 712a0004
>> [  423.735757] ---[ end trace e9b222919a470790 ]---
>>
>> This warning occurs when calling kmem_cache_zalloc() on a
>> cache having a constructor.
>>
>> In this case it happens because PGD cache and 512k hugepte cache are
>> the same size (4k). While a cache with constructor is created for
>> the PGD, hugepages create cache without constructor and uses
>> kmem_cache_zalloc(). As both expect a cache with the same size,
>> the hugepages reuse the cache created for PGD, hence the conflict.
>>
>> In order to avoid this conflict, this patch:
>> - modifies pgtable_cache_add() so that a zeroising constructor is
>> added for any cache size.
>> - replaces calls to kmem_cache_zalloc() by kmem_cache_alloc()
>>
> 
> Can't we just do kmem_cache_alloc with gfp flags __GFP_ZERO? and remove 
> the constructor completely?

I don't understand what you mean. That's exactly what I did in v1 (by 
using kmem_cache_zalloc()), and you commented that doing this we would 
zeroise at allocation whereas the constructors are called when adding 
memory to the slab and when freeing the allocated block. Or did I 
misunderstood your comment ?

static inline void *kmem_cache_zalloc(struct kmem_cache *k, gfp_t flags)
{
	return kmem_cache_alloc(k, flags | __GFP_ZERO);
}

Christophe


> 
> 
> -aneesh

Re: [PATCH v2 3/4] powerpc/mm: fix a warning when a cache is common to PGD and hugepages

[Christophe LEROY]

Aneesh,

Le 17/08/2018 à 12:44, Christophe LEROY a écrit :
> 
> 
> Le 17/08/2018 à 05:32, Aneesh Kumar K.V a écrit :
>> On 08/14/2018 08:24 PM, Christophe Leroy wrote:
>>> While implementing TLB miss HW assistance on the 8xx, the following
>>> warning was encountered:
>>>
>>> [  423.732965] WARNING: CPU: 0 PID: 345 at mm/slub.c:2412 
>>> ___slab_alloc.constprop.30+0x26c/0x46c
>>> [  423.733033] CPU: 0 PID: 345 Comm: mmap Not tainted 
>>> 4.18.0-rc8-00664-g2dfff9121c55 #671
>>> [  423.733075] NIP:  c0108f90 LR: c0109ad0 CTR: 00000004
>>> [  423.733121] REGS: c455bba0 TRAP: 0700   Not tainted 
>>> (4.18.0-rc8-00664-g2dfff9121c55)
>>> [  423.733147] MSR:  00021032 <ME,IR,DR,RI>  CR: 24224848  XER: 20000000
>>> [  423.733319]
>>> [  423.733319] GPR00: c0109ad0 c455bc50 c4521910 c60053c0 007080c0 
>>> c0011b34 c7fa41e0 c455be30
>>> [  423.733319] GPR08: 00000001 c00103a0 c7fa41e0 c49afcc4 24282842 
>>> 10018840 c079b37c 00000040
>>> [  423.733319] GPR16: 73f00000 00210d00 00000000 00000001 c455a000 
>>> 00000100 00000200 c455a000
>>> [  423.733319] GPR24: c60053c0 c0011b34 007080c0 c455a000 c455a000 
>>> c7fa41e0 00000000 00009032
>>> [  423.734190] NIP [c0108f90] ___slab_alloc.constprop.30+0x26c/0x46c
>>> [  423.734257] LR [c0109ad0] kmem_cache_alloc+0x210/0x23c
>>> [  423.734283] Call Trace:
>>> [  423.734326] [c455bc50] [00000100] 0x100 (unreliable)
>>> [  423.734430] [c455bcc0] [c0109ad0] kmem_cache_alloc+0x210/0x23c
>>> [  423.734543] [c455bcf0] [c0011b34] huge_pte_alloc+0xc0/0x1dc
>>> [  423.734633] [c455bd20] [c01044dc] hugetlb_fault+0x408/0x48c
>>> [  423.734720] [c455bdb0] [c0104b20] follow_hugetlb_page+0x14c/0x44c
>>> [  423.734826] [c455be10] [c00e8e54] __get_user_pages+0x1c4/0x3dc
>>> [  423.734919] [c455be80] [c00e9924] __mm_populate+0xac/0x140
>>> [  423.735020] [c455bec0] [c00db14c] vm_mmap_pgoff+0xb4/0xb8
>>> [  423.735127] [c455bf00] [c00f27c0] ksys_mmap_pgoff+0xcc/0x1fc
>>> [  423.735222] [c455bf40] [c000e0f8] ret_from_syscall+0x0/0x38
>>> [  423.735271] Instruction dump:
>>> [  423.735321] 7cbf482e 38fd0008 7fa6eb78 7fc4f378 4bfff5dd 7fe3fb78 
>>> 4bfffe24 81370010
>>> [  423.735536] 71280004 41a2ff88 4840c571 4bffff80 <0fe00000> 
>>> 4bfffeb8 81340010 712a0004
>>> [  423.735757] ---[ end trace e9b222919a470790 ]---
>>>
>>> This warning occurs when calling kmem_cache_zalloc() on a
>>> cache having a constructor.
>>>
>>> In this case it happens because PGD cache and 512k hugepte cache are
>>> the same size (4k). While a cache with constructor is created for
>>> the PGD, hugepages create cache without constructor and uses
>>> kmem_cache_zalloc(). As both expect a cache with the same size,
>>> the hugepages reuse the cache created for PGD, hence the conflict.
>>>
>>> In order to avoid this conflict, this patch:
>>> - modifies pgtable_cache_add() so that a zeroising constructor is
>>> added for any cache size.
>>> - replaces calls to kmem_cache_zalloc() by kmem_cache_alloc()
>>>
>>
>> Can't we just do kmem_cache_alloc with gfp flags __GFP_ZERO? and 
>> remove the constructor completely?
> 
> I don't understand what you mean. That's exactly what I did in v1 (by 
> using kmem_cache_zalloc()), and you commented that doing this we would 
> zeroise at allocation whereas the constructors are called when adding 
> memory to the slab and when freeing the allocated block. Or did I 
> misunderstood your comment ?
> 
> static inline void *kmem_cache_zalloc(struct kmem_cache *k, gfp_t flags)
> {
>      return kmem_cache_alloc(k, flags | __GFP_ZERO);
> }
> 

Wasn't it what you meant in your comment to v1 ? If not, could you 
detail your thought so that I can take it in account in a v3 ?

Thanks
Christophe

Re: [PATCH v2 3/4] powerpc/mm: fix a warning when a cache is common to PGD and hugepages

[Aneesh Kumar K.V]

On 08/17/2018 04:14 PM, Christophe LEROY wrote:
> 
> 
> Le 17/08/2018 à 05:32, Aneesh Kumar K.V a écrit :
>> On 08/14/2018 08:24 PM, Christophe Leroy wrote:
>>> While implementing TLB miss HW assistance on the 8xx, the following
>>> warning was encountered:
>>>
>>> [  423.732965] WARNING: CPU: 0 PID: 345 at mm/slub.c:2412 
>>> ___slab_alloc.constprop.30+0x26c/0x46c
>>> [  423.733033] CPU: 0 PID: 345 Comm: mmap Not tainted 
>>> 4.18.0-rc8-00664-g2dfff9121c55 #671
>>> [  423.733075] NIP:  c0108f90 LR: c0109ad0 CTR: 00000004
>>> [  423.733121] REGS: c455bba0 TRAP: 0700   Not tainted 
>>> (4.18.0-rc8-00664-g2dfff9121c55)
>>> [  423.733147] MSR:  00021032 <ME,IR,DR,RI>  CR: 24224848  XER: 20000000
>>> [  423.733319]
>>> [  423.733319] GPR00: c0109ad0 c455bc50 c4521910 c60053c0 007080c0 
>>> c0011b34 c7fa41e0 c455be30
>>> [  423.733319] GPR08: 00000001 c00103a0 c7fa41e0 c49afcc4 24282842 
>>> 10018840 c079b37c 00000040
>>> [  423.733319] GPR16: 73f00000 00210d00 00000000 00000001 c455a000 
>>> 00000100 00000200 c455a000
>>> [  423.733319] GPR24: c60053c0 c0011b34 007080c0 c455a000 c455a000 
>>> c7fa41e0 00000000 00009032
>>> [  423.734190] NIP [c0108f90] ___slab_alloc.constprop.30+0x26c/0x46c
>>> [  423.734257] LR [c0109ad0] kmem_cache_alloc+0x210/0x23c
>>> [  423.734283] Call Trace:
>>> [  423.734326] [c455bc50] [00000100] 0x100 (unreliable)
>>> [  423.734430] [c455bcc0] [c0109ad0] kmem_cache_alloc+0x210/0x23c
>>> [  423.734543] [c455bcf0] [c0011b34] huge_pte_alloc+0xc0/0x1dc
>>> [  423.734633] [c455bd20] [c01044dc] hugetlb_fault+0x408/0x48c
>>> [  423.734720] [c455bdb0] [c0104b20] follow_hugetlb_page+0x14c/0x44c
>>> [  423.734826] [c455be10] [c00e8e54] __get_user_pages+0x1c4/0x3dc
>>> [  423.734919] [c455be80] [c00e9924] __mm_populate+0xac/0x140
>>> [  423.735020] [c455bec0] [c00db14c] vm_mmap_pgoff+0xb4/0xb8
>>> [  423.735127] [c455bf00] [c00f27c0] ksys_mmap_pgoff+0xcc/0x1fc
>>> [  423.735222] [c455bf40] [c000e0f8] ret_from_syscall+0x0/0x38
>>> [  423.735271] Instruction dump:
>>> [  423.735321] 7cbf482e 38fd0008 7fa6eb78 7fc4f378 4bfff5dd 7fe3fb78 
>>> 4bfffe24 81370010
>>> [  423.735536] 71280004 41a2ff88 4840c571 4bffff80 <0fe00000> 
>>> 4bfffeb8 81340010 712a0004
>>> [  423.735757] ---[ end trace e9b222919a470790 ]---
>>>
>>> This warning occurs when calling kmem_cache_zalloc() on a
>>> cache having a constructor.
>>>
>>> In this case it happens because PGD cache and 512k hugepte cache are
>>> the same size (4k). While a cache with constructor is created for
>>> the PGD, hugepages create cache without constructor and uses
>>> kmem_cache_zalloc(). As both expect a cache with the same size,
>>> the hugepages reuse the cache created for PGD, hence the conflict.
>>>
>>> In order to avoid this conflict, this patch:
>>> - modifies pgtable_cache_add() so that a zeroising constructor is
>>> added for any cache size.
>>> - replaces calls to kmem_cache_zalloc() by kmem_cache_alloc()
>>>
>>
>> Can't we just do kmem_cache_alloc with gfp flags __GFP_ZERO? and 
>> remove the constructor completely?
> 
> I don't understand what you mean. That's exactly what I did in v1 (by 
> using kmem_cache_zalloc()), and you commented that doing this we would 
> zeroise at allocation whereas the constructors are called when adding 
> memory to the slab and when freeing the allocated block. Or did I 
> misunderstood your comment ?
> 
> static inline void *kmem_cache_zalloc(struct kmem_cache *k, gfp_t flags)
> {
>      return kmem_cache_alloc(k, flags | __GFP_ZERO);
> }
> 
>

I completely misunderstood kmem_cache_zalloc. I took it as we zero out 
after each alloc. I guess your earlier patch is then good. We may want 
to double check this, I haven't looked at the slab internals.

What we want is to make sure when we add new memory to slab, we want it 
zeroed. If we are allocating objects from existing slab memory pool, we 
don't need to zero out, because when we release objects to slab we make 
sure we clear it.

-aneesh

Re: [PATCH v2 3/4] powerpc/mm: fix a warning when a cache is common to PGD and hugepages

[Christophe LEROY]

Le 22/08/2018 à 16:20, Aneesh Kumar K.V a écrit :
> On 08/17/2018 04:14 PM, Christophe LEROY wrote:
>>
>>
>> Le 17/08/2018 à 05:32, Aneesh Kumar K.V a écrit :
>>> On 08/14/2018 08:24 PM, Christophe Leroy wrote:
>>>> While implementing TLB miss HW assistance on the 8xx, the following
>>>> warning was encountered:
>>>>
>>>> [  423.732965] WARNING: CPU: 0 PID: 345 at mm/slub.c:2412 
>>>> ___slab_alloc.constprop.30+0x26c/0x46c
>>>> [  423.733033] CPU: 0 PID: 345 Comm: mmap Not tainted 
>>>> 4.18.0-rc8-00664-g2dfff9121c55 #671
>>>> [  423.733075] NIP:  c0108f90 LR: c0109ad0 CTR: 00000004
>>>> [  423.733121] REGS: c455bba0 TRAP: 0700   Not tainted 
>>>> (4.18.0-rc8-00664-g2dfff9121c55)
>>>> [  423.733147] MSR:  00021032 <ME,IR,DR,RI>  CR: 24224848  XER: 
>>>> 20000000
>>>> [  423.733319]
>>>> [  423.733319] GPR00: c0109ad0 c455bc50 c4521910 c60053c0 007080c0 
>>>> c0011b34 c7fa41e0 c455be30
>>>> [  423.733319] GPR08: 00000001 c00103a0 c7fa41e0 c49afcc4 24282842 
>>>> 10018840 c079b37c 00000040
>>>> [  423.733319] GPR16: 73f00000 00210d00 00000000 00000001 c455a000 
>>>> 00000100 00000200 c455a000
>>>> [  423.733319] GPR24: c60053c0 c0011b34 007080c0 c455a000 c455a000 
>>>> c7fa41e0 00000000 00009032
>>>> [  423.734190] NIP [c0108f90] ___slab_alloc.constprop.30+0x26c/0x46c
>>>> [  423.734257] LR [c0109ad0] kmem_cache_alloc+0x210/0x23c
>>>> [  423.734283] Call Trace:
>>>> [  423.734326] [c455bc50] [00000100] 0x100 (unreliable)
>>>> [  423.734430] [c455bcc0] [c0109ad0] kmem_cache_alloc+0x210/0x23c
>>>> [  423.734543] [c455bcf0] [c0011b34] huge_pte_alloc+0xc0/0x1dc
>>>> [  423.734633] [c455bd20] [c01044dc] hugetlb_fault+0x408/0x48c
>>>> [  423.734720] [c455bdb0] [c0104b20] follow_hugetlb_page+0x14c/0x44c
>>>> [  423.734826] [c455be10] [c00e8e54] __get_user_pages+0x1c4/0x3dc
>>>> [  423.734919] [c455be80] [c00e9924] __mm_populate+0xac/0x140
>>>> [  423.735020] [c455bec0] [c00db14c] vm_mmap_pgoff+0xb4/0xb8
>>>> [  423.735127] [c455bf00] [c00f27c0] ksys_mmap_pgoff+0xcc/0x1fc
>>>> [  423.735222] [c455bf40] [c000e0f8] ret_from_syscall+0x0/0x38
>>>> [  423.735271] Instruction dump:
>>>> [  423.735321] 7cbf482e 38fd0008 7fa6eb78 7fc4f378 4bfff5dd 7fe3fb78 
>>>> 4bfffe24 81370010
>>>> [  423.735536] 71280004 41a2ff88 4840c571 4bffff80 <0fe00000> 
>>>> 4bfffeb8 81340010 712a0004
>>>> [  423.735757] ---[ end trace e9b222919a470790 ]---
>>>>
>>>> This warning occurs when calling kmem_cache_zalloc() on a
>>>> cache having a constructor.
>>>>
>>>> In this case it happens because PGD cache and 512k hugepte cache are
>>>> the same size (4k). While a cache with constructor is created for
>>>> the PGD, hugepages create cache without constructor and uses
>>>> kmem_cache_zalloc(). As both expect a cache with the same size,
>>>> the hugepages reuse the cache created for PGD, hence the conflict.
>>>>
>>>> In order to avoid this conflict, this patch:
>>>> - modifies pgtable_cache_add() so that a zeroising constructor is
>>>> added for any cache size.
>>>> - replaces calls to kmem_cache_zalloc() by kmem_cache_alloc()
>>>>
>>>
>>> Can't we just do kmem_cache_alloc with gfp flags __GFP_ZERO? and 
>>> remove the constructor completely?
>>
>> I don't understand what you mean. That's exactly what I did in v1 (by 
>> using kmem_cache_zalloc()), and you commented that doing this we would 
>> zeroise at allocation whereas the constructors are called when adding 
>> memory to the slab and when freeing the allocated block. Or did I 
>> misunderstood your comment ?
>>
>> static inline void *kmem_cache_zalloc(struct kmem_cache *k, gfp_t flags)
>> {
>>      return kmem_cache_alloc(k, flags | __GFP_ZERO);
>> }
>>
>>
> 
> I completely misunderstood kmem_cache_zalloc. I took it as we zero out 
> after each alloc. I guess your earlier patch is then good. We may want 
> to double check this, I haven't looked at the slab internals.

In fact no, you were right. When kmem_cache_alloc() is called with 
__GFP_ZERO, the object gets zeroised at allocation. This is done (at 
least in SLUB) at the end of function slab_alloc_node()

> 
> What we want is to make sure when we add new memory to slab, we want it 
> zeroed. If we are allocating objects from existing slab memory pool, we 
> don't need to zero out, because when we release objects to slab we make 
> sure we clear it.

It looks like when we use constructors, they are called when adding an 
object to the slab and when releasing it back to the slab. So that's 
exactly what we want then, and therefore I have the feeling that we 
should go with this v2 approach.
Those constructors are tiny (most of them are 3 insns) and we have only 
16 cache sizes hence 16 constructors so it shoudln't be an issue to have 
unused ones.
The only small problème I have is that some version of GCC seems to 
complain about big memset() (132k and 256k ones). Is there a way to tell 
GCC we really want to do it ?

Christophe

> 
> -aneesh

Re: [PATCH v2 3/4] powerpc/mm: fix a warning when a cache is common to PGD and hugepages

[Segher Boessenkool]

On Thu, Aug 23, 2018 at 11:40:22AM +0200, Christophe LEROY wrote:
> The only small problème I have is that some version of GCC seems to 
> complain about big memset() (132k and 256k ones). Is there a way to tell 
> GCC we really want to do it ?

I'm not sure what you mean.  Complain, is that a warning, is that an error?
What does it say?  Do you have some example code to reproduce it?  Etc.

Very many things use tiny memsets like that, so you must mean something
more specialised.


Segher

Re: [PATCH v2 3/4] powerpc/mm: fix a warning when a cache is common to PGD and hugepages

[Christophe LEROY]

Le 23/08/2018 à 12:36, Segher Boessenkool a écrit :
> On Thu, Aug 23, 2018 at 11:40:22AM +0200, Christophe LEROY wrote:
>> The only small problème I have is that some version of GCC seems to
>> complain about big memset() (132k and 256k ones). Is there a way to tell
>> GCC we really want to do it ?
> 
> I'm not sure what you mean.  Complain, is that a warning, is that an error?
> What does it say?  Do you have some example code to reproduce it?  Etc.

I saw the warnings in the checks at 
https://patchwork.ozlabs.org/patch/957566/
Unfortunatly the link is now broken.

Christophe

> 
> Very many things use tiny memsets like that, so you must mean something
> more specialised.
> 
> 
> Segher
> 

Re: [PATCH v2 3/4] powerpc/mm: fix a warning when a cache is common to PGD and hugepages

[Andrew Donnellan]

On 23/08/18 21:56, Michael Ellerman wrote:
> Christophe LEROY <christophe.leroy@c-s.fr> writes:
> 
>> Le 23/08/2018 à 12:36, Segher Boessenkool a écrit :
>>> On Thu, Aug 23, 2018 at 11:40:22AM +0200, Christophe LEROY wrote:
>>>> The only small problème I have is that some version of GCC seems to
>>>> complain about big memset() (132k and 256k ones). Is there a way to tell
>>>> GCC we really want to do it ?
>>>
>>> I'm not sure what you mean.  Complain, is that a warning, is that an error?
>>> What does it say?  Do you have some example code to reproduce it?  Etc.
>>
>> I saw the warnings in the checks at
>> https://patchwork.ozlabs.org/patch/957566/
>> Unfortunatly the link is now broken.
> 
> ruscur/ajd any idea what happened to the snowpatch links here?

I think they've disappeared because our log rotation is too fast - I've 
now upped it to 30 days. I guess over time we'll figure out what we need 
in this regard, ideally we'd keep logs indefinitely but they're several 
megs per build.

I've kicked off another build for this series and the links in Patchwork 
should update to point to the new job when it's done (probably in the 
next couple of hours).

-- 
Andrew Donnellan              OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com  IBM Australia Limited

Re: [PATCH v2 3/4] powerpc/mm: fix a warning when a cache is common to PGD and hugepages

[Segher Boessenkool]

On Thu, Aug 23, 2018 at 11:32:16PM +1000, Andrew Donnellan wrote:
> On 23/08/18 21:56, Michael Ellerman wrote:
> >Christophe LEROY <christophe.leroy@c-s.fr> writes:
> >
> >>Le 23/08/2018 à 12:36, Segher Boessenkool a écrit :
> >>>On Thu, Aug 23, 2018 at 11:40:22AM +0200, Christophe LEROY wrote:
> >>>>The only small problème I have is that some version of GCC seems to
> >>>>complain about big memset() (132k and 256k ones). Is there a way to tell
> >>>>GCC we really want to do it ?
> >>>
> >>>I'm not sure what you mean.  Complain, is that a warning, is that an 
> >>>error?
> >>>What does it say?  Do you have some example code to reproduce it?  Etc.
> >>
> >>I saw the warnings in the checks at
> >>https://patchwork.ozlabs.org/patch/957566/
> >>Unfortunatly the link is now broken.
> >
> >ruscur/ajd any idea what happened to the snowpatch links here?
> 
> I think they've disappeared because our log rotation is too fast - I've 
> now upped it to 30 days. I guess over time we'll figure out what we need 
> in this regard, ideally we'd keep logs indefinitely but they're several 
> megs per build.
> 
> I've kicked off another build for this series and the links in Patchwork 
> should update to point to the new job when it's done (probably in the 
> next couple of hours).

It's back, thanks Andrew!

The warnings are not from GCC at all: the warnings are from sparse.


Segher

Re: [PATCH v2 3/4] powerpc/mm: fix a warning when a cache is common to PGD and hugepages

[Christophe LEROY]

Le 23/08/2018 à 16:41, Segher Boessenkool a écrit :
> On Thu, Aug 23, 2018 at 11:32:16PM +1000, Andrew Donnellan wrote:
>> On 23/08/18 21:56, Michael Ellerman wrote:
>>> Christophe LEROY <christophe.leroy@c-s.fr> writes:
>>>
>>>> Le 23/08/2018 à 12:36, Segher Boessenkool a écrit :
>>>>> On Thu, Aug 23, 2018 at 11:40:22AM +0200, Christophe LEROY wrote:
>>>>>> The only small problème I have is that some version of GCC seems to
>>>>>> complain about big memset() (132k and 256k ones). Is there a way to tell
>>>>>> GCC we really want to do it ?
>>>>>
>>>>> I'm not sure what you mean.  Complain, is that a warning, is that an
>>>>> error?
>>>>> What does it say?  Do you have some example code to reproduce it?  Etc.
>>>>
>>>> I saw the warnings in the checks at
>>>> https://patchwork.ozlabs.org/patch/957566/
>>>> Unfortunatly the link is now broken.
>>>
>>> ruscur/ajd any idea what happened to the snowpatch links here?
>>
>> I think they've disappeared because our log rotation is too fast - I've
>> now upped it to 30 days. I guess over time we'll figure out what we need
>> in this regard, ideally we'd keep logs indefinitely but they're several
>> megs per build.
>>
>> I've kicked off another build for this series and the links in Patchwork
>> should update to point to the new job when it's done (probably in the
>> next couple of hours).
> 
> It's back, thanks Andrew!
> 
> The warnings are not from GCC at all: the warnings are from sparse.

Oh, ok, my mistake, I reminded seeing those warnings without paying much 
attention to them at that time.

Anyway, should we do anything about this warning ? If so, what 
could/should be done ?

Christophe

Re: [PATCH v2 3/4] powerpc/mm: fix a warning when a cache is common to PGD and hugepages

[Michael Ellerman]

Segher Boessenkool <segher@kernel.crashing.org> writes:

> On Thu, Aug 23, 2018 at 11:32:16PM +1000, Andrew Donnellan wrote:
>> On 23/08/18 21:56, Michael Ellerman wrote:
>> >Christophe LEROY <christophe.leroy@c-s.fr> writes:
>> >
>> >>Le 23/08/2018 à 12:36, Segher Boessenkool a écrit :
>> >>>On Thu, Aug 23, 2018 at 11:40:22AM +0200, Christophe LEROY wrote:
>> >>>>The only small problème I have is that some version of GCC seems to
>> >>>>complain about big memset() (132k and 256k ones). Is there a way to tell
>> >>>>GCC we really want to do it ?
>> >>>
>> >>>I'm not sure what you mean.  Complain, is that a warning, is that an 
>> >>>error?
>> >>>What does it say?  Do you have some example code to reproduce it?  Etc.
>> >>
>> >>I saw the warnings in the checks at
>> >>https://patchwork.ozlabs.org/patch/957566/
>> >>Unfortunatly the link is now broken.
>> >
>> >ruscur/ajd any idea what happened to the snowpatch links here?
>> 
>> I think they've disappeared because our log rotation is too fast - I've 
>> now upped it to 30 days. I guess over time we'll figure out what we need 
>> in this regard, ideally we'd keep logs indefinitely but they're several 
>> megs per build.
>> 
>> I've kicked off another build for this series and the links in Patchwork 
>> should update to point to the new job when it's done (probably in the 
>> next couple of hours).
>
> It's back, thanks Andrew!
>
> The warnings are not from GCC at all: the warnings are from sparse.

We really need to split them out so it's less confusing.

cheers

[PATCH v2 4/4] powerpc/mm: remove unnecessary test in pgtable_cache_init()

[Christophe Leroy]

pgtable_cache_add() gracefully handles the case when a cache that
size already exists by returning early with the following test:

	if (PGT_CACHE(shift))
		return; /* Already have a cache of this size */

It is then not needed to test the existence of the cache before.

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 arch/powerpc/mm/init-common.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/mm/init-common.c b/arch/powerpc/mm/init-common.c
index b7ca03643d0b..1e6910eb70ed 100644
--- a/arch/powerpc/mm/init-common.c
+++ b/arch/powerpc/mm/init-common.c
@@ -111,13 +111,13 @@ void pgtable_cache_init(void)
 {
 	pgtable_cache_add(PGD_INDEX_SIZE);
 
-	if (PMD_CACHE_INDEX && !PGT_CACHE(PMD_CACHE_INDEX))
+	if (PMD_CACHE_INDEX)
 		pgtable_cache_add(PMD_CACHE_INDEX);
 	/*
 	 * In all current configs, when the PUD index exists it's the
 	 * same size as either the pgd or pmd index except with THP enabled
 	 * on book3s 64
 	 */
-	if (PUD_CACHE_INDEX && !PGT_CACHE(PUD_CACHE_INDEX))
+	if (PUD_CACHE_INDEX)
 		pgtable_cache_add(PUD_CACHE_INDEX);
 }
-- 
2.13.3