[PATCH/RFC 0/3] Optimize CONFIG_DEBUG_PAGEALLOC

[PATCH/RFC 0/3] Optimize CONFIG_DEBUG_PAGEALLOC

[Christian Borntraeger]

As CONFIG_DEBUG_PAGEALLOC can be enabled/disabled via kernel
parameters we can optimize some cases by checking the enablement
state.

I have done s390 and x86 as examples. Other architecture can
provide followup patches later on.

Christian Borntraeger (3):
  mm: provide debug_pagealloc_enabled() without CONFIG_DEBUG_PAGEALLOC
  x86: query dynamic DEBUG_PAGEALLOC setting
  s390: query dynamic DEBUG_PAGEALLOC setting

 arch/s390/kernel/dumpstack.c |  4 +++-
 arch/s390/mm/vmem.c          | 10 ++++------
 arch/x86/kernel/dumpstack.c  |  4 +++-
 arch/x86/mm/init.c           |  7 ++++---
 arch/x86/mm/pageattr.c       | 14 ++++----------
 include/linux/mm.h           |  4 ++++
 6 files changed, 22 insertions(+), 21 deletions(-)

-- 
2.3.0

[PATCH/RFC 0/3] Optimize CONFIG_DEBUG_PAGEALLOC

[Christian Borntraeger]

As CONFIG_DEBUG_PAGEALLOC can be enabled/disabled via kernel
parameters we can optimize some cases by checking the enablement
state.

I have done s390 and x86 as examples. Other architecture can
provide followup patches later on.

Christian Borntraeger (3):
  mm: provide debug_pagealloc_enabled() without CONFIG_DEBUG_PAGEALLOC
  x86: query dynamic DEBUG_PAGEALLOC setting
  s390: query dynamic DEBUG_PAGEALLOC setting

 arch/s390/kernel/dumpstack.c |  4 +++-
 arch/s390/mm/vmem.c          | 10 ++++------
 arch/x86/kernel/dumpstack.c  |  4 +++-
 arch/x86/mm/init.c           |  7 ++++---
 arch/x86/mm/pageattr.c       | 14 ++++----------
 include/linux/mm.h           |  4 ++++
 6 files changed, 22 insertions(+), 21 deletions(-)

-- 
2.3.0

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH/RFC 1/3] mm: provide debug_pagealloc_enabled() without CONFIG_DEBUG_PAGEALLOC

[Christian Borntraeger]

We can provide debug_pagealloc_enabled() also if CONFIG_DEBUG_PAGEALLOC
is not set. It will return false in that case.

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 include/linux/mm.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 7783073..fbc5354 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2148,6 +2148,10 @@ kernel_map_pages(struct page *page, int numpages, int enable)
 extern bool kernel_page_present(struct page *page);
 #endif /* CONFIG_HIBERNATION */
 #else
+static inline bool debug_pagealloc_enabled(void)
+{
+	return false;
+}
 static inline void
 kernel_map_pages(struct page *page, int numpages, int enable) {}
 #ifdef CONFIG_HIBERNATION
-- 
2.3.0

[PATCH/RFC 1/3] mm: provide debug_pagealloc_enabled() without CONFIG_DEBUG_PAGEALLOC

[Christian Borntraeger]

We can provide debug_pagealloc_enabled() also if CONFIG_DEBUG_PAGEALLOC
is not set. It will return false in that case.

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 include/linux/mm.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 7783073..fbc5354 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2148,6 +2148,10 @@ kernel_map_pages(struct page *page, int numpages, int enable)
 extern bool kernel_page_present(struct page *page);
 #endif /* CONFIG_HIBERNATION */
 #else
+static inline bool debug_pagealloc_enabled(void)
+{
+	return false;
+}
 static inline void
 kernel_map_pages(struct page *page, int numpages, int enable) {}
 #ifdef CONFIG_HIBERNATION
-- 
2.3.0

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH/RFC 2/3] x86: query dynamic DEBUG_PAGEALLOC setting

[Christian Borntraeger]

We can use debug_pagealloc_enabled() to check if we can map
the identity mapping with 2MB pages. We can also add the state
into the dump_stack output.

The patch does not touch the code for the 1GB pages, which ignored
CONFIG_DEBUG_PAGEALLOC. Do we need to fence this as well?

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 arch/x86/kernel/dumpstack.c |  4 +++-
 arch/x86/mm/init.c          |  7 ++++---
 arch/x86/mm/pageattr.c      | 14 ++++----------
 3 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c
index 9c30acf..7971638 100644
--- a/arch/x86/kernel/dumpstack.c
+++ b/arch/x86/kernel/dumpstack.c
@@ -266,7 +266,9 @@ int __die(const char *str, struct pt_regs *regs, long err)
 	printk("SMP ");
 #endif
 #ifdef CONFIG_DEBUG_PAGEALLOC
-	printk("DEBUG_PAGEALLOC ");
+	printk("DEBUG_PAGEALLOC(%s) ",
+		debug_pagealloc_enabled() ? "enabled" : "disabled");
+
 #endif
 #ifdef CONFIG_KASAN
 	printk("KASAN");
diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
index 493f541..39823fd 100644
--- a/arch/x86/mm/init.c
+++ b/arch/x86/mm/init.c
@@ -150,13 +150,14 @@ static int page_size_mask;
 
 static void __init probe_page_size_mask(void)
 {
-#if !defined(CONFIG_DEBUG_PAGEALLOC) && !defined(CONFIG_KMEMCHECK)
+#if !defined(CONFIG_KMEMCHECK)
 	/*
-	 * For CONFIG_DEBUG_PAGEALLOC, identity mapping will use small pages.
+	 * For CONFIG_KMEMCHECK or pagealloc debugging, identity mapping will
+	 * use small pages.
 	 * This will simplify cpa(), which otherwise needs to support splitting
 	 * large pages into small in interrupt context, etc.
 	 */
-	if (cpu_has_pse)
+	if (cpu_has_pse && !debug_pagealloc_enabled())
 		page_size_mask |= 1 << PG_LEVEL_2M;
 #endif
 
diff --git a/arch/x86/mm/pageattr.c b/arch/x86/mm/pageattr.c
index a3137a4..a49c8fd 100644
--- a/arch/x86/mm/pageattr.c
+++ b/arch/x86/mm/pageattr.c
@@ -103,12 +103,6 @@ static inline unsigned long highmap_end_pfn(void)
 
 #endif
 
-#ifdef CONFIG_DEBUG_PAGEALLOC
-# define debug_pagealloc 1
-#else
-# define debug_pagealloc 0
-#endif
-
 static inline int
 within(unsigned long addr, unsigned long start, unsigned long end)
 {
@@ -703,10 +697,10 @@ static int split_large_page(struct cpa_data *cpa, pte_t *kpte,
 {
 	struct page *base;
 
-	if (!debug_pagealloc)
+	if (!debug_pagealloc_enabled())
 		spin_unlock(&cpa_lock);
 	base = alloc_pages(GFP_KERNEL | __GFP_NOTRACK, 0);
-	if (!debug_pagealloc)
+	if (!debug_pagealloc_enabled())
 		spin_lock(&cpa_lock);
 	if (!base)
 		return -ENOMEM;
@@ -1326,10 +1320,10 @@ static int __change_page_attr_set_clr(struct cpa_data *cpa, int checkalias)
 		if (cpa->flags & (CPA_ARRAY | CPA_PAGES_ARRAY))
 			cpa->numpages = 1;
 
-		if (!debug_pagealloc)
+		if (!debug_pagealloc_enabled())
 			spin_lock(&cpa_lock);
 		ret = __change_page_attr(cpa, checkalias);
-		if (!debug_pagealloc)
+		if (!debug_pagealloc_enabled())
 			spin_unlock(&cpa_lock);
 		if (ret)
 			return ret;
-- 
2.3.0

[PATCH/RFC 2/3] x86: query dynamic DEBUG_PAGEALLOC setting

[Christian Borntraeger]

We can use debug_pagealloc_enabled() to check if we can map
the identity mapping with 2MB pages. We can also add the state
into the dump_stack output.

The patch does not touch the code for the 1GB pages, which ignored
CONFIG_DEBUG_PAGEALLOC. Do we need to fence this as well?

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 arch/x86/kernel/dumpstack.c |  4 +++-
 arch/x86/mm/init.c          |  7 ++++---
 arch/x86/mm/pageattr.c      | 14 ++++----------
 3 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c
index 9c30acf..7971638 100644
--- a/arch/x86/kernel/dumpstack.c
+++ b/arch/x86/kernel/dumpstack.c
@@ -266,7 +266,9 @@ int __die(const char *str, struct pt_regs *regs, long err)
 	printk("SMP ");
 #endif
 #ifdef CONFIG_DEBUG_PAGEALLOC
-	printk("DEBUG_PAGEALLOC ");
+	printk("DEBUG_PAGEALLOC(%s) ",
+		debug_pagealloc_enabled() ? "enabled" : "disabled");
+
 #endif
 #ifdef CONFIG_KASAN
 	printk("KASAN");
diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
index 493f541..39823fd 100644
--- a/arch/x86/mm/init.c
+++ b/arch/x86/mm/init.c
@@ -150,13 +150,14 @@ static int page_size_mask;
 
 static void __init probe_page_size_mask(void)
 {
-#if !defined(CONFIG_DEBUG_PAGEALLOC) && !defined(CONFIG_KMEMCHECK)
+#if !defined(CONFIG_KMEMCHECK)
 	/*
-	 * For CONFIG_DEBUG_PAGEALLOC, identity mapping will use small pages.
+	 * For CONFIG_KMEMCHECK or pagealloc debugging, identity mapping will
+	 * use small pages.
 	 * This will simplify cpa(), which otherwise needs to support splitting
 	 * large pages into small in interrupt context, etc.
 	 */
-	if (cpu_has_pse)
+	if (cpu_has_pse && !debug_pagealloc_enabled())
 		page_size_mask |= 1 << PG_LEVEL_2M;
 #endif
 
diff --git a/arch/x86/mm/pageattr.c b/arch/x86/mm/pageattr.c
index a3137a4..a49c8fd 100644
--- a/arch/x86/mm/pageattr.c
+++ b/arch/x86/mm/pageattr.c
@@ -103,12 +103,6 @@ static inline unsigned long highmap_end_pfn(void)
 
 #endif
 
-#ifdef CONFIG_DEBUG_PAGEALLOC
-# define debug_pagealloc 1
-#else
-# define debug_pagealloc 0
-#endif
-
 static inline int
 within(unsigned long addr, unsigned long start, unsigned long end)
 {
@@ -703,10 +697,10 @@ static int split_large_page(struct cpa_data *cpa, pte_t *kpte,
 {
 	struct page *base;
 
-	if (!debug_pagealloc)
+	if (!debug_pagealloc_enabled())
 		spin_unlock(&cpa_lock);
 	base = alloc_pages(GFP_KERNEL | __GFP_NOTRACK, 0);
-	if (!debug_pagealloc)
+	if (!debug_pagealloc_enabled())
 		spin_lock(&cpa_lock);
 	if (!base)
 		return -ENOMEM;
@@ -1326,10 +1320,10 @@ static int __change_page_attr_set_clr(struct cpa_data *cpa, int checkalias)
 		if (cpa->flags & (CPA_ARRAY | CPA_PAGES_ARRAY))
 			cpa->numpages = 1;
 
-		if (!debug_pagealloc)
+		if (!debug_pagealloc_enabled())
 			spin_lock(&cpa_lock);
 		ret = __change_page_attr(cpa, checkalias);
-		if (!debug_pagealloc)
+		if (!debug_pagealloc_enabled())
 			spin_unlock(&cpa_lock);
 		if (ret)
 			return ret;
-- 
2.3.0

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH/RFC 3/3] s390: query dynamic DEBUG_PAGEALLOC setting

[Christian Borntraeger]

We can use debug_pagealloc_enabled() to check if we can map
the identity mapping with 1MB/2GB pages as well as to print
the current setting in dump_stack.

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 arch/s390/kernel/dumpstack.c |  4 +++-
 arch/s390/mm/vmem.c          | 10 ++++------
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/s390/kernel/dumpstack.c b/arch/s390/kernel/dumpstack.c
index dc8e204..a1c0530 100644
--- a/arch/s390/kernel/dumpstack.c
+++ b/arch/s390/kernel/dumpstack.c
@@ -11,6 +11,7 @@
 #include <linux/export.h>
 #include <linux/kdebug.h>
 #include <linux/ptrace.h>
+#include <linux/mm.h>
 #include <linux/module.h>
 #include <linux/sched.h>
 #include <asm/processor.h>
@@ -186,7 +187,8 @@ void die(struct pt_regs *regs, const char *str)
 	printk("SMP ");
 #endif
 #ifdef CONFIG_DEBUG_PAGEALLOC
-	printk("DEBUG_PAGEALLOC");
+	printk("DEBUG_PAGEALLOC(%s)",
+		debug_pagealloc_enabled() ? "enabled" : "disabled");
 #endif
 	printk("\n");
 	notify_die(DIE_OOPS, str, regs, 0, regs->int_code & 0xffff, SIGSEGV);
diff --git a/arch/s390/mm/vmem.c b/arch/s390/mm/vmem.c
index ef7d6c8..d27fccba 100644
--- a/arch/s390/mm/vmem.c
+++ b/arch/s390/mm/vmem.c
@@ -94,16 +94,15 @@ static int vmem_add_mem(unsigned long start, unsigned long size, int ro)
 			pgd_populate(&init_mm, pg_dir, pu_dir);
 		}
 		pu_dir = pud_offset(pg_dir, address);
-#ifndef CONFIG_DEBUG_PAGEALLOC
 		if (MACHINE_HAS_EDAT2 && pud_none(*pu_dir) && address &&
-		    !(address & ~PUD_MASK) && (address + PUD_SIZE <= end)) {
+		    !(address & ~PUD_MASK) && (address + PUD_SIZE <= end) &&
+		     !debug_pagealloc_enabled()) {
 			pud_val(*pu_dir) = __pa(address) |
 				_REGION_ENTRY_TYPE_R3 | _REGION3_ENTRY_LARGE |
 				(ro ? _REGION_ENTRY_PROTECT : 0);
 			address += PUD_SIZE;
 			continue;
 		}
-#endif
 		if (pud_none(*pu_dir)) {
 			pm_dir = vmem_pmd_alloc();
 			if (!pm_dir)
@@ -111,9 +110,9 @@ static int vmem_add_mem(unsigned long start, unsigned long size, int ro)
 			pud_populate(&init_mm, pu_dir, pm_dir);
 		}
 		pm_dir = pmd_offset(pu_dir, address);
-#ifndef CONFIG_DEBUG_PAGEALLOC
 		if (MACHINE_HAS_EDAT1 && pmd_none(*pm_dir) && address &&
-		    !(address & ~PMD_MASK) && (address + PMD_SIZE <= end)) {
+		    !(address & ~PMD_MASK) && (address + PMD_SIZE <= end) &&
+		    !debug_pagealloc_enabled()) {
 			pmd_val(*pm_dir) = __pa(address) |
 				_SEGMENT_ENTRY | _SEGMENT_ENTRY_LARGE |
 				_SEGMENT_ENTRY_YOUNG |
@@ -121,7 +120,6 @@ static int vmem_add_mem(unsigned long start, unsigned long size, int ro)
 			address += PMD_SIZE;
 			continue;
 		}
-#endif
 		if (pmd_none(*pm_dir)) {
 			pt_dir = vmem_pte_alloc(address);
 			if (!pt_dir)
-- 
2.3.0

[PATCH/RFC 3/3] s390: query dynamic DEBUG_PAGEALLOC setting

[Christian Borntraeger]

We can use debug_pagealloc_enabled() to check if we can map
the identity mapping with 1MB/2GB pages as well as to print
the current setting in dump_stack.

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 arch/s390/kernel/dumpstack.c |  4 +++-
 arch/s390/mm/vmem.c          | 10 ++++------
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/s390/kernel/dumpstack.c b/arch/s390/kernel/dumpstack.c
index dc8e204..a1c0530 100644
--- a/arch/s390/kernel/dumpstack.c
+++ b/arch/s390/kernel/dumpstack.c
@@ -11,6 +11,7 @@
 #include <linux/export.h>
 #include <linux/kdebug.h>
 #include <linux/ptrace.h>
+#include <linux/mm.h>
 #include <linux/module.h>
 #include <linux/sched.h>
 #include <asm/processor.h>
@@ -186,7 +187,8 @@ void die(struct pt_regs *regs, const char *str)
 	printk("SMP ");
 #endif
 #ifdef CONFIG_DEBUG_PAGEALLOC
-	printk("DEBUG_PAGEALLOC");
+	printk("DEBUG_PAGEALLOC(%s)",
+		debug_pagealloc_enabled() ? "enabled" : "disabled");
 #endif
 	printk("\n");
 	notify_die(DIE_OOPS, str, regs, 0, regs->int_code & 0xffff, SIGSEGV);
diff --git a/arch/s390/mm/vmem.c b/arch/s390/mm/vmem.c
index ef7d6c8..d27fccba 100644
--- a/arch/s390/mm/vmem.c
+++ b/arch/s390/mm/vmem.c
@@ -94,16 +94,15 @@ static int vmem_add_mem(unsigned long start, unsigned long size, int ro)
 			pgd_populate(&init_mm, pg_dir, pu_dir);
 		}
 		pu_dir = pud_offset(pg_dir, address);
-#ifndef CONFIG_DEBUG_PAGEALLOC
 		if (MACHINE_HAS_EDAT2 && pud_none(*pu_dir) && address &&
-		    !(address & ~PUD_MASK) && (address + PUD_SIZE <= end)) {
+		    !(address & ~PUD_MASK) && (address + PUD_SIZE <= end) &&
+		     !debug_pagealloc_enabled()) {
 			pud_val(*pu_dir) = __pa(address) |
 				_REGION_ENTRY_TYPE_R3 | _REGION3_ENTRY_LARGE |
 				(ro ? _REGION_ENTRY_PROTECT : 0);
 			address += PUD_SIZE;
 			continue;
 		}
-#endif
 		if (pud_none(*pu_dir)) {
 			pm_dir = vmem_pmd_alloc();
 			if (!pm_dir)
@@ -111,9 +110,9 @@ static int vmem_add_mem(unsigned long start, unsigned long size, int ro)
 			pud_populate(&init_mm, pu_dir, pm_dir);
 		}
 		pm_dir = pmd_offset(pu_dir, address);
-#ifndef CONFIG_DEBUG_PAGEALLOC
 		if (MACHINE_HAS_EDAT1 && pmd_none(*pm_dir) && address &&
-		    !(address & ~PMD_MASK) && (address + PMD_SIZE <= end)) {
+		    !(address & ~PMD_MASK) && (address + PMD_SIZE <= end) &&
+		    !debug_pagealloc_enabled()) {
 			pmd_val(*pm_dir) = __pa(address) |
 				_SEGMENT_ENTRY | _SEGMENT_ENTRY_LARGE |
 				_SEGMENT_ENTRY_YOUNG |
@@ -121,7 +120,6 @@ static int vmem_add_mem(unsigned long start, unsigned long size, int ro)
 			address += PMD_SIZE;
 			continue;
 		}
-#endif
 		if (pmd_none(*pm_dir)) {
 			pt_dir = vmem_pte_alloc(address);
 			if (!pt_dir)
-- 
2.3.0

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH/RFC 3/3] s390: query dynamic DEBUG_PAGEALLOC setting

[Heiko Carstens]

On Tue, Jan 26, 2016 at 10:18:25AM +0100, Christian Borntraeger wrote:
> We can use debug_pagealloc_enabled() to check if we can map
> the identity mapping with 1MB/2GB pages as well as to print
> the current setting in dump_stack.
> 
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
>  arch/s390/kernel/dumpstack.c |  4 +++-
>  arch/s390/mm/vmem.c          | 10 ++++------
>  2 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/s390/kernel/dumpstack.c b/arch/s390/kernel/dumpstack.c
> index dc8e204..a1c0530 100644
> --- a/arch/s390/kernel/dumpstack.c
> +++ b/arch/s390/kernel/dumpstack.c
> @@ -11,6 +11,7 @@
>  #include <linux/export.h>
>  #include <linux/kdebug.h>
>  #include <linux/ptrace.h>
> +#include <linux/mm.h>
>  #include <linux/module.h>
>  #include <linux/sched.h>
>  #include <asm/processor.h>
> @@ -186,7 +187,8 @@ void die(struct pt_regs *regs, const char *str)
>  	printk("SMP ");
>  #endif
>  #ifdef CONFIG_DEBUG_PAGEALLOC
> -	printk("DEBUG_PAGEALLOC");
> +	printk("DEBUG_PAGEALLOC(%s)",
> +		debug_pagealloc_enabled() ? "enabled" : "disabled");
>  #endif

I'd prefer if you change this to

	if (debug_pagealloc_enabled())
		printk("DEBUG_PAGEALLOC");

That way we can get rid of yet another ifdef. Having
"DEBUG_PAGEALLOC(disabled)" doesn't seem to be very helpful.

Re: [PATCH/RFC 3/3] s390: query dynamic DEBUG_PAGEALLOC setting

[Heiko Carstens]

On Tue, Jan 26, 2016 at 10:18:25AM +0100, Christian Borntraeger wrote:
> We can use debug_pagealloc_enabled() to check if we can map
> the identity mapping with 1MB/2GB pages as well as to print
> the current setting in dump_stack.
> 
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
>  arch/s390/kernel/dumpstack.c |  4 +++-
>  arch/s390/mm/vmem.c          | 10 ++++------
>  2 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/s390/kernel/dumpstack.c b/arch/s390/kernel/dumpstack.c
> index dc8e204..a1c0530 100644
> --- a/arch/s390/kernel/dumpstack.c
> +++ b/arch/s390/kernel/dumpstack.c
> @@ -11,6 +11,7 @@
>  #include <linux/export.h>
>  #include <linux/kdebug.h>
>  #include <linux/ptrace.h>
> +#include <linux/mm.h>
>  #include <linux/module.h>
>  #include <linux/sched.h>
>  #include <asm/processor.h>
> @@ -186,7 +187,8 @@ void die(struct pt_regs *regs, const char *str)
>  	printk("SMP ");
>  #endif
>  #ifdef CONFIG_DEBUG_PAGEALLOC
> -	printk("DEBUG_PAGEALLOC");
> +	printk("DEBUG_PAGEALLOC(%s)",
> +		debug_pagealloc_enabled() ? "enabled" : "disabled");
>  #endif

I'd prefer if you change this to

	if (debug_pagealloc_enabled())
		printk("DEBUG_PAGEALLOC");

That way we can get rid of yet another ifdef. Having
"DEBUG_PAGEALLOC(disabled)" doesn't seem to be very helpful.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH/RFC 3/3] s390: query dynamic DEBUG_PAGEALLOC setting

[Thomas Gleixner]

On Tue, 26 Jan 2016, Heiko Carstens wrote:
> On Tue, Jan 26, 2016 at 10:18:25AM +0100, Christian Borntraeger wrote:
> > We can use debug_pagealloc_enabled() to check if we can map
> > the identity mapping with 1MB/2GB pages as well as to print
> > the current setting in dump_stack.
> > 
> > Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> > ---
> >  arch/s390/kernel/dumpstack.c |  4 +++-
> >  arch/s390/mm/vmem.c          | 10 ++++------
> >  2 files changed, 7 insertions(+), 7 deletions(-)
> > 
> > diff --git a/arch/s390/kernel/dumpstack.c b/arch/s390/kernel/dumpstack.c
> > index dc8e204..a1c0530 100644
> > --- a/arch/s390/kernel/dumpstack.c
> > +++ b/arch/s390/kernel/dumpstack.c
> > @@ -11,6 +11,7 @@
> >  #include <linux/export.h>
> >  #include <linux/kdebug.h>
> >  #include <linux/ptrace.h>
> > +#include <linux/mm.h>
> >  #include <linux/module.h>
> >  #include <linux/sched.h>
> >  #include <asm/processor.h>
> > @@ -186,7 +187,8 @@ void die(struct pt_regs *regs, const char *str)
> >  	printk("SMP ");
> >  #endif
> >  #ifdef CONFIG_DEBUG_PAGEALLOC
> > -	printk("DEBUG_PAGEALLOC");
> > +	printk("DEBUG_PAGEALLOC(%s)",
> > +		debug_pagealloc_enabled() ? "enabled" : "disabled");
> >  #endif
> 
> I'd prefer if you change this to
> 
> 	if (debug_pagealloc_enabled())
> 		printk("DEBUG_PAGEALLOC");
> 
> That way we can get rid of yet another ifdef. Having
> "DEBUG_PAGEALLOC(disabled)" doesn't seem to be very helpful.

Yes, same for x86 please.

Thanks,

	tglx

Re: [PATCH/RFC 3/3] s390: query dynamic DEBUG_PAGEALLOC setting

[Thomas Gleixner]

On Tue, 26 Jan 2016, Heiko Carstens wrote:
> On Tue, Jan 26, 2016 at 10:18:25AM +0100, Christian Borntraeger wrote:
> > We can use debug_pagealloc_enabled() to check if we can map
> > the identity mapping with 1MB/2GB pages as well as to print
> > the current setting in dump_stack.
> > 
> > Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> > ---
> >  arch/s390/kernel/dumpstack.c |  4 +++-
> >  arch/s390/mm/vmem.c          | 10 ++++------
> >  2 files changed, 7 insertions(+), 7 deletions(-)
> > 
> > diff --git a/arch/s390/kernel/dumpstack.c b/arch/s390/kernel/dumpstack.c
> > index dc8e204..a1c0530 100644
> > --- a/arch/s390/kernel/dumpstack.c
> > +++ b/arch/s390/kernel/dumpstack.c
> > @@ -11,6 +11,7 @@
> >  #include <linux/export.h>
> >  #include <linux/kdebug.h>
> >  #include <linux/ptrace.h>
> > +#include <linux/mm.h>
> >  #include <linux/module.h>
> >  #include <linux/sched.h>
> >  #include <asm/processor.h>
> > @@ -186,7 +187,8 @@ void die(struct pt_regs *regs, const char *str)
> >  	printk("SMP ");
> >  #endif
> >  #ifdef CONFIG_DEBUG_PAGEALLOC
> > -	printk("DEBUG_PAGEALLOC");
> > +	printk("DEBUG_PAGEALLOC(%s)",
> > +		debug_pagealloc_enabled() ? "enabled" : "disabled");
> >  #endif
> 
> I'd prefer if you change this to
> 
> 	if (debug_pagealloc_enabled())
> 		printk("DEBUG_PAGEALLOC");
> 
> That way we can get rid of yet another ifdef. Having
> "DEBUG_PAGEALLOC(disabled)" doesn't seem to be very helpful.

Yes, same for x86 please.

Thanks,

	tglx

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH/RFC 1/3] mm: provide debug_pagealloc_enabled() without CONFIG_DEBUG_PAGEALLOC

[David Rientjes]

On Tue, 26 Jan 2016, Christian Borntraeger wrote:

> We can provide debug_pagealloc_enabled() also if CONFIG_DEBUG_PAGEALLOC
> is not set. It will return false in that case.
> 
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
>  include/linux/mm.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 7783073..fbc5354 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -2148,6 +2148,10 @@ kernel_map_pages(struct page *page, int numpages, int enable)
>  extern bool kernel_page_present(struct page *page);
>  #endif /* CONFIG_HIBERNATION */
>  #else
> +static inline bool debug_pagealloc_enabled(void)
> +{
> +	return false;
> +}
>  static inline void
>  kernel_map_pages(struct page *page, int numpages, int enable) {}
>  #ifdef CONFIG_HIBERNATION

Since the patchset needs to be refreshed based on Heiko and Thomas's 
comments, please add some /* CONFIG_DEBUG_PAGEALLOC */ annotation to the 
#else and #endif lines so this block is easier to read.  After that, feel 
free to add

	Acked-by: David Rientjes <rientjes@google.com>

Re: [PATCH/RFC 1/3] mm: provide debug_pagealloc_enabled() without CONFIG_DEBUG_PAGEALLOC

[David Rientjes]

On Tue, 26 Jan 2016, Christian Borntraeger wrote:

> We can provide debug_pagealloc_enabled() also if CONFIG_DEBUG_PAGEALLOC
> is not set. It will return false in that case.
> 
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
>  include/linux/mm.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 7783073..fbc5354 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -2148,6 +2148,10 @@ kernel_map_pages(struct page *page, int numpages, int enable)
>  extern bool kernel_page_present(struct page *page);
>  #endif /* CONFIG_HIBERNATION */
>  #else
> +static inline bool debug_pagealloc_enabled(void)
> +{
> +	return false;
> +}
>  static inline void
>  kernel_map_pages(struct page *page, int numpages, int enable) {}
>  #ifdef CONFIG_HIBERNATION

Since the patchset needs to be refreshed based on Heiko and Thomas's 
comments, please add some /* CONFIG_DEBUG_PAGEALLOC */ annotation to the 
#else and #endif lines so this block is easier to read.  After that, feel 
free to add

	Acked-by: David Rientjes <rientjes@google.com>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH/RFC 3/3] s390: query dynamic DEBUG_PAGEALLOC setting

[David Rientjes]

On Tue, 26 Jan 2016, Heiko Carstens wrote:

> On Tue, Jan 26, 2016 at 10:18:25AM +0100, Christian Borntraeger wrote:
> > We can use debug_pagealloc_enabled() to check if we can map
> > the identity mapping with 1MB/2GB pages as well as to print
> > the current setting in dump_stack.
> > 
> > Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> > ---
> >  arch/s390/kernel/dumpstack.c |  4 +++-
> >  arch/s390/mm/vmem.c          | 10 ++++------
> >  2 files changed, 7 insertions(+), 7 deletions(-)
> > 
> > diff --git a/arch/s390/kernel/dumpstack.c b/arch/s390/kernel/dumpstack.c
> > index dc8e204..a1c0530 100644
> > --- a/arch/s390/kernel/dumpstack.c
> > +++ b/arch/s390/kernel/dumpstack.c
> > @@ -11,6 +11,7 @@
> >  #include <linux/export.h>
> >  #include <linux/kdebug.h>
> >  #include <linux/ptrace.h>
> > +#include <linux/mm.h>
> >  #include <linux/module.h>
> >  #include <linux/sched.h>
> >  #include <asm/processor.h>
> > @@ -186,7 +187,8 @@ void die(struct pt_regs *regs, const char *str)
> >  	printk("SMP ");
> >  #endif
> >  #ifdef CONFIG_DEBUG_PAGEALLOC
> > -	printk("DEBUG_PAGEALLOC");
> > +	printk("DEBUG_PAGEALLOC(%s)",
> > +		debug_pagealloc_enabled() ? "enabled" : "disabled");
> >  #endif
> 
> I'd prefer if you change this to
> 
> 	if (debug_pagealloc_enabled())
> 		printk("DEBUG_PAGEALLOC");
> 
> That way we can get rid of yet another ifdef. Having
> "DEBUG_PAGEALLOC(disabled)" doesn't seem to be very helpful.
> 

I'd agree if CONFIG_DEBUG_PAGEALLOC only did anything when 
debug_pagealloc_enabled() is true, but that doesn't seem to be the case.  
When CONFIG_DEBUG_SLAB is enabled, for instance, CONFIG_DEBUG_PAGEALLOC 
also enables stackinfo storing and poisoning and it's not guarded by 
debug_pagealloc_enabled().

It seems like CONFIG_DEBUG_PAGEALLOC enables debugging functionality 
outside the scope of the debug_pagealloc=on kernel parameter, so 
DEBUG_PAGEALLOC(disabled) actually does mean something.

Re: [PATCH/RFC 3/3] s390: query dynamic DEBUG_PAGEALLOC setting

[David Rientjes]

On Tue, 26 Jan 2016, Heiko Carstens wrote:

> On Tue, Jan 26, 2016 at 10:18:25AM +0100, Christian Borntraeger wrote:
> > We can use debug_pagealloc_enabled() to check if we can map
> > the identity mapping with 1MB/2GB pages as well as to print
> > the current setting in dump_stack.
> > 
> > Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> > ---
> >  arch/s390/kernel/dumpstack.c |  4 +++-
> >  arch/s390/mm/vmem.c          | 10 ++++------
> >  2 files changed, 7 insertions(+), 7 deletions(-)
> > 
> > diff --git a/arch/s390/kernel/dumpstack.c b/arch/s390/kernel/dumpstack.c
> > index dc8e204..a1c0530 100644
> > --- a/arch/s390/kernel/dumpstack.c
> > +++ b/arch/s390/kernel/dumpstack.c
> > @@ -11,6 +11,7 @@
> >  #include <linux/export.h>
> >  #include <linux/kdebug.h>
> >  #include <linux/ptrace.h>
> > +#include <linux/mm.h>
> >  #include <linux/module.h>
> >  #include <linux/sched.h>
> >  #include <asm/processor.h>
> > @@ -186,7 +187,8 @@ void die(struct pt_regs *regs, const char *str)
> >  	printk("SMP ");
> >  #endif
> >  #ifdef CONFIG_DEBUG_PAGEALLOC
> > -	printk("DEBUG_PAGEALLOC");
> > +	printk("DEBUG_PAGEALLOC(%s)",
> > +		debug_pagealloc_enabled() ? "enabled" : "disabled");
> >  #endif
> 
> I'd prefer if you change this to
> 
> 	if (debug_pagealloc_enabled())
> 		printk("DEBUG_PAGEALLOC");
> 
> That way we can get rid of yet another ifdef. Having
> "DEBUG_PAGEALLOC(disabled)" doesn't seem to be very helpful.
> 

I'd agree if CONFIG_DEBUG_PAGEALLOC only did anything when 
debug_pagealloc_enabled() is true, but that doesn't seem to be the case.  
When CONFIG_DEBUG_SLAB is enabled, for instance, CONFIG_DEBUG_PAGEALLOC 
also enables stackinfo storing and poisoning and it's not guarded by 
debug_pagealloc_enabled().

It seems like CONFIG_DEBUG_PAGEALLOC enables debugging functionality 
outside the scope of the debug_pagealloc=on kernel parameter, so 
DEBUG_PAGEALLOC(disabled) actually does mean something.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH/RFC 3/3] s390: query dynamic DEBUG_PAGEALLOC setting

[Joonsoo Kim]

On Tue, Jan 26, 2016 at 03:29:38PM -0800, David Rientjes wrote:
> On Tue, 26 Jan 2016, Heiko Carstens wrote:
> 
> > On Tue, Jan 26, 2016 at 10:18:25AM +0100, Christian Borntraeger wrote:
> > > We can use debug_pagealloc_enabled() to check if we can map
> > > the identity mapping with 1MB/2GB pages as well as to print
> > > the current setting in dump_stack.
> > > 
> > > Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> > > ---
> > >  arch/s390/kernel/dumpstack.c |  4 +++-
> > >  arch/s390/mm/vmem.c          | 10 ++++------
> > >  2 files changed, 7 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/arch/s390/kernel/dumpstack.c b/arch/s390/kernel/dumpstack.c
> > > index dc8e204..a1c0530 100644
> > > --- a/arch/s390/kernel/dumpstack.c
> > > +++ b/arch/s390/kernel/dumpstack.c
> > > @@ -11,6 +11,7 @@
> > >  #include <linux/export.h>
> > >  #include <linux/kdebug.h>
> > >  #include <linux/ptrace.h>
> > > +#include <linux/mm.h>
> > >  #include <linux/module.h>
> > >  #include <linux/sched.h>
> > >  #include <asm/processor.h>
> > > @@ -186,7 +187,8 @@ void die(struct pt_regs *regs, const char *str)
> > >  	printk("SMP ");
> > >  #endif
> > >  #ifdef CONFIG_DEBUG_PAGEALLOC
> > > -	printk("DEBUG_PAGEALLOC");
> > > +	printk("DEBUG_PAGEALLOC(%s)",
> > > +		debug_pagealloc_enabled() ? "enabled" : "disabled");
> > >  #endif
> > 
> > I'd prefer if you change this to
> > 
> > 	if (debug_pagealloc_enabled())
> > 		printk("DEBUG_PAGEALLOC");
> > 
> > That way we can get rid of yet another ifdef. Having
> > "DEBUG_PAGEALLOC(disabled)" doesn't seem to be very helpful.
> > 
> 
> I'd agree if CONFIG_DEBUG_PAGEALLOC only did anything when 
> debug_pagealloc_enabled() is true, but that doesn't seem to be the case.  
> When CONFIG_DEBUG_SLAB is enabled, for instance, CONFIG_DEBUG_PAGEALLOC 
> also enables stackinfo storing and poisoning and it's not guarded by 
> debug_pagealloc_enabled().
> 
> It seems like CONFIG_DEBUG_PAGEALLOC enables debugging functionality 
> outside the scope of the debug_pagealloc=on kernel parameter, so 
> DEBUG_PAGEALLOC(disabled) actually does mean something.

Hello, David.

I tried to fix CONFIG_DEBUG_SLAB case on 04/16 of following patchset.

http://thread.gmane.org/gmane.linux.kernel.mm/144527

I found that there are more sites to fix but not so many.
We can do it.

Thanks.

Re: [PATCH/RFC 3/3] s390: query dynamic DEBUG_PAGEALLOC setting

[Joonsoo Kim]

On Tue, Jan 26, 2016 at 03:29:38PM -0800, David Rientjes wrote:
> On Tue, 26 Jan 2016, Heiko Carstens wrote:
> 
> > On Tue, Jan 26, 2016 at 10:18:25AM +0100, Christian Borntraeger wrote:
> > > We can use debug_pagealloc_enabled() to check if we can map
> > > the identity mapping with 1MB/2GB pages as well as to print
> > > the current setting in dump_stack.
> > > 
> > > Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> > > ---
> > >  arch/s390/kernel/dumpstack.c |  4 +++-
> > >  arch/s390/mm/vmem.c          | 10 ++++------
> > >  2 files changed, 7 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/arch/s390/kernel/dumpstack.c b/arch/s390/kernel/dumpstack.c
> > > index dc8e204..a1c0530 100644
> > > --- a/arch/s390/kernel/dumpstack.c
> > > +++ b/arch/s390/kernel/dumpstack.c
> > > @@ -11,6 +11,7 @@
> > >  #include <linux/export.h>
> > >  #include <linux/kdebug.h>
> > >  #include <linux/ptrace.h>
> > > +#include <linux/mm.h>
> > >  #include <linux/module.h>
> > >  #include <linux/sched.h>
> > >  #include <asm/processor.h>
> > > @@ -186,7 +187,8 @@ void die(struct pt_regs *regs, const char *str)
> > >  	printk("SMP ");
> > >  #endif
> > >  #ifdef CONFIG_DEBUG_PAGEALLOC
> > > -	printk("DEBUG_PAGEALLOC");
> > > +	printk("DEBUG_PAGEALLOC(%s)",
> > > +		debug_pagealloc_enabled() ? "enabled" : "disabled");
> > >  #endif
> > 
> > I'd prefer if you change this to
> > 
> > 	if (debug_pagealloc_enabled())
> > 		printk("DEBUG_PAGEALLOC");
> > 
> > That way we can get rid of yet another ifdef. Having
> > "DEBUG_PAGEALLOC(disabled)" doesn't seem to be very helpful.
> > 
> 
> I'd agree if CONFIG_DEBUG_PAGEALLOC only did anything when 
> debug_pagealloc_enabled() is true, but that doesn't seem to be the case.  
> When CONFIG_DEBUG_SLAB is enabled, for instance, CONFIG_DEBUG_PAGEALLOC 
> also enables stackinfo storing and poisoning and it's not guarded by 
> debug_pagealloc_enabled().
> 
> It seems like CONFIG_DEBUG_PAGEALLOC enables debugging functionality 
> outside the scope of the debug_pagealloc=on kernel parameter, so 
> DEBUG_PAGEALLOC(disabled) actually does mean something.

Hello, David.

I tried to fix CONFIG_DEBUG_SLAB case on 04/16 of following patchset.

http://thread.gmane.org/gmane.linux.kernel.mm/144527

I found that there are more sites to fix but not so many.
We can do it.

Thanks.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH/RFC 3/3] s390: query dynamic DEBUG_PAGEALLOC setting

[David Rientjes]

On Wed, 27 Jan 2016, Joonsoo Kim wrote:

> > I'd agree if CONFIG_DEBUG_PAGEALLOC only did anything when 
> > debug_pagealloc_enabled() is true, but that doesn't seem to be the case.  
> > When CONFIG_DEBUG_SLAB is enabled, for instance, CONFIG_DEBUG_PAGEALLOC 
> > also enables stackinfo storing and poisoning and it's not guarded by 
> > debug_pagealloc_enabled().
> > 
> > It seems like CONFIG_DEBUG_PAGEALLOC enables debugging functionality 
> > outside the scope of the debug_pagealloc=on kernel parameter, so 
> > DEBUG_PAGEALLOC(disabled) actually does mean something.
> 
> Hello, David.
> 
> I tried to fix CONFIG_DEBUG_SLAB case on 04/16 of following patchset.
> 
> http://thread.gmane.org/gmane.linux.kernel.mm/144527
> 
> I found that there are more sites to fix but not so many.
> We can do it.
> 

For the slab case, sure, this can be fixed, but there is other code that 
uses CONFIG_DEBUG_PAGEALLOC to suggest debugging is always enabled and is 
indifferent to debug_pagealloc_enabled().  I find this in powerpc and 
sparc arch code as well as generic vmalloc code.  

If we can convert existing users that only check for 
CONFIG_DEBUG_PAGEALLOC to rather check for debug_pagealloc_enabled() and 
agree that it is only enabled for debug_pagealloc=on, then this would seem 
fine.  However, I think we should at least consult with those users before 
removing an artifact from the kernel log that could be useful in debugging 
why a particular BUG() happened.

Re: [PATCH/RFC 3/3] s390: query dynamic DEBUG_PAGEALLOC setting

[David Rientjes]

On Wed, 27 Jan 2016, Joonsoo Kim wrote:

> > I'd agree if CONFIG_DEBUG_PAGEALLOC only did anything when 
> > debug_pagealloc_enabled() is true, but that doesn't seem to be the case.  
> > When CONFIG_DEBUG_SLAB is enabled, for instance, CONFIG_DEBUG_PAGEALLOC 
> > also enables stackinfo storing and poisoning and it's not guarded by 
> > debug_pagealloc_enabled().
> > 
> > It seems like CONFIG_DEBUG_PAGEALLOC enables debugging functionality 
> > outside the scope of the debug_pagealloc=on kernel parameter, so 
> > DEBUG_PAGEALLOC(disabled) actually does mean something.
> 
> Hello, David.
> 
> I tried to fix CONFIG_DEBUG_SLAB case on 04/16 of following patchset.
> 
> http://thread.gmane.org/gmane.linux.kernel.mm/144527
> 
> I found that there are more sites to fix but not so many.
> We can do it.
> 

For the slab case, sure, this can be fixed, but there is other code that 
uses CONFIG_DEBUG_PAGEALLOC to suggest debugging is always enabled and is 
indifferent to debug_pagealloc_enabled().  I find this in powerpc and 
sparc arch code as well as generic vmalloc code.  

If we can convert existing users that only check for 
CONFIG_DEBUG_PAGEALLOC to rather check for debug_pagealloc_enabled() and 
agree that it is only enabled for debug_pagealloc=on, then this would seem 
fine.  However, I think we should at least consult with those users before 
removing an artifact from the kernel log that could be useful in debugging 
why a particular BUG() happened.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH/RFC 3/3] s390: query dynamic DEBUG_PAGEALLOC setting

[Joonsoo Kim]

On Tue, Jan 26, 2016 at 04:36:11PM -0800, David Rientjes wrote:
> On Wed, 27 Jan 2016, Joonsoo Kim wrote:
> 
> > > I'd agree if CONFIG_DEBUG_PAGEALLOC only did anything when 
> > > debug_pagealloc_enabled() is true, but that doesn't seem to be the case.  
> > > When CONFIG_DEBUG_SLAB is enabled, for instance, CONFIG_DEBUG_PAGEALLOC 
> > > also enables stackinfo storing and poisoning and it's not guarded by 
> > > debug_pagealloc_enabled().
> > > 
> > > It seems like CONFIG_DEBUG_PAGEALLOC enables debugging functionality 
> > > outside the scope of the debug_pagealloc=on kernel parameter, so 
> > > DEBUG_PAGEALLOC(disabled) actually does mean something.
> > 
> > Hello, David.
> > 
> > I tried to fix CONFIG_DEBUG_SLAB case on 04/16 of following patchset.
> > 
> > http://thread.gmane.org/gmane.linux.kernel.mm/144527
> > 
> > I found that there are more sites to fix but not so many.
> > We can do it.
> > 
> 
> For the slab case, sure, this can be fixed, but there is other code that 
> uses CONFIG_DEBUG_PAGEALLOC to suggest debugging is always enabled and is 
> indifferent to debug_pagealloc_enabled().  I find this in powerpc and 
> sparc arch code as well as generic vmalloc code.  

Yes, I also found it.

> 
> If we can convert existing users that only check for 
> CONFIG_DEBUG_PAGEALLOC to rather check for debug_pagealloc_enabled() and 
> agree that it is only enabled for debug_pagealloc=on, then this would seem 
> fine.  However, I think we should at least consult with those users before 
> removing an artifact from the kernel log that could be useful in debugging 
> why a particular BUG() happened.

Yes, at least, non-architecture dependent code (vmalloc, SLAB, SLUB) should
be changed first. If Christian doesn't mind, I will try to fix above 3
things.

Thanks.

Re: [PATCH/RFC 3/3] s390: query dynamic DEBUG_PAGEALLOC setting

[Joonsoo Kim]

On Tue, Jan 26, 2016 at 04:36:11PM -0800, David Rientjes wrote:
> On Wed, 27 Jan 2016, Joonsoo Kim wrote:
> 
> > > I'd agree if CONFIG_DEBUG_PAGEALLOC only did anything when 
> > > debug_pagealloc_enabled() is true, but that doesn't seem to be the case.  
> > > When CONFIG_DEBUG_SLAB is enabled, for instance, CONFIG_DEBUG_PAGEALLOC 
> > > also enables stackinfo storing and poisoning and it's not guarded by 
> > > debug_pagealloc_enabled().
> > > 
> > > It seems like CONFIG_DEBUG_PAGEALLOC enables debugging functionality 
> > > outside the scope of the debug_pagealloc=on kernel parameter, so 
> > > DEBUG_PAGEALLOC(disabled) actually does mean something.
> > 
> > Hello, David.
> > 
> > I tried to fix CONFIG_DEBUG_SLAB case on 04/16 of following patchset.
> > 
> > http://thread.gmane.org/gmane.linux.kernel.mm/144527
> > 
> > I found that there are more sites to fix but not so many.
> > We can do it.
> > 
> 
> For the slab case, sure, this can be fixed, but there is other code that 
> uses CONFIG_DEBUG_PAGEALLOC to suggest debugging is always enabled and is 
> indifferent to debug_pagealloc_enabled().  I find this in powerpc and 
> sparc arch code as well as generic vmalloc code.  

Yes, I also found it.

> 
> If we can convert existing users that only check for 
> CONFIG_DEBUG_PAGEALLOC to rather check for debug_pagealloc_enabled() and 
> agree that it is only enabled for debug_pagealloc=on, then this would seem 
> fine.  However, I think we should at least consult with those users before 
> removing an artifact from the kernel log that could be useful in debugging 
> why a particular BUG() happened.

Yes, at least, non-architecture dependent code (vmalloc, SLAB, SLUB) should
be changed first. If Christian doesn't mind, I will try to fix above 3
things.

Thanks.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH/RFC 3/3] s390: query dynamic DEBUG_PAGEALLOC setting

