Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1317661 > unrolled thread

[PATCH/RFC 0/3] Optimize CONFIG_DEBUG_PAGEALLOC

Started byChristian Borntraeger <borntraeger@de.ibm.com>
First post2016-01-26 10:20 +0100
Last post2016-01-27 14:10 +0100
Articles 13 — 6 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH/RFC 0/3] Optimize CONFIG_DEBUG_PAGEALLOC Christian Borntraeger <borntraeger@de.ibm.com> - 2016-01-26 10:20 +0100
    [PATCH/RFC 3/3] s390: query dynamic DEBUG_PAGEALLOC setting Christian Borntraeger <borntraeger@de.ibm.com> - 2016-01-26 10:20 +0100
      Re: [PATCH/RFC 3/3] s390: query dynamic DEBUG_PAGEALLOC setting Heiko Carstens <heiko.carstens@de.ibm.com> - 2016-01-26 19:20 +0100
        Re: [PATCH/RFC 3/3] s390: query dynamic DEBUG_PAGEALLOC setting Thomas Gleixner <tglx@linutronix.de> - 2016-01-26 21:50 +0100
        Re: [PATCH/RFC 3/3] s390: query dynamic DEBUG_PAGEALLOC setting David Rientjes <rientjes@google.com> - 2016-01-27 00:30 +0100
          Re: [PATCH/RFC 3/3] s390: query dynamic DEBUG_PAGEALLOC setting Joonsoo Kim <iamjoonsoo.kim@lge.com> - 2016-01-27 01:20 +0100
            Re: [PATCH/RFC 3/3] s390: query dynamic DEBUG_PAGEALLOC setting David Rientjes <rientjes@google.com> - 2016-01-27 01:40 +0100
              Re: [PATCH/RFC 3/3] s390: query dynamic DEBUG_PAGEALLOC setting Joonsoo Kim <iamjoonsoo.kim@lge.com> - 2016-01-27 02:00 +0100
                Re: [PATCH/RFC 3/3] s390: query dynamic DEBUG_PAGEALLOC setting Christian Borntraeger <borntraeger@de.ibm.com> - 2016-01-27 09:00 +0100
                Re: [PATCH/RFC 3/3] s390: query dynamic DEBUG_PAGEALLOC setting Vlastimil Babka <vbabka@suse.cz> - 2016-01-27 13:50 +0100
                  Re: [PATCH/RFC 3/3] s390: query dynamic DEBUG_PAGEALLOC setting Christian Borntraeger <borntraeger@de.ibm.com> - 2016-01-27 13:50 +0100
                    Re: [PATCH/RFC 3/3] s390: query dynamic DEBUG_PAGEALLOC setting Christian Borntraeger <borntraeger@de.ibm.com> - 2016-01-27 14:10 +0100
                    Re: [PATCH/RFC 3/3] s390: query dynamic DEBUG_PAGEALLOC setting Vlastimil Babka <vbabka@suse.cz> - 2016-01-27 14:10 +0100

#1317661 — [PATCH/RFC 0/3] Optimize CONFIG_DEBUG_PAGEALLOC

FromChristian Borntraeger <borntraeger@de.ibm.com>
Date2016-01-26 10:20 +0100
Subject[PATCH/RFC 0/3] Optimize CONFIG_DEBUG_PAGEALLOC
Message-ID<qV7X4-1ZG-13@gated-at.bofh.it>
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

[toc] | [next] | [standalone]


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

FromChristian Borntraeger <borntraeger@de.ibm.com>
Date2016-01-26 10:20 +0100
Subject[PATCH/RFC 3/3] s390: query dynamic DEBUG_PAGEALLOC setting
Message-ID<qV7X5-1ZG-33@gated-at.bofh.it>
In reply to#1317661
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

[toc] | [prev] | [next] | [standalone]


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

FromHeiko Carstens <heiko.carstens@de.ibm.com>
Date2016-01-26 19:20 +0100
SubjectRe: [PATCH/RFC 3/3] s390: query dynamic DEBUG_PAGEALLOC setting
Message-ID<qVgnE-7Zo-15@gated-at.bofh.it>
In reply to#1317662
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.

