On Fri, Oct 16, 2020 at 02:55:21PM +0200, Thomas Gleixner wrote:
On Fri, Oct 16 2020 at 13:45, Peter Zijlstra wrote:
On Fri, Oct 09, 2020 at 12:42:55PM -0700, ira.weiny@intel.com wrote:
@@ -238,7 +236,7 @@ noinstr void idtentry_exit_nmi(struct pt_regs *regs, bool restore) rcu_nmi_exit(); lockdep_hardirq_exit();
- if (restore)
- if (irq_state->exit_rcu) lockdep_hardirqs_on(CALLER_ADDR0); __nmi_exit();
}
That's not nice.. The NMI path is different from the IRQ path and has a different variable. Yes, this works, but *groan*.
Maybe union them if you want to avoid bloating the structure, but the above makes it really hard to read.
Right, and also that nmi entry thing should not be in x86. Something like the untested below as first cleanup.
Ok, I see what Peter was talking about. I've added this patch to the series.
Thanks,
tglx
Subject: x86/entry: Move nmi entry/exit into common code From: Thomas Gleixner tglx@linutronix.de Date: Fri, 11 Sep 2020 10:09:56 +0200
Add blurb here.
How about:
To prepare for saving PKRS values across NMI's we lift the idtentry_[enter|exit]_nmi() to the common code. Rename them to irqentry_nmi_[enter|exit]() to reflect the new generic nature and store the state in the same irqentry_state_t structure as the other irqentry_*() functions. Finally, differentiate the state being stored between the NMI and IRQ path by adding 'lockdep' to irqentry_state_t.
?
Signed-off-by: Thomas Gleixner tglx@linutronix.de
arch/x86/entry/common.c | 34 ---------------------------------- arch/x86/include/asm/idtentry.h | 3 --- arch/x86/kernel/cpu/mce/core.c | 6 +++--- arch/x86/kernel/nmi.c | 6 +++--- arch/x86/kernel/traps.c | 13 +++++++------ include/linux/entry-common.h | 20 ++++++++++++++++++++ kernel/entry/common.c | 36 ++++++++++++++++++++++++++++++++++++ 7 files changed, 69 insertions(+), 49 deletions(-)
[snip]
--- a/include/linux/entry-common.h +++ b/include/linux/entry-common.h @@ -343,6 +343,7 @@ void irqentry_exit_to_user_mode(struct p #ifndef irqentry_state typedef struct irqentry_state { bool exit_rcu;
- bool lockdep;
} irqentry_state_t;
Building on what Peter said do you agree this should be made into a union?
It may not be strictly necessary in this patch but I think it would reflect the mutual exclusivity better and could be changed easy enough in the follow on patch which adds the pkrs state.
Ira