diff options
author | Nick Piggin <npiggin@suse.de> | 2009-01-21 02:12:39 -0500 |
---|---|---|
committer | Ingo Molnar <mingo@elte.hu> | 2009-02-14 17:27:49 -0500 |
commit | cf40bd16fdad42c053040bcd3988f5fdedbb6c57 (patch) | |
tree | d97ab25726981712be806d77650b7f65167cfc88 /mm/page_alloc.c | |
parent | 6f2b9b9a9d750a9175dc79c74bfed5add840983c (diff) |
lockdep: annotate reclaim context (__GFP_NOFS)
Here is another version, with the incremental patch rolled up, and
added reclaim context annotation to kswapd, and allocation tracing
to slab allocators (which may only ever reach the page allocator
in rare cases, so it is good to put annotations here too).
Haven't tested this version as such, but it should be getting closer
to merge worthy ;)
--
After noticing some code in mm/filemap.c accidentally perform a __GFP_FS
allocation when it should not have been, I thought it might be a good idea to
try to catch this kind of thing with lockdep.
I coded up a little idea that seems to work. Unfortunately the system has to
actually be in __GFP_FS page reclaim, then take the lock, before it will mark
it. But at least that might still be some orders of magnitude more common
(and more debuggable) than an actual deadlock condition, so we have some
improvement I hope (the concept is no less complete than discovery of a lock's
interrupt contexts).
I guess we could even do the same thing with __GFP_IO (normal reclaim), and
even GFP_NOIO locks too... but filesystems will have the most locks and fiddly
code paths, so let's start there and see how it goes.
It *seems* to work. I did a quick test.
=================================
[ INFO: inconsistent lock state ]
2.6.28-rc6-00007-ged31348-dirty #26
---------------------------------
inconsistent {in-reclaim-W} -> {ov-reclaim-W} usage.
modprobe/8526 [HC0[0]:SC0[0]:HE1:SE1] takes:
(testlock){--..}, at: [<ffffffffa0020055>] brd_init+0x55/0x216 [brd]
{in-reclaim-W} state was registered at:
[<ffffffff80267bdb>] __lock_acquire+0x75b/0x1a60
[<ffffffff80268f71>] lock_acquire+0x91/0xc0
[<ffffffff8070f0e1>] mutex_lock_nested+0xb1/0x310
[<ffffffffa002002b>] brd_init+0x2b/0x216 [brd]
[<ffffffff8020903b>] _stext+0x3b/0x170
[<ffffffff80272ebf>] sys_init_module+0xaf/0x1e0
[<ffffffff8020c3fb>] system_call_fastpath+0x16/0x1b
[<ffffffffffffffff>] 0xffffffffffffffff
irq event stamp: 3929
hardirqs last enabled at (3929): [<ffffffff8070f2b5>] mutex_lock_nested+0x285/0x310
hardirqs last disabled at (3928): [<ffffffff8070f089>] mutex_lock_nested+0x59/0x310
softirqs last enabled at (3732): [<ffffffff8061f623>] sk_filter+0x83/0xe0
softirqs last disabled at (3730): [<ffffffff8061f5b6>] sk_filter+0x16/0xe0
other info that might help us debug this:
1 lock held by modprobe/8526:
#0: (testlock){--..}, at: [<ffffffffa0020055>] brd_init+0x55/0x216 [brd]
stack backtrace:
Pid: 8526, comm: modprobe Not tainted 2.6.28-rc6-00007-ged31348-dirty #26
Call Trace:
[<ffffffff80265483>] print_usage_bug+0x193/0x1d0
[<ffffffff80266530>] mark_lock+0xaf0/0xca0
[<ffffffff80266735>] mark_held_locks+0x55/0xc0
[<ffffffffa0020000>] ? brd_init+0x0/0x216 [brd]
[<ffffffff802667ca>] trace_reclaim_fs+0x2a/0x60
[<ffffffff80285005>] __alloc_pages_internal+0x475/0x580
[<ffffffff8070f29e>] ? mutex_lock_nested+0x26e/0x310
[<ffffffffa0020000>] ? brd_init+0x0/0x216 [brd]
[<ffffffffa002006a>] brd_init+0x6a/0x216 [brd]
[<ffffffffa0020000>] ? brd_init+0x0/0x216 [brd]
[<ffffffff8020903b>] _stext+0x3b/0x170
[<ffffffff8070f8b9>] ? mutex_unlock+0x9/0x10
[<ffffffff8070f83d>] ? __mutex_unlock_slowpath+0x10d/0x180
[<ffffffff802669ec>] ? trace_hardirqs_on_caller+0x12c/0x190
[<ffffffff80272ebf>] sys_init_module+0xaf/0x1e0
[<ffffffff8020c3fb>] system_call_fastpath+0x16/0x1b
Signed-off-by: Nick Piggin <npiggin@suse.de>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
Diffstat (limited to 'mm/page_alloc.c')
-rw-r--r-- | mm/page_alloc.c | 5 |
1 files changed, 5 insertions, 0 deletions
diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 5675b3073854..22b15a4cde8a 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c | |||
@@ -1479,6 +1479,8 @@ __alloc_pages_internal(gfp_t gfp_mask, unsigned int order, | |||
1479 | unsigned long did_some_progress; | 1479 | unsigned long did_some_progress; |
1480 | unsigned long pages_reclaimed = 0; | 1480 | unsigned long pages_reclaimed = 0; |
1481 | 1481 | ||
1482 | lockdep_trace_alloc(gfp_mask); | ||
1483 | |||
1482 | might_sleep_if(wait); | 1484 | might_sleep_if(wait); |
1483 | 1485 | ||
1484 | if (should_fail_alloc_page(gfp_mask, order)) | 1486 | if (should_fail_alloc_page(gfp_mask, order)) |
@@ -1578,12 +1580,15 @@ nofail_alloc: | |||
1578 | */ | 1580 | */ |
1579 | cpuset_update_task_memory_state(); | 1581 | cpuset_update_task_memory_state(); |
1580 | p->flags |= PF_MEMALLOC; | 1582 | p->flags |= PF_MEMALLOC; |
1583 | |||
1584 | lockdep_set_current_reclaim_state(gfp_mask); | ||
1581 | reclaim_state.reclaimed_slab = 0; | 1585 | reclaim_state.reclaimed_slab = 0; |
1582 | p->reclaim_state = &reclaim_state; | 1586 | p->reclaim_state = &reclaim_state; |
1583 | 1587 | ||
1584 | did_some_progress = try_to_free_pages(zonelist, order, gfp_mask); | 1588 | did_some_progress = try_to_free_pages(zonelist, order, gfp_mask); |
1585 | 1589 | ||
1586 | p->reclaim_state = NULL; | 1590 | p->reclaim_state = NULL; |
1591 | lockdep_clear_current_reclaim_state(); | ||
1587 | p->flags &= ~PF_MEMALLOC; | 1592 | p->flags &= ~PF_MEMALLOC; |
1588 | 1593 | ||
1589 | cond_resched(); | 1594 | cond_resched(); |