2011-06-07 11:18:48

by Steven Rostedt

[permalink] [raw]
Subject: [PATCH] slob/lockdep: Fix gfp flags passed to lockdep


Doing a ktest.pl randconfig, I stumbled across the following bug
on boot up:

------------[ cut here ]------------
WARNING: at /home/rostedt/work/autotest/nobackup/linux-test.git/kernel/lockdep.c:2649 lockdep_trace_alloc+0xed/0x100()
Hardware name:
Modules linked in:
Pid: 0, comm: swapper Not tainted 3.0.0-rc1-test-00054-g1d68b67 #1
Call Trace:
[<ffffffff810626ad>] warn_slowpath_common+0xad/0xf0
[<ffffffff8106270a>] warn_slowpath_null+0x1a/0x20
[<ffffffff810b537d>] lockdep_trace_alloc+0xed/0x100
[<ffffffff81182fb0>] __kmalloc_node+0x30/0x2f0
[<ffffffff81153eda>] pcpu_mem_alloc+0x13a/0x180
[<ffffffff82be022c>] percpu_init_late+0x48/0xc2
[<ffffffff82bd630c>] ? mem_init+0xd8/0xe3
[<ffffffff82bbcc73>] start_kernel+0x1c2/0x449
[<ffffffff82bbc35c>] x86_64_start_reservations+0x163/0x167
[<ffffffff82bbc493>] x86_64_start_kernel+0x133/0x142^M
---[ end trace a7919e7f17c0a725 ]---

Then I ran a ktest.pl config_bisect and it came up with this config
as the problem:

CONFIG_SLOB

Looking at what is different between SLOB and SLAB and SLUB, I found
that the gfp flags are masked against gfp_allowed_mask in
SLAB and SLUB, but not SLOB.

On boot up, interrupts are disabled and lockdep will warn if some flags
are set in gfp and interrupts are disabled. But these flags are masked
off with the gfp_allowed_mask during boot. Because SLOB does not
mask the flags against gfp_allowed_mask it triggers the warn on.

Adding this mask fixes the bug. I also found that kmem_cache_alloc_node()
was missing both the mask and the lockdep check, and that was added too.

Cc: Matt Mackall <[email protected]>
Cc: Paul Mundt <[email protected]>
Cc: Nick Piggin <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>

diff --git a/mm/slob.c b/mm/slob.c
index 46e0aee..0ae8818 100644
--- a/mm/slob.c
+++ b/mm/slob.c
@@ -482,6 +482,8 @@ void *__kmalloc_node(size_t size, gfp_t gfp, int node)
int align = max(ARCH_KMALLOC_MINALIGN, ARCH_SLAB_MINALIGN);
void *ret;

+ gfp &= gfp_allowed_mask;
+
lockdep_trace_alloc(gfp);

