From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754178AbbDMPci (ORCPT ); Mon, 13 Apr 2015 11:32:38 -0400 Received: from mail-wi0-f178.google.com ([209.85.212.178]:34332 "EHLO mail-wi0-f178.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753826AbbDMPcf (ORCPT ); Mon, 13 Apr 2015 11:32:35 -0400 Date: Mon, 13 Apr 2015 17:32:29 +0200 From: Ingo Molnar To: Peter Zijlstra Cc: rusty@rustcorp.com.au, mathieu.desnoyers@efficios.com, oleg@redhat.com, paulmck@linux.vnet.ibm.com, torvalds@linux-foundation.org, linux-kernel@vger.kernel.org, andi@firstfloor.org, rostedt@goodmis.org, tglx@linutronix.de, laijs@cn.fujitsu.com, linux@horizon.com Subject: Re: [PATCH v5 01/10] module: Sanitize RCU usage and locking Message-ID: <20150413153229.GA6040@gmail.com> References: <20150413141126.756350256@infradead.org> <20150413141213.241687161@infradead.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150413141213.241687161@infradead.org> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org * Peter Zijlstra wrote: > Currently the RCU usage in module is an inconsistent mess of RCU and > RCU-sched, this is broken for CONFIG_PREEMPT where synchronize_rcu() > does not imply synchronize_sched(). > > Most usage sites use preempt_{dis,en}able() which is RCU-sched, but > (most of) the modification sites use synchronize_rcu(). With the > exception of the module bug list, which actually uses RCU. > > Convert everything over to RCU-sched. > > Furthermore add lockdep asserts to all sites, because its not at all > clear to me the required locking is observed, esp. on exported > functions. nit: s/its/it's > +static void module_assert_mutex_or_preempt(void) > +{ > +#ifdef CONFIG_LOCKDEP > + int rcu_held = rcu_read_lock_sched_held(); > + int mutex_held = 1; > + > + if (debug_locks) > + mutex_held = lockdep_is_held(&module_mutex); > + > + WARN_ON(!rcu_held && !mutex_held); So because rcu_read_lock_sched_held() also depends on debug_locks being on to be fully correct, shouldn't the warning also be within the debug_locks condition? > @@ -3106,11 +3128,11 @@ static noinline int do_init_module(struc > mod->init_text_size = 0; > /* > * We want to free module_init, but be aware that kallsyms may be > + * walking this with preempt disabled. In all the failure paths, we > + * call synchronize_sched, but we don't want to slow down the success > + * path, so use actual RCU here. nit: s/synchronize_sched /synchronize_sched() Thanks, Ingo