From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751662AbcGRDRl (ORCPT ); Sun, 17 Jul 2016 23:17:41 -0400 Received: from ozlabs.org ([103.22.144.67]:35854 "EHLO ozlabs.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751455AbcGRDRi (ORCPT ); Sun, 17 Jul 2016 23:17:38 -0400 From: Rusty Russell To: Steven Rostedt Cc: LKML , Andrew Morton , Jason Baron , "Peter Zijlstra" , "Jessica Yu" Subject: Re: [PATCH] module: Do a WARN_ON_ONCE() for assert module mutex not held In-Reply-To: <20160712091550.30ddfff7@gandalf.local.home> References: <20160712091550.30ddfff7@gandalf.local.home> User-Agent: Notmuch/0.21 (http://notmuchmail.org) Emacs/24.5.1 (x86_64-pc-linux-gnu) Date: Mon, 18 Jul 2016 12:46:41 +0930 Message-ID: <8760s3iqjq.fsf@rustcorp.com.au> MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Steven Rostedt writes: > When running with lockdep enabled, I triggered the WARN_ON() in the > module code that asserts when module_mutex or rcu_read_lock_sched are > not held. The issue I have is that this can also be called from the > dump_stack() code, causing us to enter an infinite loop... Thanks, applied. It would be good to see a proper stacktrace though. Hmm, this caller looks like it might be unprotected: arch_prepare_optimized_kprobe -> copy_optimized_instructions -> jump_label_text_reserved -> __jump_label_mod_text_reserved -> __module_text_address It holds text_mutex, but preempt is still enabled AFAICT. Does this help? Rusty. diff --git a/kernel/jump_label.c b/kernel/jump_label.c index 4b353e0be121..d636ce4af995 100644 --- a/kernel/jump_label.c +++ b/kernel/jump_label.c @@ -284,11 +284,14 @@ static int __jump_label_mod_text_reserved(void *start, void *end) { struct module *mod; + preempt_disable(); mod = __module_text_address((unsigned long)start); + WARN_ON_ONCE(__module_text_address((unsigned long)end) != mod); + preempt_enable(); + if (!mod) return 0; - WARN_ON_ONCE(__module_text_address((unsigned long)end) != mod); return __jump_label_text_reserved(mod->jump_entries, mod->jump_entries + mod->num_jump_entries, > ------------[ cut here ]------------ > WARNING: CPU: 1 PID: 0 at kernel/module.c:268 module_assert_mutex_or_preempt+0x3c/0x3e > Modules linked in: ip6t_REJECT nf_reject_ipv6 nf_conntrack_ipv6 > CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.7.0-rc3-test-00013-g501c2375253c #14 > Hardware name: MSI MS-7823/CSM-H87M-G43 (MS-7823), BIOS V1.6 02/22/2014 > ffff880215e8fa70 ffff880215e8fa70 ffffffff812fc8e3 0000000000000000 > ffffffff81d3e55b ffff880215e8fac0 ffffffff8104fc88 ffffffff8104fcab > 0000000915e88300 0000000000000046 ffffffffa019b29a 0000000000000001 > Call Trace: > [] dump_stack+0x67/0x90 > [] __warn+0xcb/0xe9 > [] ? warn_slowpath_null+0x5/0x1f > ------------[ cut here ]------------ > WARNING: CPU: 1 PID: 0 at kernel/module.c:268 module_assert_mutex_or_preempt+0x3c/0x3e > Modules linked in: ip6t_REJECT nf_reject_ipv6 nf_conntrack_ipv6 > CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.7.0-rc3-test-00013-g501c2375253c #14 > Hardware name: MSI MS-7823/CSM-H87M-G43 (MS-7823), BIOS V1.6 02/22/2014 > ffff880215e8f7a0 ffff880215e8f7a0 ffffffff812fc8e3 0000000000000000 > ffffffff81d3e55b ffff880215e8f7f0 ffffffff8104fc88 ffffffff8104fcab > 0000000915e88300 0000000000000046 ffffffffa019b29a 0000000000000001 > Call Trace: > [] dump_stack+0x67/0x90 > [] __warn+0xcb/0xe9 > [] ? warn_slowpath_null+0x5/0x1f > ------------[ cut here ]------------ > WARNING: CPU: 1 PID: 0 at kernel/module.c:268 module_assert_mutex_or_preempt+0x3c/0x3e > Modules linked in: ip6t_REJECT nf_reject_ipv6 nf_conntrack_ipv6 > CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.7.0-rc3-test-00013-g501c2375253c #14 > Hardware name: MSI MS-7823/CSM-H87M-G43 (MS-7823), BIOS V1.6 02/22/2014 > ffff880215e8f4d0 ffff880215e8f4d0 ffffffff812fc8e3 0000000000000000 > ffffffff81d3e55b ffff880215e8f520 ffffffff8104fc88 ffffffff8104fcab > 0000000915e88300 0000000000000046 ffffffffa019b29a 0000000000000001 > Call Trace: > [] dump_stack+0x67/0x90 > [] __warn+0xcb/0xe9 > [] ? warn_slowpath_null+0x5/0x1f > ------------[ cut here ]------------ > WARNING: CPU: 1 PID: 0 at kernel/module.c:268 module_assert_mutex_or_preempt+0x3c/0x3e > [...] > > Which gives us rather useless information. Worse yet, there's some race > that causes this, and I seldom trigger it, so I have no idea what > happened. > > This would not be an issue if that warning was a WARN_ON_ONCE(). > > Signed-off-by: Steven Rostedt > --- > diff --git a/kernel/module.c b/kernel/module.c > index 5f71aa63ed2a..51c89d86752c 100644 > --- a/kernel/module.c > +++ b/kernel/module.c > @@ -264,7 +264,7 @@ static void module_assert_mutex_or_preempt(void) > if (unlikely(!debug_locks)) > return; > > - WARN_ON(!rcu_read_lock_sched_held() && > + WARN_ON_ONCE(!rcu_read_lock_sched_held() && > !lockdep_is_held(&module_mutex)); > #endif > }