if (size < PAGE_SIZE - align) {
@@ -608,6 +610,10 @@ void *kmem_cache_alloc_node(struct kmem_cache *c, gfp_t flags, int node)
{
void *b;

+ flags &= gfp_allowed_mask;
+
+ lockdep_trace_alloc(flags);
+
if (c->size < PAGE_SIZE) {
b = slob_alloc(c->size, flags, c->align, node);
trace_kmem_cache_alloc_node(_RET_IP_, b, c->size,


2011-06-07 11:54:16

by Pekka Enberg

[permalink] [raw]
Subject: Re: [PATCH] slob/lockdep: Fix gfp flags passed to lockdep

On Tue, Jun 7, 2011 at 2:18 PM, Steven Rostedt <[email protected]> wrote:
>
> Doing a ktest.pl randconfig, I stumbled across the following bug
> on boot up:
>
> ------------[ cut here ]------------
> WARNING: at /home/rostedt/work/autotest/nobackup/linux-test.git/kernel/lockdep.c:2649 lockdep_trace_alloc+0xed/0x100()
> Hardware name:
> Modules linked in:
> Pid: 0, comm: swapper Not tainted 3.0.0-rc1-test-00054-g1d68b67 #1
> Call Trace:
> ?[<ffffffff810626ad>] warn_slowpath_common+0xad/0xf0
> ?[<ffffffff8106270a>] warn_slowpath_null+0x1a/0x20
> ?[<ffffffff810b537d>] lockdep_trace_alloc+0xed/0x100
> ?[<ffffffff81182fb0>] __kmalloc_node+0x30/0x2f0
> ?[<ffffffff81153eda>] pcpu_mem_alloc+0x13a/0x180
> ?[<ffffffff82be022c>] percpu_init_late+0x48/0xc2
> ?[<ffffffff82bd630c>] ? mem_init+0xd8/0xe3
> ?[<ffffffff82bbcc73>] start_kernel+0x1c2/0x449
> ?[<ffffffff82bbc35c>] x86_64_start_reservations+0x163/0x167
> ?[<ffffffff82bbc493>] x86_64_start_kernel+0x133/0x142^M
> ---[ end trace a7919e7f17c0a725 ]---
>
> Then I ran a ktest.pl config_bisect and it came up with this config
> as the problem:
>
> ?CONFIG_SLOB
>
> Looking at what is different between SLOB and SLAB and SLUB, I found
> that the gfp flags are masked against gfp_allowed_mask in
> SLAB and SLUB, but not SLOB.
>
> On boot up, interrupts are disabled and lockdep will warn if some flags
> are set in gfp and interrupts are disabled. But these flags are masked
> off with the gfp_allowed_mask during boot. Because SLOB does not
> mask the flags against gfp_allowed_mask it triggers the warn on.
>
> Adding this mask fixes the bug. I also found that kmem_cache_alloc_node()
> was missing both the mask and the lockdep check, and that was added too.
>
> Cc: Matt Mackall <[email protected]>
> Cc: Paul Mundt <[email protected]>
> Cc: Nick Piggin <[email protected]>
> Signed-off-by: Steven Rostedt <[email protected]>
>
> diff --git a/mm/slob.c b/mm/slob.c
> index 46e0aee..0ae8818 100644
> --- a/mm/slob.c
> +++ b/mm/slob.c
> @@ -482,6 +482,8 @@ void *__kmalloc_node(size_t size, gfp_t gfp, int node)
> ? ? ? ?int align = max(ARCH_KMALLOC_MINALIGN, ARCH_SLAB_MINALIGN);
> ? ? ? ?void *ret;
>
> + ? ? ? gfp &= gfp_allowed_mask;
> +
> ? ? ? ?lockdep_trace_alloc(gfp);
>
> ? ? ? ?if (size < PAGE_SIZE - align) {
> @@ -608,6 +610,10 @@ void *kmem_cache_alloc_node(struct kmem_cache *c, gfp_t flags, int node)
> ?{
> ? ? ? ?void *b;
>
> + ? ? ? flags &= gfp_allowed_mask;
> +
> + ? ? ? lockdep_trace_alloc(flags);
> +
> ? ? ? ?if (c->size < PAGE_SIZE) {
> ? ? ? ? ? ? ? ?b = slob_alloc(c->size, flags, c->align, node);
> ? ? ? ? ? ? ? ?trace_kmem_cache_alloc_node(_RET_IP_, b, c->size,

Matt, any objections to merging this?

2011-06-07 12:49:17

by Matt Mackall

[permalink] [raw]
Subject: Re: [PATCH] slob/lockdep: Fix gfp flags passed to lockdep

On Tue, 2011-06-07 at 14:54 +0300, Pekka Enberg wrote:
> On Tue, Jun 7, 2011 at 2:18 PM, Steven Rostedt <[email protected]> wrote:
> >
> > Doing a ktest.pl randconfig, I stumbled across the following bug
> > on boot up:
> >
> > ------------[ cut here ]------------
> > WARNING: at /home/rostedt/work/autotest/nobackup/linux-test.git/kernel/lockdep.c:2649 lockdep_trace_alloc+0xed/0x100()
> > Hardware name:
> > Modules linked in:
> > Pid: 0, comm: swapper Not tainted 3.0.0-rc1-test-00054-g1d68b67 #1
> > Call Trace:
> > [<ffffffff810626ad>] warn_slowpath_common+0xad/0xf0
> > [<ffffffff8106270a>] warn_slowpath_null+0x1a/0x20
> > [<ffffffff810b537d>] lockdep_trace_alloc+0xed/0x100
> > [<ffffffff81182fb0>] __kmalloc_node+0x30/0x2f0
> > [<ffffffff81153eda>] pcpu_mem_alloc+0x13a/0x180
> > [<ffffffff82be022c>] percpu_init_late+0x48/0xc2
> > [<ffffffff82bd630c>] ? mem_init+0xd8/0xe3
> > [<ffffffff82bbcc73>] start_kernel+0x1c2/0x449
> > [<ffffffff82bbc35c>] x86_64_start_reservations+0x163/0x167
> > [<ffffffff82bbc493>] x86_64_start_kernel+0x133/0x142^M
> > ---[ end trace a7919e7f17c0a725 ]---
> >
> > Then I ran a ktest.pl config_bisect and it came up with this config
> > as the problem:
> >
> > CONFIG_SLOB
> >
> > Looking at what is different between SLOB and SLAB and SLUB, I found
> > that the gfp flags are masked against gfp_allowed_mask in
> > SLAB and SLUB, but not SLOB.
> >
> > On boot up, interrupts are disabled and lockdep will warn if some flags
> > are set in gfp and interrupts are disabled. But these flags are masked
> > off with the gfp_allowed_mask during boot. Because SLOB does not
> > mask the flags against gfp_allowed_mask it triggers the warn on.
> >
> > Adding this mask fixes the bug. I also found that kmem_cache_alloc_node()
> > was missing both the mask and the lockdep check, and that was added too.
> >
> > Cc: Matt Mackall <[email protected]>
> > Cc: Paul Mundt <[email protected]>
> > Cc: Nick Piggin <[email protected]>
> > Signed-off-by: Steven Rostedt <[email protected]>
> >
> > diff --git a/mm/slob.c b/mm/slob.c
> > index 46e0aee..0ae8818 100644
> > --- a/mm/slob.c
> > +++ b/mm/slob.c
> > @@ -482,6 +482,8 @@ void *__kmalloc_node(size_t size, gfp_t gfp, int node)
> > int align = max(ARCH_KMALLOC_MINALIGN, ARCH_SLAB_MINALIGN);
> > void *ret;
> >
> > + gfp &= gfp_allowed_mask;
> > +
> > lockdep_trace_alloc(gfp);
> >
> > if (size < PAGE_SIZE - align) {
> > @@ -608,6 +610,10 @@ void *kmem_cache_alloc_node(struct kmem_cache *c, gfp_t flags, int node)
> > {
> > void *b;
> >
> > + flags &= gfp_allowed_mask;
> > +
> > + lockdep_trace_alloc(flags);
> > +
> > if (c->size < PAGE_SIZE) {
> > b = slob_alloc(c->size, flags, c->align, node);
> > trace_kmem_cache_alloc_node(_RET_IP_, b, c->size,
>
> Matt, any objections to merging this?

Looks good.

Acked-by: Matt Mackall <[email protected]>


--
Mathematics is the supreme nostalgia of our time.