From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.2 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6B14AC56202 for ; Tue, 27 Oct 2020 07:07:56 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id C9AF821556 for ; Tue, 27 Oct 2020 07:07:55 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org C9AF821556 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 2EBB36B005D; Tue, 27 Oct 2020 03:07:55 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 29BD86B0062; Tue, 27 Oct 2020 03:07:55 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 18ADD6B006C; Tue, 27 Oct 2020 03:07:55 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0139.hostedemail.com [216.40.44.139]) by kanga.kvack.org (Postfix) with ESMTP id E01296B005D for ; Tue, 27 Oct 2020 03:07:54 -0400 (EDT) Received: from smtpin10.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id 8B001180AD807 for ; Tue, 27 Oct 2020 07:07:54 +0000 (UTC) X-FDA: 77416825668.10.vase87_4b130942727a Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin10.hostedemail.com (Postfix) with ESMTP id 6CAE516A0D1 for ; Tue, 27 Oct 2020 07:07:54 +0000 (UTC) X-HE-Tag: vase87_4b130942727a X-Filterd-Recvd-Size: 5531 Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by imf48.hostedemail.com (Postfix) with ESMTP for ; Tue, 27 Oct 2020 07:07:53 +0000 (UTC) IronPort-SDR: HslJBODy30ubz9ZigHsNd+pwTyTdqhM3l8dF+T6Q/Z2EZ8azHk6DBhoyyJTCQKyFuOHMiW7wDq l3OL0PRWFfWA== X-IronPort-AV: E=McAfee;i="6000,8403,9786"; a="165446728" X-IronPort-AV: E=Sophos;i="5.77,422,1596524400"; d="scan'208";a="165446728" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga004.jf.intel.com ([10.7.209.38]) by fmsmga104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 27 Oct 2020 00:07:51 -0700 IronPort-SDR: A5IXsCcNEXQXEcg/oDhntuvq+fWDgRPE+ldHDsUxyxBUCd/F6AsJTG2smijql50iqT70s1/Kgh K0bYXPlZ5eAw== X-IronPort-AV: E=Sophos;i="5.77,422,1596524400"; d="scan'208";a="468194512" Received: from iweiny-desk2.sc.intel.com (HELO localhost) ([10.3.52.147]) by orsmga004-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 27 Oct 2020 00:07:50 -0700 Date: Tue, 27 Oct 2020 00:07:50 -0700 From: Ira Weiny To: Thomas Gleixner , "Paul E. McKenney" Cc: Ingo Molnar , Borislav Petkov , Andy Lutomirski , Peter Zijlstra , x86@kernel.org, Dave Hansen , Dan Williams , Andrew Morton , Fenghua Yu , linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-nvdimm@lists.01.org, linux-mm@kvack.org, linux-kselftest@vger.kernel.org Subject: Re: [PATCH 06/10] x86/entry: Move nmi entry/exit into common code Message-ID: <20201027070750.GM534324@iweiny-DESK2.sc.intel.com> References: <20201022222701.887660-1-ira.weiny@intel.com> <20201022222701.887660-7-ira.weiny@intel.com> <874kmk6298.fsf@nanos.tec.linutronix.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <874kmk6298.fsf@nanos.tec.linutronix.de> User-Agent: Mutt/1.11.1 (2018-12-01) X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Fri, Oct 23, 2020 at 11:50:11PM +0200, Thomas Gleixner wrote: > On Thu, Oct 22 2020 at 15:26, ira weiny wrote: > > > From: Thomas Gleixner > > > > Lockdep state handling on NMI enter and exit is nothing specific to X86. It's > > not any different on other architectures. Also the extra state type is not > > necessary, irqentry_state_t can carry the necessary information as well. > > > > Move it to common code and extend irqentry_state_t to carry lockdep > > state. > > This lacks something like: > > [ Ira: Made the states a union as they are mutually exclusive and added > the missing kernel doc ] Fair enough. done. > > Hrm. > > > #ifndef irqentry_state > > typedef struct irqentry_state { > > - bool exit_rcu; > > + union { > > + bool exit_rcu; > > + bool lockdep; > > + }; > > } irqentry_state_t; > > #endif > > -E_NO_KERNELDOC Adding: Paul McKenney I'm happy to write something but I'm very unfamiliar with this code. So I'm getting confused what exactly exit_rcu is flagging. I can see that exit_rcu is a bad name for the state used in irqentry_nmi_[enter|exit](). Furthermore, I see why 'lockdep' is a better name. But similar lockdep handling is used in irqentry_exit() if exit_rcu is true... Given my limited knowledge; here is my proposed text: /** * struct irqentry_state - Opaque object for exception state storage * @exit_rcu: Used exclusively in the irqentry_*() calls; tracks if the * exception hit the idle task which requires special handling, * including calling rcu_irq_exit(), when the exception exits. * @lockdep: Used exclusively in the irqentry_nmi_*() calls; ensures lockdep * tracking is maintained if hardirqs were already enabled * * This opaque object is filled in by the irqentry_*_enter() functions and * should be passed back into the corresponding irqentry_*_exit() functions * when the exception is complete. * * Callers of irqentry_*_[enter|exit]() should consider this structure opaque * and all members private. Descriptions of the members are provided to aid in * the maintenance of the irqentry_*() functions. */ Perhaps Paul can enlighten me on how exit_rcu is used beyond just flagging a call to rcu_irq_exit()? Why do we call lockdep_hardirqs_off() only when in the idle task? That implies that regs_irqs_disabled() can only be false if we were in the idle task to match up the lockdep on/off calls. This does not make sense to me because why do we need the extra check for exit_rcu? I'm still trying to understand when regs_irqs_disabled() is false. } else if (!regs_irqs_disabled(regs)) { ... } else { /* * IRQ flags state is correct already. Just tell RCU if it * was not watching on entry. */ if (state.exit_rcu) rcu_irq_exit(); } Also, the comment in irqentry_enter() refers to irq_enter_from_user_mode() which does not seem to exist anymore. So I'm not sure what careful sequence it is referring to. /* * If RCU is not watching then the same careful * sequence vs. lockdep and tracing is required * as in irq_enter_from_user_mode(). */ ? Ira