On Wed, Oct 14, 2020 at 09:06:44PM -0700, Dave Hansen wrote:
On 10/14/20 8:46 PM, Ira Weiny wrote:
On Tue, Oct 13, 2020 at 11:52:32AM -0700, Dave Hansen wrote:
On 10/9/20 12:42 PM, ira.weiny@intel.com wrote:
@@ -341,6 +341,9 @@ noinstr void irqentry_enter(struct pt_regs *regs, irqentry_state_t *state) /* Use the combo lockdep/tracing function */ trace_hardirqs_off(); instrumentation_end();
+done:
- irq_save_pkrs(state);
}
One nit: This saves *and* sets PKRS. It's not obvious from the call here that PKRS is altered at this site. Seems like there could be a better name.
Even if we did:
irq_save_set_pkrs(state, INIT_VAL);
It would probably compile down to the same thing, but be *really* obvious what's going on.
I suppose that is true. But I think it is odd having a parameter which is the same for every call site.
Well, it depends on what you optimize for. I'm trying to optimize for the code being understood quickly the first time someone reads it. To me, that's more important than minimizing the number of function parameters (which are essentially free).
Agreed. Sorry I was not trying to be confrontational. There is just enough other things which are going to take me time to get right I need to focus on them... :-D
Sorry, Ira