[Christian Borntraeger]

On 01/27/2016 01:59 AM, Joonsoo Kim wrote:
> On Tue, Jan 26, 2016 at 04:36:11PM -0800, David Rientjes wrote:
>> On Wed, 27 Jan 2016, Joonsoo Kim wrote:
>>
>>>> I'd agree if CONFIG_DEBUG_PAGEALLOC only did anything when 
>>>> debug_pagealloc_enabled() is true, but that doesn't seem to be the case.  
>>>> When CONFIG_DEBUG_SLAB is enabled, for instance, CONFIG_DEBUG_PAGEALLOC 
>>>> also enables stackinfo storing and poisoning and it's not guarded by 
>>>> debug_pagealloc_enabled().
>>>>
>>>> It seems like CONFIG_DEBUG_PAGEALLOC enables debugging functionality 
>>>> outside the scope of the debug_pagealloc=on kernel parameter, so 
>>>> DEBUG_PAGEALLOC(disabled) actually does mean something.
>>>
>>> Hello, David.
>>>
>>> I tried to fix CONFIG_DEBUG_SLAB case on 04/16 of following patchset.
>>>
>>> http://thread.gmane.org/gmane.linux.kernel.mm/144527
>>>
>>> I found that there are more sites to fix but not so many.
>>> We can do it.
>>>
>>
>> For the slab case, sure, this can be fixed, but there is other code that 
>> uses CONFIG_DEBUG_PAGEALLOC to suggest debugging is always enabled and is 
>> indifferent to debug_pagealloc_enabled().  I find this in powerpc and 
>> sparc arch code as well as generic vmalloc code.  
> 
> Yes, I also found it.
> 
>>
>> If we can convert existing users that only check for 
>> CONFIG_DEBUG_PAGEALLOC to rather check for debug_pagealloc_enabled() and 
>> agree that it is only enabled for debug_pagealloc=on, then this would seem 
>> fine.  However, I think we should at least consult with those users before 
>> removing an artifact from the kernel log that could be useful in debugging 
>> why a particular BUG() happened.
> 
> Yes, at least, non-architecture dependent code (vmalloc, SLAB, SLUB) should
> be changed first. If Christian doesn't mind, I will try to fix above 3
> things.

