From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756085AbaEaUec (ORCPT ); Sat, 31 May 2014 16:34:32 -0400 Received: from mail-pb0-f43.google.com ([209.85.160.43]:52988 "EHLO mail-pb0-f43.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755481AbaEaUea (ORCPT ); Sat, 31 May 2014 16:34:30 -0400 Date: Sat, 31 May 2014 13:33:13 -0700 (PDT) From: Hugh Dickins X-X-Sender: hugh@eggly.anvils To: Dave Jones cc: Peter Zijlstra , Andrew Morton , linux-kernel@vger.kernel.org, linux-mm@kvack.org Subject: Re: sleeping function warning from __put_anon_vma In-Reply-To: <20140530000944.GA29942@redhat.com> Message-ID: References: <20140530000944.GA29942@redhat.com> User-Agent: Alpine 2.11 (LSU 23 2013-08-11) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 29 May 2014, Dave Jones wrote: > 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:[] 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: > [] dump_stack+0x4e/0x7a > [] __might_sleep+0x11c/0x1b0 > [] down_write+0x20/0x40 > [] __put_anon_vma+0x3d/0xc0 > [] page_get_anon_vma+0x68/0xb0 > [] migrate_pages+0x449/0x880 > [] ? isolate_freepages_block+0x360/0x360 > [] compact_zone+0x38a/0x580 > [] compact_zone_order+0xa9/0x130 > [] try_to_compact_pages+0xe9/0x140 > [] __alloc_pages_direct_compact+0x7a/0x250 > [] __alloc_pages_nodemask+0x70b/0xbb0 > [] alloc_pages_vma+0xaf/0x1c0 > [] do_huge_pmd_anonymous_page+0xed/0x3d0 > [] handle_mm_fault+0x1b4/0xc50 > [] ? retint_restore_args+0xe/0xe > [] __do_page_fault+0x1c9/0x630 > [] ? __acct_update_integrals+0x8b/0x120 > [] ? preempt_count_sub+0xab/0x100 > [] do_page_fault+0x1e/0x70 > [] 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. Fixes: 88c22088bf23 ("mm: optimize page_lock_anon_vma() fast-path") Reported-by: Dave Jones Signed-off-by: Hugh Dickins Needs-Ack-from: Peter Zijlstra --- mm/rmap.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) --- 3.15-rc7/mm/rmap.c 2014-04-13 17:24:36.680507189 -0700 +++ linux/mm/rmap.c 2014-05-31 12:02:08.496088637 -0700 @@ -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; }