From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752563AbdIVWzA (ORCPT ); Fri, 22 Sep 2017 18:55:00 -0400 Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:42790 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752041AbdIVWy7 (ORCPT ); Fri, 22 Sep 2017 18:54:59 -0400 Date: Fri, 22 Sep 2017 15:54:55 -0700 From: "Paul E. McKenney" To: Steven Rostedt Cc: linux-kernel@vger.kernel.org, ngo Molnar , Andrew Morton , stable@vger.kernel.org Subject: Re: [PATCH 4/4] tracing: Remove RCU work arounds from stack tracer Reply-To: paulmck@linux.vnet.ibm.com References: <20170922221543.900812109@goodmis.org> <20170922221837.883750092@goodmis.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170922221837.883750092@goodmis.org> User-Agent: Mutt/1.5.21 (2010-09-15) X-TM-AS-GCONF: 00 x-cbid: 17092222-0024-0000-0000-000002D692F4 X-IBM-SpamModules-Scores: X-IBM-SpamModules-Versions: BY=3.00007780; HX=3.00000241; KW=3.00000007; PH=3.00000004; SC=3.00000231; SDB=6.00920913; UDB=6.00462786; IPR=6.00701133; BA=6.00005601; NDR=6.00000001; ZLA=6.00000005; ZF=6.00000009; ZB=6.00000000; ZP=6.00000000; ZH=6.00000000; ZU=6.00000002; MB=3.00017252; XFM=3.00000015; UTC=2017-09-22 22:54:55 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17092222-0025-0000-0000-00004582E349 Message-Id: <20170922225455.GA3521@linux.vnet.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-09-22_09:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=0 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1707230000 definitions=main-1709220322 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Sep 22, 2017 at 06:15:47PM -0400, Steven Rostedt wrote: > From: "Steven Rostedt (VMware)" > > While debugging some RCU issues with the stack tracer, it was discovered > that the problem was much more than with the stack tracer itself, but with > the saving of the stack trace, which could happen from any WARN() as well. > The problem was fixed within kernel_text_address(). > > One of the bugs that was discovered was that the stack tracer called > rcu_enter_irq() unconditionally. Paul McKenney said that could cause issues > as well. Instead of adding logic to only call rcu_enter_irq() if RCU is not > watching from within the stack tracer, since the core issue has been fixed > (within save_stack_trace()), we can simply remove all the logic in the stack > tracer that deals with RCU work arounds. I must confess that I am having some difficulty parsing this paragraph, especially the last sentence... Does this capture it? One problem is that the stack tracer called rcu_irq_enter() unconditionally, which is problematic if RCU's last not-watching-to-watching transition was carried out by rcu_nmi_enter. In that case, rcu_irq_enter() actually switches RCU back to the not-watching state for this CPU, which results in lockdep splats complaining about rcu_read_lock() being used on an idle (not-watched) CPU. The first patch of this series addressed this problem by having rcu_irq_enter() and rcu_irq_exit() refrain from doing anything when rcu_nmi_enter() caused RCU to start watching this CPU. The third patch in this series caused save_stack_trace() to invoke rcu_nmi_enter() and rcu_nmi_exit() as needed, so this fourth patch now removes the rcu_irq_enter() and rcu_irq_exit() from within the stack tracer. One further question... Can I now remove the rcu_irq_enter_disabled() logic? > Cc: "Paul E. McKenney" > Cc: stable@vger.kernel.org > Fixes: 0be964be0 ("module: Sanitize RCU usage and locking") > Suggested-by: "Paul E. McKenney" > Signed-off-by: Steven Rostedt (VMware) With the hard-to-parse paragraph fixed: Acked-by: Paul E. McKenney > --- > kernel/trace/trace_stack.c | 15 --------------- > 1 file changed, 15 deletions(-) > > diff --git a/kernel/trace/trace_stack.c b/kernel/trace/trace_stack.c > index a4df67cbc711..49cb41412eec 100644 > --- a/kernel/trace/trace_stack.c > +++ b/kernel/trace/trace_stack.c > @@ -96,23 +96,9 @@ check_stack(unsigned long ip, unsigned long *stack) > if (in_nmi()) > return; > > - /* > - * There's a slight chance that we are tracing inside the > - * RCU infrastructure, and rcu_irq_enter() will not work > - * as expected. > - */ > - if (unlikely(rcu_irq_enter_disabled())) > - return; > - > local_irq_save(flags); > arch_spin_lock(&stack_trace_max_lock); > > - /* > - * RCU may not be watching, make it see us. > - * The stack trace code uses rcu_sched. > - */ > - rcu_irq_enter(); > - > /* In case another CPU set the tracer_frame on us */ > if (unlikely(!frame_size)) > this_size -= tracer_frame; > @@ -205,7 +191,6 @@ check_stack(unsigned long ip, unsigned long *stack) > } > > out: > - rcu_irq_exit(); > arch_spin_unlock(&stack_trace_max_lock); > local_irq_restore(flags); > } > -- > 2.13.2 > >