Ok. So I will change this as Heiko and Thomas suggested for s390 and x86 and 
resend these 3 patches. Feel free to work on the other areas.

Christian

Re: [PATCH/RFC 3/3] s390: query dynamic DEBUG_PAGEALLOC setting

[Christian Borntraeger]

On 01/27/2016 01:59 AM, Joonsoo Kim wrote:
> On Tue, Jan 26, 2016 at 04:36:11PM -0800, David Rientjes wrote:
>> On Wed, 27 Jan 2016, Joonsoo Kim wrote:
>>
>>>> I'd agree if CONFIG_DEBUG_PAGEALLOC only did anything when 
>>>> debug_pagealloc_enabled() is true, but that doesn't seem to be the case.  
>>>> When CONFIG_DEBUG_SLAB is enabled, for instance, CONFIG_DEBUG_PAGEALLOC 
>>>> also enables stackinfo storing and poisoning and it's not guarded by 
>>>> debug_pagealloc_enabled().
>>>>
>>>> It seems like CONFIG_DEBUG_PAGEALLOC enables debugging functionality 
>>>> outside the scope of the debug_pagealloc=on kernel parameter, so 
>>>> DEBUG_PAGEALLOC(disabled) actually does mean something.
>>>
>>> Hello, David.
>>>
>>> I tried to fix CONFIG_DEBUG_SLAB case on 04/16 of following patchset.
>>>
>>> http://thread.gmane.org/gmane.linux.kernel.mm/144527
>>>
>>> I found that there are more sites to fix but not so many.
>>> We can do it.
>>>
>>
>> For the slab case, sure, this can be fixed, but there is other code that 
>> uses CONFIG_DEBUG_PAGEALLOC to suggest debugging is always enabled and is 
>> indifferent to debug_pagealloc_enabled().  I find this in powerpc and 
>> sparc arch code as well as generic vmalloc code.  
> 
> Yes, I also found it.
> 
>>
>> If we can convert existing users that only check for 
>> CONFIG_DEBUG_PAGEALLOC to rather check for debug_pagealloc_enabled() and 
>> agree that it is only enabled for debug_pagealloc=on, then this would seem 
>> fine.  However, I think we should at least consult with those users before 
>> removing an artifact from the kernel log that could be useful in debugging 
>> why a particular BUG() happened.
> 
> Yes, at least, non-architecture dependent code (vmalloc, SLAB, SLUB) should
> be changed first. If Christian doesn't mind, I will try to fix above 3
> things.

