From: Mark Rutland <mark.rutland@arm.com> To: Nicolas Saenz Julienne <nsaenzju@redhat.com> Cc: linux-kernel <linux-kernel@vger.kernel.org>, linux-arm-kernel <linux-arm-kernel@lists.infradead.org>, rcu@vger.kernel.org, Thomas Gleixner <tglx@linutronix.de>, Peter Zijlstra <peterz@infradead.org>, Steven Rostedt <rostedt@goodmis.org>, mtosatti <mtosatti@redhat.com>, frederic <frederic@kernel.org>, paulmck@kernel.org Subject: Re: Question WRT early IRQ/NMI entry code Date: Tue, 30 Nov 2021 12:50:30 +0000 [thread overview] Message-ID: <YaYeFu4hi3uVkhkN@FVFF77S0Q05N> (raw) In-Reply-To: <8719ad46cc29a2c5d7baac3c35770e5460ab8d5c.camel@redhat.com> On Tue, Nov 30, 2021 at 12:28:41PM +0100, Nicolas Saenz Julienne wrote: > Hi All, Hi Nicolas, > while going over the IRQ/NMI entry code I've found a small 'inconsistency': > while in the IRQ entry path, we inform RCU of the context change *before* > incrementing the preempt counter, the opposite happens for the NMI entry > path. This applies to both arm64 and x86[1]. For arm64, the style was copied from the x86 code, and (AFAIK) I had no particular reason for following either order other than consistency with x86. > Actually, rcu_nmi_enter() — which is also the main RCU context switch function > for the IRQ entry path — uses the preempt counter to verify it's not in NMI > context. So it would make sense to assume all callers have the same updated > view of the preempt count, which isn't true ATM. I agree consistency would be nice, assuming there's no issue preventing us from moving the IRQ preempt_count logic earlier. It sounds like today the ordering is only *required* when entering an NMI, and we already do the right thing there. Do you see a case where something would go wrong (or would behave differently with the flipped ordering) for IRQ today? > I'm sure there an obscure/non-obvious reason for this, right? TBH I suspect this is mostly oversight / legacy, and likely something we can tighten up. Thanks, Mark. > > Thanks! > Nicolas > > [1] > IRQ path: > -> x86_64 asm (entry_64.S) > -> irqentry_enter() -> rcu_irq_enter() -> *rcu_nmi_enter()* > -> run_irq_on_irqstack_cond() -> irq_exit_rcu() -> *preempt_count_add(HARDIRQ_OFFSET)* > -> // Run IRQ... > > NMI path: > -> x86_64 asm (entry_64.S) > -> irqentry_nmi_enter() -> __nmi_enter() -> *__preempt_count_add(NMI_OFFSET + HARDIRQ_OFFSET)* > -> *rcu_nmi_enter()* > > For arm64, see 'arch/arm64/kernel/entry-common.c'. >
WARNING: multiple messages have this Message-ID (diff)
From: Mark Rutland <mark.rutland@arm.com> To: Nicolas Saenz Julienne <nsaenzju@redhat.com> Cc: linux-kernel <linux-kernel@vger.kernel.org>, linux-arm-kernel <linux-arm-kernel@lists.infradead.org>, rcu@vger.kernel.org, Thomas Gleixner <tglx@linutronix.de>, Peter Zijlstra <peterz@infradead.org>, Steven Rostedt <rostedt@goodmis.org>, mtosatti <mtosatti@redhat.com>, frederic <frederic@kernel.org>, paulmck@kernel.org Subject: Re: Question WRT early IRQ/NMI entry code Date: Tue, 30 Nov 2021 12:50:30 +0000 [thread overview] Message-ID: <YaYeFu4hi3uVkhkN@FVFF77S0Q05N> (raw) In-Reply-To: <8719ad46cc29a2c5d7baac3c35770e5460ab8d5c.camel@redhat.com> On Tue, Nov 30, 2021 at 12:28:41PM +0100, Nicolas Saenz Julienne wrote: > Hi All, Hi Nicolas, > while going over the IRQ/NMI entry code I've found a small 'inconsistency': > while in the IRQ entry path, we inform RCU of the context change *before* > incrementing the preempt counter, the opposite happens for the NMI entry > path. This applies to both arm64 and x86[1]. For arm64, the style was copied from the x86 code, and (AFAIK) I had no particular reason for following either order other than consistency with x86. > Actually, rcu_nmi_enter() — which is also the main RCU context switch function > for the IRQ entry path — uses the preempt counter to verify it's not in NMI > context. So it would make sense to assume all callers have the same updated > view of the preempt count, which isn't true ATM. I agree consistency would be nice, assuming there's no issue preventing us from moving the IRQ preempt_count logic earlier. It sounds like today the ordering is only *required* when entering an NMI, and we already do the right thing there. Do you see a case where something would go wrong (or would behave differently with the flipped ordering) for IRQ today? > I'm sure there an obscure/non-obvious reason for this, right? TBH I suspect this is mostly oversight / legacy, and likely something we can tighten up. Thanks, Mark. > > Thanks! > Nicolas > > [1] > IRQ path: > -> x86_64 asm (entry_64.S) > -> irqentry_enter() -> rcu_irq_enter() -> *rcu_nmi_enter()* > -> run_irq_on_irqstack_cond() -> irq_exit_rcu() -> *preempt_count_add(HARDIRQ_OFFSET)* > -> // Run IRQ... > > NMI path: > -> x86_64 asm (entry_64.S) > -> irqentry_nmi_enter() -> __nmi_enter() -> *__preempt_count_add(NMI_OFFSET + HARDIRQ_OFFSET)* > -> *rcu_nmi_enter()* > > For arm64, see 'arch/arm64/kernel/entry-common.c'. > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-11-30 12:50 UTC|newest] Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-11-30 11:28 Question WRT early IRQ/NMI entry code Nicolas Saenz Julienne 2021-11-30 11:28 ` Nicolas Saenz Julienne 2021-11-30 12:05 ` Frederic Weisbecker 2021-11-30 12:05 ` Frederic Weisbecker 2021-11-30 12:50 ` Mark Rutland [this message] 2021-11-30 12:50 ` Mark Rutland 2021-11-30 13:47 ` Thomas Gleixner 2021-11-30 13:47 ` Thomas Gleixner 2021-11-30 14:13 ` Steven Rostedt 2021-11-30 14:13 ` Steven Rostedt 2021-11-30 22:31 ` [PATCH] Documentation: Fill the gaps about entry/noinstr constraints Thomas Gleixner 2021-11-30 22:31 ` Thomas Gleixner 2021-12-01 10:56 ` Mark Rutland 2021-12-01 10:56 ` Mark Rutland 2021-12-01 18:14 ` Thomas Gleixner 2021-12-01 18:14 ` Thomas Gleixner 2021-12-01 18:23 ` Mark Rutland 2021-12-01 18:23 ` Mark Rutland 2021-12-01 20:28 ` Thomas Gleixner 2021-12-01 20:28 ` Thomas Gleixner 2021-12-01 20:35 ` [PATCH v2] " Thomas Gleixner 2021-12-01 20:35 ` Thomas Gleixner 2021-12-02 10:03 ` Mark Rutland 2021-12-02 10:03 ` Mark Rutland 2021-12-03 20:08 ` Paul E. McKenney 2021-12-03 20:08 ` Paul E. McKenney 2021-12-13 10:36 ` Nicolas Saenz Julienne 2021-12-13 10:36 ` Nicolas Saenz Julienne 2021-12-13 16:41 ` Paul E. McKenney 2021-12-13 16:41 ` Paul E. McKenney 2021-12-04 3:48 ` Randy Dunlap 2021-12-04 3:48 ` Randy Dunlap 2021-12-06 17:36 ` Mark Rutland 2021-12-06 17:36 ` Mark Rutland 2021-12-06 17:53 ` Paul E. McKenney 2021-12-06 17:53 ` Paul E. McKenney 2021-12-06 21:24 ` Randy Dunlap 2021-12-06 21:24 ` Randy Dunlap 2021-12-06 21:36 ` Paul E. McKenney 2021-12-06 21:36 ` Paul E. McKenney 2021-11-30 15:13 ` Question WRT early IRQ/NMI entry code Nicolas Saenz Julienne 2021-11-30 15:13 ` Nicolas Saenz Julienne
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=YaYeFu4hi3uVkhkN@FVFF77S0Q05N \ --to=mark.rutland@arm.com \ --cc=frederic@kernel.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=mtosatti@redhat.com \ --cc=nsaenzju@redhat.com \ --cc=paulmck@kernel.org \ --cc=peterz@infradead.org \ --cc=rcu@vger.kernel.org \ --cc=rostedt@goodmis.org \ --cc=tglx@linutronix.de \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.