Received: by 2002:a05:6a10:6006:0:0:0:0 with SMTP id w6csp88545pxa; Wed, 26 Aug 2020 18:07:40 -0700 (PDT) X-Google-Smtp-Source: ABdhPJxL8HxyVgHE7qHSR8gmxikkQhOgr89j94R3ijV/xK8JmBtKtf3eMnDuiMHdk0l/xior+JI2 X-Received: by 2002:a05:6402:1350:: with SMTP id y16mr15645880edw.68.1598490460062; Wed, 26 Aug 2020 18:07:40 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1598490460; cv=none; d=google.com; s=arc-20160816; b=Plj1p8H6+gW2OrHOFf98WfbjPJhESVYtVaopayegjampzf0XhOjAbeV+BTivqmVZm8 NHNJ4SOpWbDUydf08CC8XWXOeLUXQQnn4UcCoP5n0Wdjo+raR3TNODrS1dJ7lrA7ZWdn 1nuavXQhZmgFn2SbGee9++K9cTQ8/EBHaE6qyXcPIeOu/Nvmd2k5qVK7W2yNWhGBiQYg ZsUPfqxp7mT/Q+s0iDRluBfYUNwSwXLrL0A3fx7UYGteLQuQhcdqUMPzheFmWBu2hkrd V/RMzEV7oKQJadI41Wi9WJXvRsmxoEBCpYO8rz64Lp1d0BbRZ7pYFmmzv5/QvQcUdpju pH9Q== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:in-reply-to:content-disposition :mime-version:references:message-id:subject:cc:to:from:date :dkim-signature; bh=Vx3vK6z7068nWwSolZcqRqbNaxoD/OKZuHAXpq7JL4o=; b=kv+P5ATBHdlmwaCL/nazEPpco9b+CGoFx2Q7F/2NLaenk0bpbPhxRvCyHX9Hbwq4El 8Mwc/Sb2BkvAxb5UL38z+FMk1YOMIaV2JP30pwz4lMAo80kdS6po4hKEOInI2dGoMPuU xBXtcfs+d6dnYrIHQXLj5hmG9d0IQANK1KMoCmlNhBaJDyyWFrOawVHLvsOWDrUrvI0O 3Fhvjdka1jnyb1YzgITOvMuWjFIHuhhL7f8IAJ7csNaKdcXR70KyySH1l6s+7dpvgNWm 5WJHcuc0xh9k5xzuOv9/6Z6qNtJDrmrbQAdBDRstmGem/Mh839n8hPqAQgU8AAL4HHvf J2Xg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@joelfernandes.org header.s=google header.b=THtB5AIn; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id zh8si329026ejb.475.2020.08.26.18.07.15; Wed, 26 Aug 2020 18:07:40 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@joelfernandes.org header.s=google header.b=THtB5AIn; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726788AbgH0BFq (ORCPT + 99 others); Wed, 26 Aug 2020 21:05:46 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:44758 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726148AbgH0BFp (ORCPT ); Wed, 26 Aug 2020 21:05:45 -0400 Received: from mail-qk1-x742.google.com (mail-qk1-x742.google.com [IPv6:2607:f8b0:4864:20::742]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4EDCBC061797 for ; Wed, 26 Aug 2020 18:05:45 -0700 (PDT) Received: by mail-qk1-x742.google.com with SMTP id x69so4526875qkb.1 for ; Wed, 26 Aug 2020 18:05:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=joelfernandes.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=Vx3vK6z7068nWwSolZcqRqbNaxoD/OKZuHAXpq7JL4o=; b=THtB5AInNMjra8njAqz/vs+P5k664twRLsPYgVfjTGeryiFUn6Oy9I3TXkhBjVVbGY mGS7fnAQYKQlttAELxhEM/AqYs3nCPaL+U/0tk9mUVqot2nvJt11I+1zlyhd8TOvrVU6 kBRrJhDScw7JDeIQVARwFTn55Usf8ILOc50hg= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=Vx3vK6z7068nWwSolZcqRqbNaxoD/OKZuHAXpq7JL4o=; b=TtZJitUwOrfsgDeiG5IZb/vboA3GjZFwqM9hWmf+Csj82rBt86T0bYHfDOtOp3+fvA +rzUL3BRrm9n/fbuidqSXW9xHu/x+Z+VWDPwljtR1KNEmDcSzjk7ADFGLMWH9YllDtJ3 KC90J3oiLkPvas0CTkKUfi/XmuFvsRqI5UespT/BYzagzP/G7qwfDN9pK/b4ORQKOWAY BRfS10wf2a0hnKxY/MK6tV6tk3D37HiqMqFJiJpYSwNj+7vTXtW1T6/jOBbt8aFpHdiI mfLuz0GfYDfFAl/IyBoDvR7Bq0OnkQIylaeI0jFpW7ycUvwblhkSMjWCzcLRnZdaweB4 cl0Q== X-Gm-Message-State: AOAM531l4koV/H0pEck1UD9JkOqC2/gH8rya+ZXjDFdnFzzldRkvcVw7 dgSSWV1Xs5h2fF3QDkwLMTbBKA== X-Received: by 2002:a37:30e:: with SMTP id 14mr16662052qkd.251.1598490344349; Wed, 26 Aug 2020 18:05:44 -0700 (PDT) Received: from localhost ([2620:15c:6:12:cad3:ffff:feb3:bd59]) by smtp.gmail.com with ESMTPSA id k48sm659103qtk.44.2020.08.26.18.05.43 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 26 Aug 2020 18:05:43 -0700 (PDT) Date: Wed, 26 Aug 2020 21:05:43 -0400 From: Joel Fernandes To: Peter Zijlstra Cc: linux-kernel@vger.kernel.org, mingo@kernel.org, will@kernel.org, npiggin@gmail.com, elver@google.com, jgross@suse.com, paulmck@kernel.org, rostedt@goodmis.org, rjw@rjwysocki.net, svens@linux.ibm.com, tglx@linutronix.de Subject: Re: [PATCH v2 01/11] lockdep: Use raw_cpu_*() for per-cpu variables Message-ID: <20200827010543.GA1939707@google.com> References: <20200821084738.508092956@infradead.org> <20200821085348.192346882@infradead.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200821085348.192346882@infradead.org> Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Aug 21, 2020 at 10:47:39AM +0200, Peter Zijlstra wrote: > Sven reported that commit a21ee6055c30 ("lockdep: Change > hardirq{s_enabled,_context} to per-cpu variables") caused trouble on > s390 because their this_cpu_*() primitives disable preemption which > then lands back tracing. > > On the one hand, per-cpu ops should use preempt_*able_notrace() and > raw_local_irq_*(), on the other hand, we can trivialy use raw_cpu_*() > ops for this. > > Fixes: a21ee6055c30 ("lockdep: Change hardirq{s_enabled,_context} to per-cpu variables") > Reported-by: Sven Schnelle > Reviewed-by: Steven Rostedt (VMware) > Tested-by: Marco Elver Reviewed-by: Joel Fernandes (Google) thanks, - Joel > Signed-off-by: Peter Zijlstra (Intel) > --- > include/linux/irqflags.h | 6 +++--- > include/linux/lockdep.h | 18 +++++++++++++----- > kernel/locking/lockdep.c | 4 ++-- > 3 files changed, 18 insertions(+), 10 deletions(-) > > --- a/include/linux/irqflags.h > +++ b/include/linux/irqflags.h > @@ -53,13 +53,13 @@ DECLARE_PER_CPU(int, hardirq_context); > extern void trace_hardirqs_off_finish(void); > extern void trace_hardirqs_on(void); > extern void trace_hardirqs_off(void); > -# define lockdep_hardirq_context() (this_cpu_read(hardirq_context)) > +# define lockdep_hardirq_context() (raw_cpu_read(hardirq_context)) > # define lockdep_softirq_context(p) ((p)->softirq_context) > # define lockdep_hardirqs_enabled() (this_cpu_read(hardirqs_enabled)) > # define lockdep_softirqs_enabled(p) ((p)->softirqs_enabled) > # define lockdep_hardirq_enter() \ > do { \ > - if (this_cpu_inc_return(hardirq_context) == 1) \ > + if (__this_cpu_inc_return(hardirq_context) == 1)\ > current->hardirq_threaded = 0; \ > } while (0) > # define lockdep_hardirq_threaded() \ > @@ -68,7 +68,7 @@ do { \ > } while (0) > # define lockdep_hardirq_exit() \ > do { \ > - this_cpu_dec(hardirq_context); \ > + __this_cpu_dec(hardirq_context); \ > } while (0) > # define lockdep_softirq_enter() \ > do { \ > --- a/include/linux/lockdep.h > +++ b/include/linux/lockdep.h > @@ -535,19 +535,27 @@ do { \ > DECLARE_PER_CPU(int, hardirqs_enabled); > DECLARE_PER_CPU(int, hardirq_context); > > +/* > + * The below lockdep_assert_*() macros use raw_cpu_read() to access the above > + * per-cpu variables. This is required because this_cpu_read() will potentially > + * call into preempt/irq-disable and that obviously isn't right. This is also > + * correct because when IRQs are enabled, it doesn't matter if we accidentally > + * read the value from our previous CPU. > + */ > + > #define lockdep_assert_irqs_enabled() \ > do { \ > - WARN_ON_ONCE(debug_locks && !this_cpu_read(hardirqs_enabled)); \ > + WARN_ON_ONCE(debug_locks && !raw_cpu_read(hardirqs_enabled)); \ > } while (0) > > #define lockdep_assert_irqs_disabled() \ > do { \ > - WARN_ON_ONCE(debug_locks && this_cpu_read(hardirqs_enabled)); \ > + WARN_ON_ONCE(debug_locks && raw_cpu_read(hardirqs_enabled)); \ > } while (0) > > #define lockdep_assert_in_irq() \ > do { \ > - WARN_ON_ONCE(debug_locks && !this_cpu_read(hardirq_context)); \ > + WARN_ON_ONCE(debug_locks && !raw_cpu_read(hardirq_context)); \ > } while (0) > > #define lockdep_assert_preemption_enabled() \ > @@ -555,7 +563,7 @@ do { \ > WARN_ON_ONCE(IS_ENABLED(CONFIG_PREEMPT_COUNT) && \ > debug_locks && \ > (preempt_count() != 0 || \ > - !this_cpu_read(hardirqs_enabled))); \ > + !raw_cpu_read(hardirqs_enabled))); \ > } while (0) > > #define lockdep_assert_preemption_disabled() \ > @@ -563,7 +571,7 @@ do { \ > WARN_ON_ONCE(IS_ENABLED(CONFIG_PREEMPT_COUNT) && \ > debug_locks && \ > (preempt_count() == 0 && \ > - this_cpu_read(hardirqs_enabled))); \ > + raw_cpu_read(hardirqs_enabled))); \ > } while (0) > > #else > --- a/kernel/locking/lockdep.c > +++ b/kernel/locking/lockdep.c > @@ -3756,7 +3756,7 @@ void noinstr lockdep_hardirqs_on(unsigne > > skip_checks: > /* we'll do an OFF -> ON transition: */ > - this_cpu_write(hardirqs_enabled, 1); > + __this_cpu_write(hardirqs_enabled, 1); > trace->hardirq_enable_ip = ip; > trace->hardirq_enable_event = ++trace->irq_events; > debug_atomic_inc(hardirqs_on_events); > @@ -3795,7 +3795,7 @@ void noinstr lockdep_hardirqs_off(unsign > /* > * We have done an ON -> OFF transition: > */ > - this_cpu_write(hardirqs_enabled, 0); > + __this_cpu_write(hardirqs_enabled, 0); > trace->hardirq_disable_ip = ip; > trace->hardirq_disable_event = ++trace->irq_events; > debug_atomic_inc(hardirqs_off_events); > >