Ok. So I will change this as Heiko and Thomas suggested for s390 and x86 and 
resend these 3 patches. Feel free to work on the other areas.

Christian

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH/RFC 3/3] s390: query dynamic DEBUG_PAGEALLOC setting

[Vlastimil Babka]

On 01/27/2016 01:59 AM, Joonsoo Kim wrote:
> On Tue, Jan 26, 2016 at 04:36:11PM -0800, David Rientjes wrote:
>> 
>> If we can convert existing users that only check for 
>> CONFIG_DEBUG_PAGEALLOC to rather check for debug_pagealloc_enabled() and 
>> agree that it is only enabled for debug_pagealloc=on, then this would seem 
>> fine.  However, I think we should at least consult with those users before 
>> removing an artifact from the kernel log that could be useful in debugging 
>> why a particular BUG() happened.
> 
> Yes, at least, non-architecture dependent code (vmalloc, SLAB, SLUB) should
> be changed first. If Christian doesn't mind, I will try to fix above 3
> things.

I think it might be worth also to convert debug_pagealloc_enabled() to be based
on static key, like I did for page_owner [1]. That should help make it possible
to have virtually no overhead when compiling kernel with CONFIG_DEBUG_PAGEALLOC
without enabling it boot-time. I assume it's one of the goals here?

