From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965846AbbBCNjb (ORCPT ); Tue, 3 Feb 2015 08:39:31 -0500 Received: from e36.co.us.ibm.com ([32.97.110.154]:37304 "EHLO e36.co.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S965578AbbBCNj1 (ORCPT ); Tue, 3 Feb 2015 08:39:27 -0500 Date: Tue, 3 Feb 2015 05:39:20 -0800 From: "Paul E. McKenney" To: Peter Zijlstra Cc: linux-kernel@vger.kernel.org, Steven Rostedt Subject: Re: [PATCH RFC] Make rcu_dereference_raw() safe for NMI etc. Message-ID: <20150203133920.GO19109@linux.vnet.ibm.com> Reply-To: paulmck@linux.vnet.ibm.com References: <20150202195532.GA2218@linux.vnet.ibm.com> <20150203110040.GJ26304@twins.programming.kicks-ass.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150203110040.GJ26304@twins.programming.kicks-ass.net> User-Agent: Mutt/1.5.21 (2010-09-15) X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 15020313-0021-0000-0000-000008442709 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Feb 03, 2015 at 12:00:40PM +0100, Peter Zijlstra wrote: > On Mon, Feb 02, 2015 at 11:55:33AM -0800, Paul E. McKenney wrote: > > As promised/threatened on IRC. > > > > Thanx, Paul > > > > ------------------------------------------------------------------------ > > > > rcu: Reverse rcu_dereference_check() conditions > > > > The rcu_dereference_check() family of primitives evaluates the RCU > > lockdep expression first, and only then evaluates the expression passed > > in. This works fine normally, but can potentially fail in environments > > (such as NMI handlers) where lockdep cannot be invoked. The problem is > > that even if the expression passed in is "1", the compiler would need to > > prove that the RCU lockdep expression (rcu_read_lock_held(), for example) > > is free of side effects in order to be able to elide it. Given that > > rcu_read_lock_held() is sometimes separately compiled, the compiler cannot > > always use this optimization. > > > > This commit therefore reverse the order of evaluation, so that the > > expression passed in is evaluated first, and the RCU lockdep expression is > > evaluated only if the passed-in expression evaluated to false, courtesy > > of the C-language short-circuit boolean evaluation rules. This compells > > the compiler to forego executing the RCU lockdep expression in cases > > where the passed-in expression evaluates to "1" at compile time, so that > > (for example) rcu_dereference_raw() can be guaranteed to execute safely > > withing an NMI handler. > > My particular worry yesterday was tracing; I was looking at > rcu_read_{,un}lock_notrace() and wondered what would happen if I used > list_for_each_entry_rcu() under it. > > _If_ it would indeed do that call, we can end up in: > > list_entry_rcu() -> rcu_dereference_raw() -> rcu_dereference_check() > -> rcu_read_lock_held() -> rcu_lockdep_current_cpu_online() > -> preempt_disable() > > And preempt_disable() is a traceable thing -- not to mention half the > callstack above doesn't have notrace annotations and would equally > generate function trace events. > > Thereby rendering the rcu list ops unsuitable for using under _notrace() > rcu primitives. > > So yes, fully agreed on this patch. > > Acked-by: Peter Zijlstra (Intel) Applied your Acked-by, thank you! > FWIW I think I won't be needing the rcu _notrace() bits (for now), but > it leading to this patch was worth it anyhow ;-) No argument here! This could have been a nasty one to track down, depending on exactly how it manifested. Much easier this way! ;-) Thanx, Paul