On Mon, Oct 19, 2020 at 11:32:50AM +0200, Thomas Gleixner wrote: > On Sun, Oct 18 2020 at 22:37, Ira Weiny wrote: > > On Fri, Oct 16, 2020 at 02:55:21PM +0200, Thomas Gleixner wrote: > >> Subject: x86/entry: Move nmi entry/exit into common code > >> From: Thomas Gleixner <t...@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. > > No. This has absolutely nothing to do with PKRS. It's a cleanup valuable > by itself and that's how it should have been done right away. > > So the proper changelog is: > > 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.
Ok sounds good, thanks. > > >> --- 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. > > Why the heck should it be changed in a patch which adds something > completely different? Because the PKRS stuff is used in both NMI and IRQ state. > > Either it's mutually exclusive or not and if so it want's to be done in > this patch and not in a change which extends the struct for other > reasons. Sorry, let me clarify. After this patch we have. typedef union irqentry_state { bool exit_rcu; bool lockdep; } irqentry_state_t; Which reflects the mutual exclusion of the 2 variables. But then when the pkrs stuff is added the union changes back to a structure and looks like this. typedef struct irqentry_state { #ifdef CONFIG_ARCH_HAS_SUPERVISOR_PKEYS u32 pkrs; u32 thread_pkrs; #endif union { bool exit_rcu; bool lockdep; }; } irqentry_state_t; Because the pkrs information is in addition to exit_rcu OR lockdep. So this is what I meant by 'could be changed easy enough in the follow on patch'. Is that clear? Ira _______________________________________________ Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org To unsubscribe send an email to linux-nvdimm-le...@lists.01.org