[toc] | [prev] | [next] | [standalone]


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

FromThomas Gleixner <tglx@linutronix.de>
Date2016-01-26 21:50 +0100
SubjectRe: [PATCH/RFC 3/3] s390: query dynamic DEBUG_PAGEALLOC setting
Message-ID<qViIO-1ad-17@gated-at.bofh.it>
In reply to#1318218
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

[toc] | [prev] | [next] | [standalone]


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

FromDavid Rientjes <rientjes@google.com>
Date2016-01-27 00:30 +0100
SubjectRe: [PATCH/RFC 3/3] s390: query dynamic DEBUG_PAGEALLOC setting
Message-ID<qVldF-33s-25@gated-at.bofh.it>
In reply to#1318218
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.

[toc] | [prev] | [next] | [standalone]


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

FromJoonsoo Kim <iamjoonsoo.kim@lge.com>
Date2016-01-27 01:20 +0100
SubjectRe: [PATCH/RFC 3/3] s390: query dynamic DEBUG_PAGEALLOC setting
Message-ID<qVm01-3EK-5@gated-at.bofh.it>
In reply to#1318468
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.

[toc] | [prev] | [next] | [standalone]


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

FromDavid Rientjes <rientjes@google.com>
Date2016-01-27 01:40 +0100
SubjectRe: [PATCH/RFC 3/3] s390: query dynamic DEBUG_PAGEALLOC setting
Message-ID<qVmjn-3ND-17@gated-at.bofh.it>
In reply to#1318492
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.

[toc] | [prev] | [next] | [standalone]


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

FromJoonsoo Kim <iamjoonsoo.kim@lge.com>
Date2016-01-27 02:00 +0100
SubjectRe: [PATCH/RFC 3/3] s390: query dynamic DEBUG_PAGEALLOC setting
Message-ID<qVmCJ-3UC-1@gated-at.bofh.it>
In reply to#1318502
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.

[toc] | [prev] | [next] | [standalone]


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

FromChristian Borntraeger <borntraeger@de.ibm.com>
Date2016-01-27 09:00 +0100
SubjectRe: [PATCH/RFC 3/3] s390: query dynamic DEBUG_PAGEALLOC setting
Message-ID<qVtbd-cl-9@gated-at.bofh.it>
In reply to#1318513
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

[toc] | [prev] | [next] | [standalone]


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

FromVlastimil Babka <vbabka@suse.cz>
Date2016-01-27 13:50 +0100
SubjectRe: [PATCH/RFC 3/3] s390: query dynamic DEBUG_PAGEALLOC setting
Message-ID<qVxHQ-3xq-7@gated-at.bofh.it>
In reply to#1318513
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>
> 

[toc] | [prev] | [next] | [standalone]


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

FromChristian Borntraeger <borntraeger@de.ibm.com>
Date2016-01-27 13:50 +0100
SubjectRe: [PATCH/RFC 3/3] s390: query dynamic DEBUG_PAGEALLOC setting
Message-ID<qVxHR-3xq-19@gated-at.bofh.it>
In reply to#1318923
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

[toc] | [prev] | [next] | [standalone]


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

FromChristian Borntraeger <borntraeger@de.ibm.com>
Date2016-01-27 14:10 +0100
SubjectRe: [PATCH/RFC 3/3] s390: query dynamic DEBUG_PAGEALLOC setting
Message-ID<qVy1c-3W1-13@gated-at.bofh.it>
In reply to#1318926
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.

[toc] | [prev] | [next] | [standalone]


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

FromVlastimil Babka <vbabka@suse.cz>
Date2016-01-27 14:10 +0100
SubjectRe: [PATCH/RFC 3/3] s390: query dynamic DEBUG_PAGEALLOC setting
Message-ID<qVy1c-3W1-15@gated-at.bofh.it>
In reply to#1318926
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>
> 

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web