[1] http://www.spinics.net/lists/linux-mm/msg100795.html

> Thanks.
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
> 

Re: [PATCH/RFC 3/3] s390: query dynamic DEBUG_PAGEALLOC setting

[Vlastimil Babka]

On 01/27/2016 01:59 AM, Joonsoo Kim wrote:
> On Tue, Jan 26, 2016 at 04:36:11PM -0800, David Rientjes wrote:
>> 
>> If we can convert existing users that only check for 
>> CONFIG_DEBUG_PAGEALLOC to rather check for debug_pagealloc_enabled() and 
>> agree that it is only enabled for debug_pagealloc=on, then this would seem 
>> fine.  However, I think we should at least consult with those users before 
>> removing an artifact from the kernel log that could be useful in debugging 
>> why a particular BUG() happened.
> 
> Yes, at least, non-architecture dependent code (vmalloc, SLAB, SLUB) should
> be changed first. If Christian doesn't mind, I will try to fix above 3
> things.

I think it might be worth also to convert debug_pagealloc_enabled() to be based
on static key, like I did for page_owner [1]. That should help make it possible
to have virtually no overhead when compiling kernel with CONFIG_DEBUG_PAGEALLOC
without enabling it boot-time. I assume it's one of the goals here?

[1] http://www.spinics.net/lists/linux-mm/msg100795.html

