Messages in this thread | | | Date | Fri, 14 Nov 2008 02:16:10 +0100 | From | Nick Piggin <> | Subject | Re: [rfc] x86: optimise page fault path a little |
| |
On Thu, Nov 13, 2008 at 05:06:33PM +0100, Ingo Molnar wrote: > > * Nick Piggin <npiggin@suse.de> wrote: > > > > 32-bit should use oops_begin() too. Solves the previous comment as > > > well. > > > > > > > +#ifdef CONFIG_X86_32 > > > > + die("Oops", regs, error_code); > > > > + bust_spinlocks(0); > > > > + do_exit(SIGKILL); > > > > +#else > > > > + if (__die("Oops", regs, error_code)) > > > > + regs = NULL; > > > > + /* Executive summary in case the body of the oops scrolled away */ > > > > + printk(KERN_EMERG "CR2: %016lx\n", address); > > > > + oops_end(flags, regs, SIGKILL); > > > > +#endif > > > > > > this difference seems unnecessary too - 32-bit should use oops_end() > > > too. > > > > Probably all 3 good comments, but I didn't want to be tempted into > > changing behaviour (modulo adding bugs). Easy to merge them up in a > > subsequent patch, however... > > please do feel tempted to clean this code up - and if it changes > behavior, split it up into smaller steps. The patch is already quite > large with a flux of 430 lines: > > 1 file changed, 251 insertions(+), 188 deletions(-) > > ... your primary goal is faster code, we also want cleaner code. I > think we can meet in the middle, have faster _and_ cleaner code, and > it's a done deal ;)
I might. But the flux is "supposed" to just be moving things around so it compiles better. Any behaviour change should be a bug.
So I definitely won't add any of these types of cleanups in the same patch. And with or without cleanups, the patch should stand on its own to get merged -- if it's good, it's good; if not, not :)
That, and, I don't really know the subtlties of this code or what made it so tricky that such a cleanup wasn't done when merging the two files.
| |