sleeping function warning from __put_anon_vma
Subsystems (2)
BUG: sleeping function called from invalid context at kernel/locking/rwsem.c:47 in_atomic(): 0, irqs_disabled(): 0, pid: 5787, name: trinity-c27 Preemption disabled at:[<ffffffff990acc7e>] vtime_account_system+0x1e/0x50 CPU: 0 PID: 5787 Comm: trinity-c27 Not tainted 3.15.0-rc7+ #219 ffffffff99a47203 0000000099b50bef ffff880239f138c8 ffffffff99739dfb 0000000000000000 ffff880239f138f0 ffffffff990a026c ffff8801078b5458 ffff8801078b5450 ffffea00044be980 ffff880239f13908 ffffffff99741c30 Call Trace: [<ffffffff99739dfb>] dump_stack+0x4e/0x7a [<ffffffff990a026c>] __might_sleep+0x11c/0x1b0 [<ffffffff99741c30>] down_write+0x20/0x40 [<ffffffff9919337d>] __put_anon_vma+0x3d/0xc0 [<ffffffff99193998>] page_get_anon_vma+0x68/0xb0 [<ffffffff991b97e9>] migrate_pages+0x449/0x880 [<ffffffff9917dc00>] ? isolate_freepages_block+0x360/0x360 [<ffffffff9917ec8a>] compact_zone+0x38a/0x580 [<ffffffff9917ef29>] compact_zone_order+0xa9/0x130 [<ffffffff9917f329>] try_to_compact_pages+0xe9/0x140 [<ffffffff991616da>] __alloc_pages_direct_compact+0x7a/0x250 [<ffffffff99161fbb>] __alloc_pages_nodemask+0x70b/0xbb0 [<ffffffff991a9c3f>] alloc_pages_vma+0xaf/0x1c0 [<ffffffff991bdc8d>] do_huge_pmd_anonymous_page+0xed/0x3d0 [<ffffffff991871b4>] handle_mm_fault+0x1b4/0xc50 [<ffffffff9974358d>] ? retint_restore_args+0xe/0xe [<ffffffff99746939>] __do_page_fault+0x1c9/0x630 [<ffffffff99118acb>] ? __acct_update_integrals+0x8b/0x120 [<ffffffff9974725b>] ? preempt_count_sub+0xab/0x100 [<ffffffff99746dbe>] do_page_fault+0x1e/0x70 [<ffffffff997437f2>] page_fault+0x22/0x30
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 21 lines · Dave Jones
BUG: sleeping function called from invalid context at kernel/locking/rwsem.c:47 in_atomic(): 0, irqs_disabled(): 0, pid: 5787, name: trinity-c27 Preemption disabled at:[<ffffffff990acc7e>] vtime_account_system+0x1e/0x50 CPU: 0 PID: 5787 Comm: trinity-c27 Not tainted 3.15.0-rc7+ #219 ffffffff99a47203 0000000099b50bef ffff880239f138c8 ffffffff99739dfb 0000000000000000 ffff880239f138f0 ffffffff990a026c ffff8801078b5458 ffff8801078b5450 ffffea00044be980 ffff880239f13908 ffffffff99741c30 Call Trace: [<ffffffff99739dfb>] dump_stack+0x4e/0x7a [<ffffffff990a026c>] __might_sleep+0x11c/0x1b0 [<ffffffff99741c30>] down_write+0x20/0x40 [<ffffffff9919337d>] __put_anon_vma+0x3d/0xc0 [<ffffffff99193998>] page_get_anon_vma+0x68/0xb0 [<ffffffff991b97e9>] migrate_pages+0x449/0x880 [<ffffffff9917dc00>] ? isolate_freepages_block+0x360/0x360 [<ffffffff9917ec8a>] compact_zone+0x38a/0x580 [<ffffffff9917ef29>] compact_zone_order+0xa9/0x130 [<ffffffff9917f329>] try_to_compact_pages+0xe9/0x140 [<ffffffff991616da>] __alloc_pages_direct_compact+0x7a/0x250 [<ffffffff99161fbb>] __alloc_pages_nodemask+0x70b/0xbb0 [<ffffffff991a9c3f>] alloc_pages_vma+0xaf/0x1c0
Quoted 8 lines
[<ffffffff991bdc8d>] do_huge_pmd_anonymous_page+0xed/0x3d0 [<ffffffff991871b4>] handle_mm_fault+0x1b4/0xc50 [<ffffffff9974358d>] ? retint_restore_args+0xe/0xe [<ffffffff99746939>] __do_page_fault+0x1c9/0x630 [<ffffffff99118acb>] ? __acct_update_integrals+0x8b/0x120 [<ffffffff9974725b>] ? preempt_count_sub+0xab/0x100 [<ffffffff99746dbe>] do_page_fault+0x1e/0x70 [<ffffffff997437f2>] page_fault+0x22/0x30
Well caught again. This one doesn't seem to have been plaguing users, but nice to find and fix it. I don't think there are any gotchas to the patch below (in each case we've done a good atomic_inc_not_zero, so the anon_vma cannot be surprisingly freed when we rcu_read_unlock). But even so, I don't think it should go into a tree without Ack from Peter - in looking back there, he might notice something else has changed, or be inspired to fix it in a more satisfying way.
Hugh
[PATCH] mm: fix sleeping function warning from __put_anon_vma
Trinity reports BUG:
sleeping function called from invalid context at kernel/locking/rwsem.c:47 in_atomic(): 0, irqs_disabled(): 0, pid: 5787, name: trinity-c27 __might_sleep < down_write < __put_anon_vma < page_get_anon_vma < migrate_pages < compact_zone < compact_zone_order < try_to_compact_pages ..
Right, since conversion to mutex then rwsem, we should not put_anon_vma() from inside an rcu_read_lock()ed section: fix the two places that did so.
Needs-Ack-from: Peter Zijlstra <[email protected]>
---
mm/rmap.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
mm/rmap.c +6 -2
@@ -426,12 +426,14 @@ struct anon_vma *page_get_anon_vma(struc* above cannot corrupt). */ if (!page_mapped (page)) { +rcu_read_unlock ();put_anon_vma (anon_vma); anon_vma =NULL ; + goto outer; } out:rcu_read_unlock (); - +outer:return anon_vma; }
@@ -477,9 +479,10 @@ struct anon_vma *page_lock_anon_vma_read }if (!page_mapped (page)) { +rcu_read_unlock ();put_anon_vma (anon_vma); anon_vma =NULL ; - goto out; + goto outer; }/* we pinned the anon_vma, its safe to sleep */
@@ -501,6 +504,7 @@ struct anon_vma *page_lock_anon_vma_read out:rcu_read_unlock (); +outer:return anon_vma; }
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 5 lines · Hugh Dickins
Quoted 3 lines · Dave Jones
BUG: sleeping function called from invalid context at kernel/locking/rwsem.c:47
in_atomic(): 0,irqs_disabled(): 0, pid: 5787, name: trinity-c27 Preemption disabled at:[<ffffffff990acc7e>]vtime_account_system+0x1e/0x50
Just wondering, since I'm not familiar with this kind of bug, is the line above bogus or what does it mean? I don't see how the stack trace or the fix patch is related to vtime_account_system?
Thanks,
Vlastimil
Quoted 97 lines
Quoted 18 lines
CPU: 0 PID: 5787 Comm: trinity-c27 Not tainted 3.15.0-rc7+ #219 ffffffff99a47203 0000000099b50bef ffff880239f138c8 ffffffff99739dfb 0000000000000000 ffff880239f138f0 ffffffff990a026c ffff8801078b5458 ffff8801078b5450 ffffea00044be980 ffff880239f13908 ffffffff99741c30 Call Trace: [<ffffffff99739dfb>] dump_stack+0x4e/0x7a [<ffffffff990a026c>] __might_sleep+0x11c/0x1b0 [<ffffffff99741c30>] down_write+0x20/0x40 [<ffffffff9919337d>] __put_anon_vma+0x3d/0xc0 [<ffffffff99193998>] page_get_anon_vma+0x68/0xb0 [<ffffffff991b97e9>] migrate_pages+0x449/0x880 [<ffffffff9917dc00>] ? isolate_freepages_block+0x360/0x360 [<ffffffff9917ec8a>] compact_zone+0x38a/0x580 [<ffffffff9917ef29>] compact_zone_order+0xa9/0x130 [<ffffffff9917f329>] try_to_compact_pages+0xe9/0x140 [<ffffffff991616da>] __alloc_pages_direct_compact+0x7a/0x250 [<ffffffff99161fbb>] __alloc_pages_nodemask+0x70b/0xbb0 [<ffffffff991a9c3f>] alloc_pages_vma+0xaf/0x1c0Quoted 8 lines
[<ffffffff991bdc8d>] do_huge_pmd_anonymous_page+0xed/0x3d0 [<ffffffff991871b4>] handle_mm_fault+0x1b4/0xc50 [<ffffffff9974358d>] ? retint_restore_args+0xe/0xe [<ffffffff99746939>] __do_page_fault+0x1c9/0x630 [<ffffffff99118acb>] ? __acct_update_integrals+0x8b/0x120 [<ffffffff9974725b>] ? preempt_count_sub+0xab/0x100 [<ffffffff99746dbe>] do_page_fault+0x1e/0x70 [<ffffffff997437f2>] page_fault+0x22/0x30Well caught again. This one doesn't seem to have been plaguing users, but nice to find and fix it. I don't think there are any gotchas to the patch below (in each case we've done a good
atomic_inc_not_zero, so theanon_vmacannot be surprisingly freed when wercu_read_unlock). But even so, I don't think it should go into a tree without Ack from Peter - in looking back there, he might notice something else has changed, or be inspired to fix it in a more satisfying way.Hugh
[PATCH] mm: fix sleeping function warning from
__put_anon_vmaTrinity reports BUG:
sleeping function called from invalid context atkernel/locking/rwsem.c:47in_atomic(): 0,irqs_disabled(): 0, pid: 5787, name: trinity-c27__might_sleep<down_write<__put_anon_vma<page_get_anon_vma<migrate_pages<compact_zone<compact_zone_order<try_to_compact_pages..Right, since conversion to mutex then rwsem, we should not
put_anon_vma()from inside an rcu_read_lock()ed section: fix the two places that did so.Fixes 88c22088bf23 ("mm: optimize page_lock_anon_vma() fast-path")Reported-by Dave Jones <[email protected]>Signed-off-by Hugh Dickins <[email protected]>Needs-Ack-from: Peter Zijlstra <[email protected]>
---mm/rmap.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)mm/rmap.c +6
-3@@ -426,12 +426,14 @@ struct anon_vma *page_get_anon_vma(struc* above cannot corrupt). */ if (!page_mapped (page)) { +rcu_read_unlock ();put_anon_vma (anon_vma); anon_vma =NULL ; + goto outer; } out:rcu_read_unlock (); - +outer:return anon_vma; }@@ -477,9 +479,10 @@ struct anon_vma *page_lock_anon_vma_read }if (!page_mapped (page)) { +rcu_read_unlock ();put_anon_vma (anon_vma); anon_vma =NULL ; - goto out; + goto outer; }/* we pinned the anon_vma, its safe to sleep */ @@ -501,6 +504,7 @@ struct anon_vma *page_lock_anon_vma_read out:rcu_read_unlock (); +outer:return anon_vma; } --
Quoted 5 lines
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 10 lines · Vlastimil Babka
Quoted 5 lines · Hugh Dickins
Quoted 3 lines · Dave Jones
BUG: sleeping function called from invalid context at kernel/locking/rwsem.c:47
in_atomic(): 0,irqs_disabled(): 0, pid: 5787, name: trinity-c27 Preemption disabled at:[<ffffffff990acc7e>]vtime_account_system+0x1e/0x50Just wondering, since I'm not familiar with this kind of bug, is the line above bogus or what does it mean? I don't see how the stack trace or the fix patch is related to
vtime_account_system?
I know no more about it than you do, never noticed such a message before, and clearly not helpful here. I expect it's like those "last sysfs file" messages, occasionally useful but mostly noise.
Hugh
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 27 lines · Hugh Dickins
[PATCH] mm: fix sleeping function warning from
__put_anon_vmaTrinity reports BUG:
sleeping function called from invalid context atkernel/locking/rwsem.c:47in_atomic(): 0,irqs_disabled(): 0, pid: 5787, name: trinity-c27__might_sleep<down_write<__put_anon_vma<page_get_anon_vma<migrate_pages<compact_zone<compact_zone_order<try_to_compact_pages..Right, since conversion to mutex then rwsem, we should not
put_anon_vma()from inside an rcu_read_lock()ed section: fix the two places that did so.Fixes 88c22088bf23 ("mm: optimize page_lock_anon_vma() fast-path")Reported-by Dave Jones <[email protected]>Signed-off-by Hugh Dickins <[email protected]>Needs-Ack-from: Peter Zijlstra <[email protected]>
---mm/rmap.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)mm/rmap.c +1
-0@@ -426,12 +426,14 @@ struct anon_vma *page_get_anon_vma(struc* above cannot corrupt). */ if (!page_mapped (page)) { +rcu_read_unlock ();
mm/rmap.c struct anon_vma *page_get_anon_vma(struc · line 438
Quoted 10 lines
mm/rmap.c +2
-1put_anon_vma (anon_vma); anon_vma =NULL ; + goto outer; } out:rcu_read_unlock (); - +outer:return anon_vma; }
I think we can do that without the goto if we write something like:
if (!page_mapped(page)) {
rcu_read_unlock();
put_anon_vma(anon_vma);
return NULL;
}Quoted 13 lines
mm/rmap.c +2
-1@@ -477,9 +479,10 @@ struct anon_vma *page_lock_anon_vma_read }if (!page_mapped (page)) { +rcu_read_unlock ();put_anon_vma (anon_vma); anon_vma =NULL ; - goto out; + goto outer; }/* we pinned the anon_vma, its safe to sleep */
mm/rmap.c struct anon_vma *page_lock_anon_vma_read · line 509
Quoted 6 lines
mm/rmap.c +1
-0@@ -501,6 +504,7 @@ struct anon_vma *page_lock_anon_vma_read out:rcu_read_unlock (); +outer:return anon_vma; }
Same here too, I suppose.
Interesting that we never managed to hit this one; it might also make sense to put a might_sleep() in anon_vma_free().
Other than that, I don't see anything really odd, then again, its sunday evening and my thinking cap isn't exactly on proper.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 41 lines · Peter Zijlstra
Quoted 27 lines · Hugh Dickins
[PATCH] mm: fix sleeping function warning from
__put_anon_vmaTrinity reports BUG:
sleeping function called from invalid context atkernel/locking/rwsem.c:47in_atomic(): 0,irqs_disabled(): 0, pid: 5787, name: trinity-c27__might_sleep<down_write<__put_anon_vma<page_get_anon_vma<migrate_pages<compact_zone<compact_zone_order<try_to_compact_pages..Right, since conversion to mutex then rwsem, we should not
put_anon_vma()from inside an rcu_read_lock()ed section: fix the two places that did so.Fixes 88c22088bf23 ("mm: optimize page_lock_anon_vma() fast-path")Reported-by Dave Jones <[email protected]>Signed-off-by Hugh Dickins <[email protected]>Needs-Ack-from: Peter Zijlstra <[email protected]>
---mm/rmap.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)mm/rmap.c +1
-0@@ -426,12 +426,14 @@ struct anon_vma *page_get_anon_vma(struc* above cannot corrupt). */ if (!page_mapped (page)) { +rcu_read_unlock ();mm/rmap.c struct anon_vma *page_get_anon_vma(struc · line 438
Quoted 10 lines
mm/rmap.c +2
-1put_anon_vma (anon_vma); anon_vma =NULL ; + goto outer; } out:rcu_read_unlock (); - +outer:return anon_vma; }I think we can do that without the goto if we write something like:
Quoted 5 lines
if (!page_mapped(page)) {
rcu_read_unlock();
put_anon_vma(anon_vma);
return NULL;
}Yes, that was my preference too. Then I thought we'd be accused of mixing early returns and gotos, so did it the goto way. Since we both prefer the early return, I'll put it back to that.
Quoted 23 lines
Quoted 14 lines
mm/rmap.c +2
-1@@ -477,9 +479,10 @@ struct anon_vma *page_lock_anon_vma_read }if (!page_mapped (page)) { +rcu_read_unlock ();put_anon_vma (anon_vma); anon_vma =NULL ; - goto out; + goto outer; }/* we pinned the anon_vma, its safe to sleep */ mm/rmap.c struct anon_vma *page_lock_anon_vma_read · line 509
Quoted 6 lines
mm/rmap.c +1
-0@@ -501,6 +504,7 @@ struct anon_vma *page_lock_anon_vma_read out:rcu_read_unlock (); +outer:return anon_vma; }Same here too, I suppose.
Quoted 2 lines
Interesting that we never managed to hit this one; it might also make sense to put a
might_sleep()inanon_vma_free().
Yes, good idea, that's an excellent place for a might_sleep(): done.
Though it does change the patch from a "push this out immediately" to a "let's wait a little to check we don't now see lots of new warnings".
But that's okay, we've survived three years without it; and I saw no no warnings myself from the new patch, when running under load plus lots of page migration.
Quoted 3 lines
Other than that, I don't see anything really odd, then again, its sunday evening and my thinking cap isn't exactly on proper.
Thank you - shout if any worries arise once the cap is back on. I think it's fine, I just felt more comfortable bringing you in.
[PATCH v2] mm: fix sleeping function warning from __put_anon_vma
Trinity reports BUG:
sleeping function called from invalid context at kernel/locking/rwsem.c:47 in_atomic(): 0, irqs_disabled(): 0, pid: 5787, name: trinity-c27 __might_sleep < down_write < __put_anon_vma < page_get_anon_vma < migrate_pages < compact_zone < compact_zone_order < try_to_compact_pages ..
Right, since conversion to mutex then rwsem, we should not put_anon_vma() from inside an rcu_read_lock()ed section: fix the two places that did so. And add might_sleep() to anon_vma_free(), as suggested by Peter Zijlstra.
---
v2: use early return instead of goto
add might_sleep in anon_vma_free
mm/rmap.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
mm/rmap.c +5 -3
@@ -103,6 +103,7 @@ static inline void anon_vma_free(struct* LOCK should suffice since the actual taking of the lock must * happen _before_ what follows. */ +might_sleep ();if (rwsem_is_locked (&anon_vma->root->rwsem)) {anon_vma_lock_write (anon_vma);anon_vma_unlock_write (anon_vma);
@@ -426,8 +427,9 @@ struct anon_vma *page_get_anon_vma(struc* above cannot corrupt). */ if (!page_mapped (page)) { +rcu_read_unlock ();put_anon_vma (anon_vma); - anon_vma =NULL ; +return NULL ; } out:rcu_read_unlock ();
@@ -477,9 +479,9 @@ struct anon_vma *page_lock_anon_vma_read }if (!page_mapped (page)) { +rcu_read_unlock ();put_anon_vma (anon_vma); - anon_vma =NULL ; - goto out; +return NULL ; }/* we pinned the anon_vma, its safe to sleep */
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>