> Thanks.
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
> 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH/RFC 3/3] s390: query dynamic DEBUG_PAGEALLOC setting

[Christian Borntraeger]

On 01/27/2016 01:41 PM, Vlastimil Babka wrote:
> On 01/27/2016 01:59 AM, Joonsoo Kim wrote:
>> On Tue, Jan 26, 2016 at 04:36:11PM -0800, David Rientjes wrote:
>>>
>>> If we can convert existing users that only check for 
>>> CONFIG_DEBUG_PAGEALLOC to rather check for debug_pagealloc_enabled() and 
>>> agree that it is only enabled for debug_pagealloc=on, then this would seem 
>>> fine.  However, I think we should at least consult with those users before 
>>> removing an artifact from the kernel log that could be useful in debugging 
>>> why a particular BUG() happened.
>>
>> Yes, at least, non-architecture dependent code (vmalloc, SLAB, SLUB) should
>> be changed first. If Christian doesn't mind, I will try to fix above 3
>> things.
> 
> I think it might be worth also to convert debug_pagealloc_enabled() to be based
> on static key, like I did for page_owner [1]. That should help make it possible
> to have virtually no overhead when compiling kernel with CONFIG_DEBUG_PAGEALLOC
> without enabling it boot-time. I assume it's one of the goals here?

We could do something like that but dump_stack and setup of the initial identity
mapping of the kernel as well as the initial page protection are not hot path
as far as I can tell. Any other places?

> 
> [1] http://www.spinics.net/lists/linux-mm/msg100795.html

Re: [PATCH/RFC 3/3] s390: query dynamic DEBUG_PAGEALLOC setting

[Christian Borntraeger]

