Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1317661 > unrolled thread
| Started by | Christian Borntraeger <borntraeger@de.ibm.com> |
|---|---|
| First post | 2016-01-26 10:20 +0100 |
| Last post | 2016-01-27 14:10 +0100 |
| Articles | 13 — 6 participants |
Back to article view | Back to linux.kernel
[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
| From | Christian Borntraeger <borntraeger@de.ibm.com> |
|---|---|
| Date | 2016-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]
| From | Christian Borntraeger <borntraeger@de.ibm.com> |
|---|---|
| Date | 2016-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]
| From | Heiko Carstens <heiko.carstens@de.ibm.com> |
|---|---|
| Date | 2016-01-26 19:20 +0100 |
| Subject | Re: [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]
| From | Thomas Gleixner <tglx@linutronix.de> |
|---|---|
| Date | 2016-01-26 21:50 +0100 |
| Subject | Re: [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]
| From | David Rientjes <rientjes@google.com> |
|---|---|
| Date | 2016-01-27 00:30 +0100 |
| Subject | Re: [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]
| From | Joonsoo Kim <iamjoonsoo.kim@lge.com> |
|---|---|
| Date | 2016-01-27 01:20 +0100 |
| Subject | Re: [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]
| From | David Rientjes <rientjes@google.com> |
|---|---|
| Date | 2016-01-27 01:40 +0100 |
| Subject | Re: [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]
| From | Joonsoo Kim <iamjoonsoo.kim@lge.com> |
|---|---|
| Date | 2016-01-27 02:00 +0100 |
| Subject | Re: [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]
| From | Christian Borntraeger <borntraeger@de.ibm.com> |
|---|---|
| Date | 2016-01-27 09:00 +0100 |
| Subject | Re: [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]
| From | Vlastimil Babka <vbabka@suse.cz> |
|---|---|
| Date | 2016-01-27 13:50 +0100 |
| Subject | Re: [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]
| From | Christian Borntraeger <borntraeger@de.ibm.com> |
|---|---|
| Date | 2016-01-27 13:50 +0100 |
| Subject | Re: [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]
| From | Christian Borntraeger <borntraeger@de.ibm.com> |
|---|---|
| Date | 2016-01-27 14:10 +0100 |
| Subject | Re: [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]
| From | Vlastimil Babka <vbabka@suse.cz> |
|---|---|
| Date | 2016-01-27 14:10 +0100 |
| Subject | Re: [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