On 01/27/2016 01:41 PM, Vlastimil Babka wrote:
> On 01/27/2016 01:59 AM, Joonsoo Kim wrote:
>> On Tue, Jan 26, 2016 at 04:36:11PM -0800, David Rientjes wrote:
>>>
>>> If we can convert existing users that only check for 
>>> CONFIG_DEBUG_PAGEALLOC to rather check for debug_pagealloc_enabled() and 
>>> agree that it is only enabled for debug_pagealloc=on, then this would seem 
>>> fine.  However, I think we should at least consult with those users before 
>>> removing an artifact from the kernel log that could be useful in debugging 
>>> why a particular BUG() happened.
>>
>> Yes, at least, non-architecture dependent code (vmalloc, SLAB, SLUB) should
>> be changed first. If Christian doesn't mind, I will try to fix above 3
>> things.
> 
> I think it might be worth also to convert debug_pagealloc_enabled() to be based
> on static key, like I did for page_owner [1]. That should help make it possible
> to have virtually no overhead when compiling kernel with CONFIG_DEBUG_PAGEALLOC
> without enabling it boot-time. I assume it's one of the goals here?

We could do something like that but dump_stack and setup of the initial identity
mapping of the kernel as well as the initial page protection are not hot path
as far as I can tell. Any other places?

> 
> [1] http://www.spinics.net/lists/linux-mm/msg100795.html



--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH/RFC 3/3] s390: query dynamic DEBUG_PAGEALLOC setting

[Vlastimil Babka]

On 01/27/2016 01:47 PM, Christian Borntraeger wrote:
> On 01/27/2016 01:41 PM, Vlastimil Babka wrote:
>> On 01/27/2016 01:59 AM, Joonsoo Kim wrote:
>> 
>> I think it might be worth also to convert debug_pagealloc_enabled() to be based
>> on static key, like I did for page_owner [1]. That should help make it possible
>> to have virtually no overhead when compiling kernel with CONFIG_DEBUG_PAGEALLOC
>> without enabling it boot-time. I assume it's one of the goals here?
> 
> We could do something like that but dump_stack and setup of the initial identity
> mapping of the kernel as well as the initial page protection are not hot path
> as far as I can tell. Any other places?

Well, mostly kernel_map_pages() which is used in page allocation hotpaths.

>> 
>> [1] http://www.spinics.net/lists/linux-mm/msg100795.html
> 
> 
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
> 

Re: [PATCH/RFC 3/3] s390: query dynamic DEBUG_PAGEALLOC setting

[Vlastimil Babka]

On 01/27/2016 01:47 PM, Christian Borntraeger wrote:
> On 01/27/2016 01:41 PM, Vlastimil Babka wrote:
>> On 01/27/2016 01:59 AM, Joonsoo Kim wrote:
>> 
>> I think it might be worth also to convert debug_pagealloc_enabled() to be based
>> on static key, like I did for page_owner [1]. That should help make it possible
>> to have virtually no overhead when compiling kernel with CONFIG_DEBUG_PAGEALLOC
>> without enabling it boot-time. I assume it's one of the goals here?
> 
> We could do something like that but dump_stack and setup of the initial identity
> mapping of the kernel as well as the initial page protection are not hot path
> as far as I can tell. Any other places?

Well, mostly kernel_map_pages() which is used in page allocation hotpaths.

>> 
>> [1] http://www.spinics.net/lists/linux-mm/msg100795.html
> 
> 
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
> 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH/RFC 3/3] s390: query dynamic DEBUG_PAGEALLOC setting

[Christian Borntraeger]

On 01/27/2016 02:03 PM, Vlastimil Babka wrote:
> On 01/27/2016 01:47 PM, Christian Borntraeger wrote:
>> On 01/27/2016 01:41 PM, Vlastimil Babka wrote:
>>> On 01/27/2016 01:59 AM, Joonsoo Kim wrote:
>>>
>>> I think it might be worth also to convert debug_pagealloc_enabled() to be based
>>> on static key, like I did for page_owner [1]. That should help make it possible
>>> to have virtually no overhead when compiling kernel with CONFIG_DEBUG_PAGEALLOC
>>> without enabling it boot-time. I assume it's one of the goals here?
>>
>> We could do something like that but dump_stack and setup of the initial identity
>> mapping of the kernel as well as the initial page protection are not hot path
>> as far as I can tell. Any other places?
> 
> Well, mostly kernel_map_pages() which is used in page allocation hotpaths.

Right. So indeed, static_key would be a good thing, but certainly an addon patch.

Re: [PATCH/RFC 3/3] s390: query dynamic DEBUG_PAGEALLOC setting

[Christian Borntraeger]

On 01/27/2016 02:03 PM, Vlastimil Babka wrote:
> On 01/27/2016 01:47 PM, Christian Borntraeger wrote:
>> On 01/27/2016 01:41 PM, Vlastimil Babka wrote:
>>> On 01/27/2016 01:59 AM, Joonsoo Kim wrote:
>>>
>>> I think it might be worth also to convert debug_pagealloc_enabled() to be based
>>> on static key, like I did for page_owner [1]. That should help make it possible
>>> to have virtually no overhead when compiling kernel with CONFIG_DEBUG_PAGEALLOC
>>> without enabling it boot-time. I assume it's one of the goals here?
>>
>> We could do something like that but dump_stack and setup of the initial identity
>> mapping of the kernel as well as the initial page protection are not hot path
>> as far as I can tell. Any other places?
> 
> Well, mostly kernel_map_pages() which is used in page allocation hotpaths.

Right. So indeed, static_key would be a good thing, but certainly an addon patch.



--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH/RFC 3/3] s390: query dynamic DEBUG_PAGEALLOC setting

[Joonsoo Kim]

On Wed, Jan 27, 2016 at 02:03:36PM +0100, Vlastimil Babka wrote:
> On 01/27/2016 01:47 PM, Christian Borntraeger wrote:
> > On 01/27/2016 01:41 PM, Vlastimil Babka wrote:
> >> On 01/27/2016 01:59 AM, Joonsoo Kim wrote:
> >> 
> >> I think it might be worth also to convert debug_pagealloc_enabled() to be based
> >> on static key, like I did for page_owner [1]. That should help make it possible
> >> to have virtually no overhead when compiling kernel with CONFIG_DEBUG_PAGEALLOC
> >> without enabling it boot-time. I assume it's one of the goals here?
> > 
> > We could do something like that but dump_stack and setup of the initial identity
> > mapping of the kernel as well as the initial page protection are not hot path
> > as far as I can tell. Any other places?
> 
> Well, mostly kernel_map_pages() which is used in page allocation hotpaths.

We cannot just convert it because setup_arch() is called before
jump_label_init(). We can do it by introducing _early variant but
I'm not sure it's worth doing. I think that just make it unlikely
works well. Recently, I tested a micro benchmark on slab alloc/free
on CONFIG_SLUB with applying static branch for debug option which
currently is using unlikely branch and can't find any noticeble
performance difference.

Thanks.

Re: [PATCH/RFC 3/3] s390: query dynamic DEBUG_PAGEALLOC setting

[Joonsoo Kim]

On Wed, Jan 27, 2016 at 02:03:36PM +0100, Vlastimil Babka wrote:
> On 01/27/2016 01:47 PM, Christian Borntraeger wrote:
> > On 01/27/2016 01:41 PM, Vlastimil Babka wrote:
> >> On 01/27/2016 01:59 AM, Joonsoo Kim wrote:
> >> 
> >> I think it might be worth also to convert debug_pagealloc_enabled() to be based
> >> on static key, like I did for page_owner [1]. That should help make it possible
> >> to have virtually no overhead when compiling kernel with CONFIG_DEBUG_PAGEALLOC
> >> without enabling it boot-time. I assume it's one of the goals here?
> > 
> > We could do something like that but dump_stack and setup of the initial identity
> > mapping of the kernel as well as the initial page protection are not hot path
> > as far as I can tell. Any other places?
> 
> Well, mostly kernel_map_pages() which is used in page allocation hotpaths.

We cannot just convert it because setup_arch() is called before
jump_label_init(). We can do it by introducing _early variant but
I'm not sure it's worth doing. I think that just make it unlikely
works well. Recently, I tested a micro benchmark on slab alloc/free
on CONFIG_SLUB with applying static branch for debug option which
currently is using unlikely branch and can't find any noticeble
performance difference.

Thanks.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>